Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade client active object mgr tests to Catch2 #14565

Merged

Conversation

JosiahWI
Copy link
Contributor

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-unittests look correct.

JosiahWI and others added 5 commits April 20, 2024 07:01
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 JosiahWI mentioned this pull request Apr 22, 2024
5 tasks
JosiahWI and others added 2 commits May 14, 2024 11:07
This switches infostream to rawstream so that test runner output is
displayed, and returns the correct boolean depending on the results. The
tests are now found by setting the configuration instead of invoking the
command line parser.
Co-Authored-By: Lars Müller <appgurulars@gmx.de>
@JosiahWI JosiahWI force-pushed the feat/catch2-client-active-object-mgr-tests branch from 45860f7 to c2bd250 Compare May 14, 2024 16:21
Copy link
Contributor

@appgurueu appgurueu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of c2bd250 and b02ed55 this has my reapproval.

Tested --run-unittests and --run-unittests --test-module ... with TestClientActiveObjectMgr and "test client active object manager" and a nonexistent module name.

(Also ported the tags from the old PR.)

@appgurueu appgurueu added One approval ✅ ◻️ Roadmap The change matches an item on the current roadmap. Feature ✨ PRs that add or enhance a feature Code quality labels May 14, 2024
src/unittest/test.cpp Show resolved Hide resolved
src/unittest/test.cpp Outdated Show resolved Hide resolved
@sfan5 sfan5 changed the title Upgrade client active object mgr tests to Catch2 (reviving #13609) Upgrade client active object mgr tests to Catch2 May 15, 2024
JosiahWI and others added 4 commits May 17, 2024 06:27
It's necessary to flush stderr before printing to stdout in adition to
flushing stdout before printing to stderr, to make sure all output is
ordered correctly.
@rubenwardy rubenwardy self-requested a review May 18, 2024 13:53
@appgurueu appgurueu merged commit 1298374 into minetest:master May 22, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality Feature ✨ PRs that add or enhance a feature Roadmap The change matches an item on the current roadmap. >= Two approvals ✅ ✅
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants