diff --git a/CMakeLists.txt b/CMakeLists.txt index b8254db7fab8..4c6c60ba98ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -278,7 +278,7 @@ if(NOT USE_LUAJIT) add_subdirectory(lib/bitop) endif() -if(BUILD_BENCHMARKS) +if(BUILD_UNITTESTS OR BUILD_BENCHMARKS) add_subdirectory(lib/catch2) endif() diff --git a/lib/catch2/CMakeLists.txt b/lib/catch2/CMakeLists.txt index 3c471d49dfcf..07727257182c 100644 --- a/lib/catch2/CMakeLists.txt +++ b/lib/catch2/CMakeLists.txt @@ -13,4 +13,5 @@ # + return os << std::fixed << duration.value() << ' ' << duration.unitsAsString(); add_library(catch2 INTERFACE) -target_include_directories(catch2 INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}) +add_library(Catch2::Catch2 ALIAS catch2) +target_include_directories(catch2 INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}") diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index bad7e2afb952..6ea544980ee2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -615,8 +615,8 @@ if(BUILD_CLIENT) if (USE_SPATIAL) target_link_libraries(${PROJECT_NAME} ${SPATIAL_LIBRARY}) endif() - if(BUILD_BENCHMARKS) - target_link_libraries(${PROJECT_NAME} catch2) + if(BUILD_UNITTESTS OR BUILD_BENCHMARKS) + target_link_libraries(${PROJECT_NAME} Catch2::Catch2) endif() endif(BUILD_CLIENT) @@ -677,8 +677,8 @@ if(BUILD_SERVER) ${CURL_LIBRARY} ) endif() - if(BUILD_BENCHMARKS) - target_link_libraries(${PROJECT_NAME}server catch2) + if(BUILD_UNITTESTS OR BUILD_BENCHMARKS) + target_link_libraries(${PROJECT_NAME}server Catch2::Catch2) endif() endif(BUILD_SERVER) diff --git a/src/activeobjectmgr.h b/src/activeobjectmgr.h index 0205aee85223..369e8acf51e2 100644 --- a/src/activeobjectmgr.h +++ b/src/activeobjectmgr.h @@ -31,7 +31,6 @@ class TestServerActiveObjectMgr; template class ActiveObjectMgr { - friend class ::TestClientActiveObjectMgr; friend class ::TestServerActiveObjectMgr; public: diff --git a/src/unittest/test.cpp b/src/unittest/test.cpp index 7141ee0c6b91..2fbba5e4a79f 100644 --- a/src/unittest/test.cpp +++ b/src/unittest/test.cpp @@ -17,6 +17,11 @@ with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#define CATCH_CONFIG_RUNNER +// we want to have catch write to rawstream (stderr) instead of stdout +#define CATCH_CONFIG_NOSTDOUT +#include + #include "test.h" #include "nodedef.h" @@ -27,6 +32,21 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "porting.h" #include "debug.h" +#include + +// make catch write everything to rawstream +namespace Catch { + std::ostream& cout() { + return rawstream; + } + std::ostream& clog() { + return rawstream; + } + std::ostream& cerr() { + return rawstream; + } +} + content_t t_CONTENT_STONE; content_t t_CONTENT_GRASS; content_t t_CONTENT_TORCH; @@ -234,6 +254,16 @@ bool run_tests() num_total_tests_run += testmod->num_tests_run; } + rawstream << "Catch test results: " << std::endl; + auto num_catch_tests_failed = Catch::Session().run(); + // We count the all the Catch tests as one test for Minetest's own logging + // because we don't have a way to tell how many individual tests Catch ran. + ++num_total_tests_run; + if (num_catch_tests_failed > 0) { + ++num_modules_failed; + ++num_total_tests_failed; + } + u64 tdiff = porting::getTimeMs() - t1; g_logger.setLevelSilenced(LL_ERROR, false); @@ -269,8 +299,11 @@ bool run_tests(const std::string &module_name) auto testmod = findTestModule(module_name); if (!testmod) { - errorstream << "Test module not found: " << module_name << std::endl; - return 1; + rawstream << "Did not find module, searching Catch tests: " << module_name << std::endl; + Catch::Session session; + session.configData().testsOrTags.push_back(module_name); + auto catch_test_failures = session.run(); + return catch_test_failures == 0; } g_logger.setLevelSilenced(LL_ERROR, true); diff --git a/src/unittest/test_clientactiveobjectmgr.cpp b/src/unittest/test_clientactiveobjectmgr.cpp index d3cd83dc97de..2df099fb1754 100644 --- a/src/unittest/test_clientactiveobjectmgr.cpp +++ b/src/unittest/test_clientactiveobjectmgr.cpp @@ -17,11 +17,14 @@ with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include "client/activeobjectmgr.h" -#include #include "test.h" -#include "profiler.h" +#include "client/activeobjectmgr.h" + +#include + +#include +#include class TestClientActiveObject : public ClientActiveObject { @@ -58,9 +61,6 @@ class TestClientActiveObjectMgr : public TestBase void runTests(IGameDef *gamedef); - void testFreeID(); - void testRegisterObject(); - void testRemoveObject(); void testGetActiveSelectableObjects(); }; @@ -68,75 +68,74 @@ static TestClientActiveObjectMgr g_test_instance; void TestClientActiveObjectMgr::runTests(IGameDef *gamedef) { - TEST(testFreeID); - TEST(testRegisterObject) - TEST(testRemoveObject) TEST(testGetActiveSelectableObjects) } //////////////////////////////////////////////////////////////////////////////// -void TestClientActiveObjectMgr::testFreeID() -{ - client::ActiveObjectMgr caomgr; - std::vector aoids; - - u16 aoid = caomgr.getFreeId(); - // Ensure it's not the same id - UASSERT(caomgr.getFreeId() != aoid); - - aoids.push_back(aoid); - - // Register basic objects, ensure we never found - for (u8 i = 0; i < UINT8_MAX; i++) { - // Register an object - auto tcao_u = std::make_unique(); - auto tcao = tcao_u.get(); - caomgr.registerObject(std::move(tcao_u)); - aoids.push_back(tcao->getId()); - - // Ensure next id is not in registered list - UASSERT(std::find(aoids.begin(), aoids.end(), caomgr.getFreeId()) == - aoids.end()); - } - - caomgr.clear(); +static TestClientActiveObject* register_default_test_object( + client::ActiveObjectMgr &caomgr) { + auto test_obj = std::make_unique(); + auto result = test_obj.get(); + REQUIRE(caomgr.registerObject(std::move(test_obj))); + return result; } -void TestClientActiveObjectMgr::testRegisterObject() +TEST_CASE("test client active object manager") { client::ActiveObjectMgr caomgr; - auto tcao_u = std::make_unique(); - auto tcao = tcao_u.get(); - UASSERT(caomgr.registerObject(std::move(tcao_u))); - - u16 id = tcao->getId(); - - auto tcaoToCompare = caomgr.getActiveObject(id); - UASSERT(tcaoToCompare->getId() == id); - UASSERT(tcaoToCompare == tcao); - - tcao_u = std::make_unique(); - tcao = tcao_u.get(); - UASSERT(caomgr.registerObject(std::move(tcao_u))); - UASSERT(caomgr.getActiveObject(tcao->getId()) == tcao); - UASSERT(caomgr.getActiveObject(tcao->getId()) != tcaoToCompare); - - caomgr.clear(); -} - -void TestClientActiveObjectMgr::testRemoveObject() -{ - client::ActiveObjectMgr caomgr; - auto tcao_u = std::make_unique(); - auto tcao = tcao_u.get(); - UASSERT(caomgr.registerObject(std::move(tcao_u))); + auto tcao1 = register_default_test_object(caomgr); + + SECTION("When we register many client objects, " + "then all the assigned IDs should be unique.") + { + // This should be enough rounds to be pretty confident + // there are no duplicates. + u16 n = 255; + std::unordered_set ids; + ids.insert(tcao1->getId()); + for (u16 i = 0; i < n; ++i) { + auto other_tcao = register_default_test_object(caomgr); + ids.insert(other_tcao->getId()); + } + // n added objects & tcao1 + CHECK(n + 1 == static_cast(ids.size())); + } - u16 id = tcao->getId(); - UASSERT(caomgr.getActiveObject(id) != nullptr) + SECTION("two registered objects") + { + auto tcao2 = register_default_test_object(caomgr); + auto tcao2_id = tcao2->getId(); + + auto obj1 = caomgr.getActiveObject(tcao1->getId()); + REQUIRE(obj1 != nullptr); + + auto obj2 = caomgr.getActiveObject(tcao2_id); + REQUIRE(obj2 != nullptr); + + SECTION("When we query an object by its ID, " + "then we should get back an object with that ID.") + { + CHECK(obj1->getId() == tcao1->getId()); + CHECK(obj2->getId() == tcao2->getId()); + } + + SECTION("When we register and query for an object, " + "its memory location should not have changed.") + { + CHECK(obj1 == tcao1); + CHECK(obj2 == tcao2); + } + } - caomgr.removeObject(tcao->getId()); - UASSERT(caomgr.getActiveObject(id) == nullptr) + SECTION("Given an object has been removed, " + "when we query for it, " + "then we should get nullptr.") + { + auto id = tcao1->getId(); + caomgr.removeObject(tcao1->getId()); // may invalidate tcao1 + CHECK(caomgr.getActiveObject(id) == nullptr); + } caomgr.clear(); }