Replace doctests with in-house equivalent

[Repiteo]

I'm in the process of creating a proposal for standardizing benchmarking across the godot repo, and am making several comparisons to both the existing doctests system & our existing benchmarking repo. What I've come to realize is that our use of doctests has been mishandled to the point it warrants a dedicated discussion. Our current implementation has a required header include for every single test & exposes all test internals in header files, which serves to hide dependency issues that're currently running rampant in our repo. While doctools should have a system where we can declare functions in header files & define them in source files, we still risk hiding issues from the required workarounds for converting to their dedicated systems & equality functions. This, compounded with simply not needing the majority of doctest's functionality, makes me believe that we should migrate to an in-house system instead.

This would, understandably, be a rather significant change. So it feels appropriate to lay bare what kind of goals and requirements would be expected of an idealized testing system (non-exhaustive):

1. Migrate tests to source files. This is the area which should be possible while still using doctests, making it the most suitable for a transitionary PR. Having the test functions only declared in an otherwise blank header file would mean that the source could include exclusively what it needs to include, nailing down the troublemakers of our faulty #include system.
2. Remove stress-tests/benchmarks. Our test system should not double as a benchmarking utility. Our tests are meant for sanity checks and ensuring nuanced/fringe behavior doesn't suddenly break after seemingly innocuous changes. Anything that strays from this should either have scale reduced to account for it, or be removed entirely.
3. Utilize static assertions. Any test that can statically assert, should statically assert. These are especially important because users wouldn't have to individually run tests to catch regressions, they'd be caught on build or even in the IDE itself well ahead of time. Note that this is somewhat limited by several areas of core not yet being constexpr, though that's being worked on.
4. Fail on error output. Right now, our test output is absolutely littered with error messages, despite the tests themselves passing. This went unnoticed because CI doesn't fail on Godot error messages (yet?), but that ultimately shouldn't be burdened by external checkers; the test itself should be responsible. An in-house tester would be able to track these more easily, as it could integrate with our existing logging/stderr systems (though those might warrant an overhaul in their own right, but that's beyond the scope of this discussion).

[Calinou]

One downside of a in-house unit testing solution is that we'll lose the benefits of IDE integration. I'm not sure how many people rely on IDE integrations for our existing doctest suite though (I don't).

2. Remove stress-tests/benchmarks. Our test system should not double as a benchmarking utility. Our tests are meant for sanity checks and ensuring nuanced/fringe behavior doesn't suddenly break after seemingly innocuous changes. Anything that strays from this should either have scale reduced to account for it, or be removed entirely.

Stress tests were added before godot-benchmarks was in place, so I agree these should all be moved to godot-benchmarks now.

3. Utilize static assertions. Any test that can statically assert, should statically assert. These are especially important because users wouldn't have to individually run tests to catch regressions, they'd be caught on build or even in the IDE itself well ahead of time. Note that this is somewhat limited by several areas of core not yet being constexpr, though that's being worked on.

C++20 supports if constexpr in more scenarios. Would it be acceptable to require it for compiling unit tests only? The rest of the engine would still only require C++17 for now.

This went unnoticed because CI doesn't fail on Godot error messages (yet?), but that ultimately shouldn't be burdened by external checkers; the test itself should be responsible.

godotengine/godot#99254 should handle this in the future. While having this behavior part of the unit test might make more sense, we can make it so that --test implies --fail-on-error.

[Repiteo]

Oh wow, that addresses several of my concerns with the system straight away! Maybe this is as simple as refactoring our doctest integration after all.

One downside of a in-house unit testing solution is that we'll lose the benefits of IDE integration. I'm not sure how many people rely on IDE integrations for our existing doctest suite though (I don't).

…I had no idea there was IDE integration to begin with. If we stick with doctest, that'd be good to take advantage of.

C++20 supports if constexpr in more scenarios. Would it be acceptable to require it for compiling unit tests only? The rest of the engine would still only require C++17 for now.

Considering we're already doing exactly that for the metal driver, I see no issue with this. Though we should wait for a scenario where the added functionality would be relevant to crop up first, as nothing immediately springs to mind with our existing tests/structures.

godotengine/godot#99254 should handle this in the future. While having this behavior part of the unit test might make more sense, we can make it so that --test implies --fail-on-error.

That, or the CI test could pass --fail-on-error explicitly. Either way, it would handle that headache quite nicely.

[Calinou]

…I had no idea there was IDE integration to begin with. If we stick with doctest, that'd be good to take advantage of.

CLion mentions doctest integration on its product page: https://www.jetbrains.com/clion/#:~:text=testboost.testcatch2doctest

[van800]

In Rider it is possible to run/debug doctests. For Godot it is currently required to add --test to Project | Properties | [Configuration] | Test Runner | Command Argument. I want to make it auto-detected though.

[fire]

Fail on error output. @Ivorforce I think this is an issue report. Thoughts?

[Ivorforce]

I'm not a CLion expert, but it sounds like it.

[simpkins]

1. Migrate tests to source files. This is the area which should be possible while still using doctests, making it the most suitable for a transitionary PR. Having the test functions only declared in an otherwise blank header file would mean that the source could include exclusively what it needs to include, nailing down the troublemakers of our faulty #include system.

The fact that all of the unit tests are compiled as part of test_main.cpp does seem rather annoying. It takes around 50s to compile test_main.cpp on my system, which is a little painful when you just are trying to iterate on a single test. It's possible to just #if 0 out the other test headers I don't care about while working on one test, but this is rather awkward.

In order to be able to define test cases in separate .cpp files we would need to link the test library with -Wl,--whole-archive (for gcc/clang) or /WHOLEARCHIVE (for MSVC). This flag is needed to ensure the linker includes the test cpp files even if nothing else directly references them. (Having a constructor functions that register test cases unfortunately isn't sufficient to ensure that the gets included.) If that's the only blocker to being able to define tests in .cpp files I can send out a diff to add this flag for the test library.

[simpkins]

I've made a PR to add these linker flags: godotengine/godot#108415
I've tested it on Linux with gcc and Windows with MSVC, but it could probably use testing on other platforms.

[natinusala]

This is a welcome change, not only for building times but also for IDE support. Intellisense on VSCode works very slowly when editing header files for some reason. It takes about 30s to refresh at every file change, it's unusable. Writing entire tests in headers is a pain for me ATM because of this. I'm looking forward to seeing this merged!