Add test for issue #40794

[Xrayez]

This is currently a proof-of-concept towards TDD approach to testing in Godot. 🙂

Aims to cover #40794 once fixed.

This PR currently fails (not detected by CI, only with --no-skip option). , this can be either:

1. Rebased to ensure correct functionality (once Validate_Json() and parse_json does not catch missing start "{" squirly bracket #40794 is fixed).
2. Merged so that other devs could work on pending issues like this (just run godot --test --no-skip to work on documented pending tests/issues known to be reproducible).

[Xrayez]

Current output:

[doctest] doctest version is "2.4.0"
[doctest] run with "--help" for options
===============================================================================
tests/test_json.h:42:
TEST SUITE: [IO][JSON] JSON issues
TEST CASE:  Issue #40676

tests/test_json.h:53: ERROR: CHECK( parsed_type != Variant::STRING ) is NOT correct!
  values: CHECK( 4 != 4 )
  logged: Should not prematurely parse as string.

tests/test_json.h:54: FATAL ERROR: REQUIRE( parsed_type == Variant::DICTIONARY ) is NOT correct!
  values: REQUIRE( 4 == 24 )
  logged: Parsed JSON should represent a dictionary

===============================================================================
[doctest] test cases:     36 |     35 passed |      1 failed |      0 skipped
[doctest] assertions:    153 |    151 passed |      2 failed |
[doctest] Status: FAILURE!
##[error]Process completed with exit code 1.

[Xrayez]

Actually the test was "wrong", it should treat the invalid JSON as parse error, but it currently doesn't, so it proceeds to parsing the next token as a string:

[doctest] doctest version is "2.4.0"
[doctest] run with "--help" for options
===============================================================================
D:\src\godot\tests\test_json.h(42):
TEST SUITE: [IO][JSON] JSON issues
TEST CASE:  Issue #40676

D:\src\godot\tests\test_json.h(51): ERROR: CHECK( err == ERR_PARSE_ERROR ) is NOT correct!
  values: CHECK( 0 == 43 )
  logged: Missing starting curly bracket, this should be treated as a parse error.

D:\src\godot\tests\test_json.h(52): ERROR: CHECK( parsed.get_type() != Variant::STRING ) is NOT correct!
  values: CHECK( 4 != 4 )
  logged: Should not prematurely parse as string.

===============================================================================
[doctest] test cases:     36 |     35 passed |      1 failed |      0 skipped
[doctest] assertions:    153 |    151 passed |      2 failed |
[doctest] Status: FAILURE!

[Xrayez]

As it turns out, doctest provides a bunch of decorators.

So, we could use may_fail decorator for CI, so they are still printed as failures but do not contribute to CI result, which is nice.

But that also mean that when we accumulate many such pending tests without actually fixing them, the output would be polluted with those errors, and would make it more difficult to navigate to errors which caused by actual regressions (if any), though the CI would still fail in that case, so no problem.

There's another skip decorator. Using it on test cases completely prevents those kinds of tests to be run in the first place. Those who'd like to work on fixing issues can run the tests with --no-skip option locally:

godot --test --no-skip

The downside of using skip is that pending tests are sort of hidden from developers unless they know about --no-skip option, but that could be documented, so no problem.

CI configs don't need to be changed either.

Or use both decorators...

[Xrayez]

Notice "1 skipped" originating from this PR.

Run ./bin/godot.linuxbsd.opt.tools.64.mono --test
[doctest] doctest version is "2.4.0"
[doctest] run with "--help" for options
===============================================================================
[doctest] test cases:     37 |     37 passed |      0 failed |      1 skipped
[doctest] assertions:    161 |    161 passed |      0 failed |
[doctest] Status: SUCCESS!

[Xrayez]

This also works:

Suggested change

	TEST_CASE("Issue #40676" * doctest::skip()) {
	TEST_CASE("Issue #40676" * doctest::may_fail() * doctest::skip()) {

[Xrayez]

#40804 is there to fix #40794. With #40804, all assertions in this PR currently pass, which is great. What's left is either merging this PR so that #40804 could be rebased (the author would also have to remove doctest::skip() decorator from the test to make it as ready), or this PR would have to be rebased once #40804 is merged to master. In either case, this kind of workflow would be proven to be useful IMO.

[Calinou]

Raw string literals could be useful to make JSON in C++ easier to read: R"({ "a": 12345, "b": 12345 })"

They also accept line breaks so you can write JSON over several lines without relying on string concatenation.

We already use them elsewhere in Godot, so feel free to use them here.

[Xrayez]

Thanks, yeah I recall going through C++11 primer back in the days mentioning this in fact, this is more widespread now after 10 years it seems. 😄

[Xrayez]

Rebased to use TEST_CASE_PENDING macro alias added in #40883.

[Xrayez]

Found 3 years old QA tests in godotengine/godot-tests#3, most are already fixed but I suppose this is an example of something similar that I'm trying to do here.

[Xrayez]

There was a discussion that this kind of TDD approach wouldn't be welcomed for unit testing, this was done as a proof-of-concept, so closing.