Upgrade client active object mgr tests to Catch2 #13609
Conversation
wsor4035
added
the
Roadmap: Needs approval
|
roadmap label added per zughy request. unsubscribing |
rubenwardy
added
Concept approved
Zughy
added
Roadmap
JosiahWI
force-pushed
the
feat/catch2-client-active-object-mgr-tests
branch
2 times, most recently
from
e3bc72e
to
38d2de6
Compare
|
I haven't converted the new tests in this file after the rebase, and I'm not currently planning to do it in this PR. Those tests look very clean to me, and although it would be very good to convert them, I would like to focus efforts first on tests that are fragile or incomplete. However, if someone really wants me to convert those in this PR as well, please say so. |
In addition to invoking Catch2's test runner after Minetest's homemade runner, this refactors the tests to follow the DRY principle, and gives them expressive names and clear assertions. Catch2 is already bundled with Minetest, so there are no added dependencies.
JosiahWI
force-pushed
the
feat/catch2-client-active-object-mgr-tests
branch
from
490437e
to
47e88c6
Compare
| << "++++++++++++++++++++++++++++++++++++++++" << std::endl; | ||
| << "++++++++++++++++++++++++++++++++++++++++" << std::endl | ||
| << "Catch test results: " << std::endl; | ||
| auto catch_test_failures = Catch::Session().run(); |
There was a problem hiding this comment.
this just runs the other tests unconditionally after the normal ones with no integration with --test-module
| TEST(testGetActiveSelectableObjects) | ||
| } | ||
|
|
||
| //////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| void TestClientActiveObjectMgr::testFreeID() |
There was a problem hiding this comment.
what is the plan if any converted tests need to access the gamedef?
There was a problem hiding this comment.
Good question. The plan is to move the TestGameDef to its own file and instantiate an instance of it for each Catch test that needs it. If that proves to be too slow, then I think mocking some parts of the gamedef might be appropriate. That's more work than a global gamedef, which almost looks like an amazing solution, but there's a big danger with that because the global instance might be modified by a test and not completely reset at the end. The tests would no longer be independent of each other and the order in which the tests are run could start to matter, which is a big mess. Looking at the code, it seems we could already have this problem.
sfan5
added
the
Action / change needed
Zughy
added
the
Adoption needed
|
I'll open this when I'm ready to work on it again; probably at the end of the spring semester. |
In addition to invoking Catch2's test runner after Minetest's homemade runner, this refactors the tests to have expressive names and removes some redundant or useless checks.
The Catch2 runner is invoked after Minetest's runner is finished, and "Catch2 test results:" will be printed out directly before invoking the Catch2 runner.
Part of #13610
To do
Ready for Review.
How to test
Confirm that the results of
minetest --run-unittestslook correct.