GD-168: Incorrect assertion messages

[esainane]

The used GdUnit4 version

4.4.4 (Pre Release/Master branch)

The used Godot version

v4.3.1.rc.mono.custom_build [ff9bc0422]

Operating System

6.11.5+bpo-amd64 #1 SMP PREEMPT_DYNAMIC Debian GNU/Linux trixie/sid 6.11.5-1~bpo12+1 (2024-11-11) x86_64

Describe the bug

When an assertion fails, some error messages place the actual value after the expected: marker, and the expected value after the but was marker. The expected value should come after Expected:.

For example, this current code prints Expecting: <expected> not equal to <actual>, as expected:

static func error_not_equal(current :Variant, expected :Variant) -> String:
	return "%s\n %s\n not equal to\n %s" % [_error("Expecting:"), _colored_value(expected), _colored_value(current)]

However, this current code prints messages with the actual and expected values reversed, in the form Unexpected type comparison: Expecting type <actual> but is <expected>:

static func error_is_wrong_type(current_type :Variant.Type, expected_type :Variant.Type) -> String:
	return "%s\n Expecting type %s but is %s" % [
		_error("Unexpected type comparison:"),
		_colored_value(GdObjects.type_as_string(current_type)),
		_colored_value(GdObjects.type_as_string(expected_type))]

Many other functions also have this problem. For example, all of the array assert functions have the actual and expected parameters swapped in their error message.

Steps to Reproduce

Create a small test case with a failing array assertion:

namespace GDUnit4AssertionTestExample;

using Godot;
using GdUnit4;

using static GdUnit4.Assertions;

[TestSuite]
public class ExampleTest
{
    [TestCase]
    public void FailMe() => AssertArray(new[] { "foo" }).ContainsExactly(new[] { "bar" });
}

This produces the following error:

Linked Issues

assert_string

[esainane]

I would like to fix these messages, clean up some related grammar ("do contain"), typos ("others where not expected"), use standard nomenclature (expected and actual), and adjust the related test cases accordingly, if this would be acceptable?

[MikeSchulze]

The array assert error is correct, it follows the pattern

<general error message>
<current>
"do contains"
<expected>
<additinoal infos >

It follows the JUnit error style

[esainane]

The array assert error is correct, it follows the pattern

<general error message>
<current>
"do contains"
<expected>
<additinoal infos >

It follows the JUnit error style

Thank you for clarifying the intended format! Could you confirm if this style is inspired by a specific configuration or extension, such as JUnit with Hamcrest extensions? Plain JUnit5 typically prints concise error messages:

import static org.junit.jupiter.api.Assertions.*;
import org.junit.jupiter.api.Test;

public class ExampleTest {
    @Test
    public void failMe() {
        String[] actual = {"foo"};
        String[] expected = {"bar"};
        assertArrayEquals(expected, actual);
    }
}

This prints the message "Expected [bar] but was [foo]".

A few areas I feel could be refined:

• Order and Clarity: The current message currently lists the expected value as being both expected and not expected. For example, giving AssertArray(IEnumerable<TValue> current) an expected ["expected"] and actual ["actual"]:

  AssertArray(new[] { "actual" }).ContainsExactly(new[] { "expected" })
  

  the GdUnit4 error message is:

  Expecting contains exactly elements:
   '["expected"]'
   do contains (in same order)
   '["actual"]'
  but some elements where not expected:
   '["expected"]'
  and could not find elements:
   '["actual"]'
  

  The order doesn't seem right. The first two messages read like the order is meant to be expected then actual, but if the order is actual then expected, then the first two do need to be swapped, and the messages adjusted accordingly. In the second half, the "expected" is described with "but some elements where not expected", and the "actual" is described with "and some elements could not be found".
• Grammatical: "do contains" should be "which does contain". If the order is actual then expected, it may be clearer as "Expected <actual> to contain <expected>".
• Terminology: JUnit uses expected and actual instead of expected and current, which might improve clarity and consistency:
• Typographical: "where" in "others where not expected" should be "were".
• Formatting: The inconsistent alignment in continuation lines looks unusual in the Godot Editor Plugin, though when using VS Code it seems to correct this automatically:

A revised format might look like this:

Expected array to be:
<expected>
 but was:
<actual>
 These elements were not expected:
<unexpected>
 and these elements were missing:
<missing>.

Or a concise version:

Expected array to contain exactly:
Expected: <expected>
Actual: <actual>
Unexpected: <unexpected>
Missing: <missing>

Do you think adopting either of these could improve clarity, while staying consistent with your intended conventions?

[MikeSchulze]

i test it on latest build and can't confirm that.

    [TestCase]
    public void ContainsExactly()
    {
        // test against only one element
        AssertArray(new[] { "actual" }).ContainsExactly("expected");
        AssertArray(new[] { "a", "b", "c" }).ContainsExactly("expected");
    }

Please check you used api version, can be find in the test execution log. 07:33:48.199 |I| TestRunner: JetBrains.ReSharper.TestRunner.Adapters.VsTest.Discoverer CheckGdUnit4ApiVersion gdUnit4Api, Version=4.4.0.0 Same result in Godot using your example project

Mybe i had fixed that in the past, but i can't remember it.

[MikeSchulze]

@esainane can you confirm this?

[esainane]

First, version check. When running in VS Code:

CheckGdUnit4ApiVersion gdUnit4Api, Version=4.4.0.0

I cannot seem to find a test log in the godot editor, but the panel says it is gdUnit4 4.4.4.

---

On the order topic, you are right about the array assertion example. I made a mistake when porting the test to GDScript. The order is:

line 10: Expecting contains exactly elements:
 '["actual"]'
 do contains (in same order)
 '["expected"]'
but some elements where not expected:
 '["actual"]'
and could not find elements:
 '["expected"]'

in both CS and GDScript. I've attached a fixed test project here.

The overall message still does not make sense to me, as it seems to be listing "actual" as an element both expected (in the first printed value) and not expected (in the third printed value)? I think I'm still missing something here?

The mismatched type example has a clearer message, and the order does seem wrong here:

func test_assert_vector_typefail():
    var expected = Vector2(1, 2)
    var actual = false
    assert_vector(actual).is_equal(expected)

which produces: I would expect something like "Expecting type 'Vector2' but was 'bool'"?

---

On the spacing topic, this only shows up in a GDScript test, and does not seem to be related to any behaviour in VS Code, as I previously incorrectly assumed.

When running a C# test in the godot editor, the message is consistently aligned.

When running a GDScript test in the godot editor, the second text line ("do contains ...") has a leading space, but the others do not:

gdunit4-assert-test-example.zip

[MikeSchulze]

The mismatched type example has a clearer message, and the order does seem wrong here: I would expect something like "Expecting type 'Vector2' but was 'bool'"?

Nope, the failure say the used type 'bool' is wrong for assert_vector. So it expects type bool but is Vector2 But the failure is propagated twice, yes

The problem of using wrong type is only an issue on GDScript side, on C# it is not possible to use a wrong type on typed asserts.

Ok I'm on your side, there are some failure messages that not nice to read. Before we start to refactor the failure messages, we need to define the pattern and build a list of them for each type. Then we can refactor it by build a task for each assert and describe the pattern and expected messages.

Thanks for looking into this topic

[esainane]

Ah, my apologies if this came across as accusing or argumentative! I'm really grateful for gdUnit4, and wanted to help give back.

Is there a way I could help review testing assertion report formats? I'm most familiar with catch2, though I've also used jest, pytest, python-unittest, gtest, junit, and probably more which I'm not immediately remembering right now

[MikeSchulze]

All good ;), I am grateful for any help to improve the API.

The bug reports are heavily based on JUnit and should stay in that pattern.

If you really want to get involved here, I ask you to create new tickets for each assert type separately for GDScript and CSharp API.

• Each ticket has a list of assert functions with an indication of consistent or should be revised. (checkmark)
• Add for each function a details section
  • function signature
  • test example
  • report output + screenshot
  • remarks (describe why the report is misleading and how it should look like)

The pattern is

<details>
<summary>func_name</summary>

<function_signature>

<example_test>
  
### Failure Report
<report_text>
<report_screenshot>


### Remark
<your_remark>
</details>

An example issue for GDScript 'assert_string': (not fullfilled) https://github.com/MikeSchulze/gdUnit4/issues/611

Finally, link the issue with this issue. (see main description)

[MikeSchulze]

@esainane you are still interested in improving the messages?

[esainane]

Hi, yes! Very sorry about the delay, I've been travelling and offline a lot lately. I should be able to take a look at this again tonight.

[MikeSchulze]

Please note, the master is actual under heavy development to introduce the new test engine.

[esainane]

If it would be inconvenient or distracting right now, I could also wait for a less busy time?

[MikeSchulze]

Yes 👍please wait until I get the first stable version of the new test engine running.

[esainane]

Will do!

(And as always, thank you for all of your hard work on gdUnit4!)