From 26a9bf967193b45fa1de9c3f6d836786b66e86cc Mon Sep 17 00:00:00 2001 From: Tim Siegel Date: Wed, 20 Apr 2022 20:28:13 -0400 Subject: [PATCH 01/25] unit tests: Add CTest support, and a trivial first unit test If BUILD_TESTS=ON: - Adds a 'test' target for ninja - Adds a library/MiscUtils.test unit test executable --- .gitignore | 2 ++ CMakeLists.txt | 10 +++++++- library/CMakeLists.txt | 10 ++++++++ library/MiscUtils.test.cpp | 47 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 library/MiscUtils.test.cpp diff --git a/.gitignore b/.gitignore index 3867b9dbf..06f2b42e0 100644 --- a/.gitignore +++ b/.gitignore @@ -24,6 +24,8 @@ build/Makefile build/CMakeCache.txt build/cmake_install.cmake build/CMakeFiles +build/CTestTestfile.cmake +build/DartConfiguration.tcl build/data build/docs build/lua diff --git a/CMakeLists.txt b/CMakeLists.txt index 054430ba6..f8fbf3b39 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -497,14 +497,22 @@ if(BUILD_DOCS) install(FILES "README.html" DESTINATION "${DFHACK_DATA_DESTINATION}") endif() -option(BUILD_TESTS "Include tests (currently just installs Lua tests into the scripts folder)" OFF) +option(BUILD_TESTS "Build 'test' target, and install integration tests in hack/scripts/test" OFF) if(BUILD_TESTS) + include(CTest) + # Install Lua tests into the scripts folder if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") endif() install(DIRECTORY ${dfhack_SOURCE_DIR}/test DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) +else() + add_custom_target(test + COMMENT "Nothing to do: CMake option BUILD_TESTS is OFF" + # Portable NOOP; need to put something here or the comment isn't displayed + COMMAND cd + ) endif() # Packaging with CPack! diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 026558ab0..767a26c8a 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -82,6 +82,16 @@ set(MAIN_SOURCES RemoteTools.cpp ) +if(BUILD_TESTS) + # TODO Make a function or macro for this + add_executable( + MiscUtils.test + MiscUtils.test.cpp + MiscUtils.cpp) + add_test(NAME MiscUtils.test + COMMAND MiscUtils.test) +endif() + if(WIN32) set(CONSOLE_SOURCES Console-windows.cpp) else() diff --git a/library/MiscUtils.test.cpp b/library/MiscUtils.test.cpp new file mode 100644 index 000000000..f3360b121 --- /dev/null +++ b/library/MiscUtils.test.cpp @@ -0,0 +1,47 @@ +#include "Internal.h" +#include "MiscUtils.h" + +#include +#include +using namespace std; + +int compare_result(const vector &expect, const vector &result) +{ + if (result == expect) + { + cout << "ok\n"; + return 0; + } + else { + cout << "not ok\n"; + auto e = expect.begin(); + auto r = result.begin(); + cout << "# " << setw(20) << left << "Expected" << " " << left << "Got\n"; + while (e < expect.end() || r < result.end()) + { + cout + << "# " + << setw(20) << left << ":" + (e < expect.end() ? *e++ : "") + ":" + << " " + << setw(20) << left << ":" + (r < result.end() ? *r++ : "") + ":" + << "\n"; + } + return 1; + } +} + +int main() +{ + int fails = 0; +#define TEST_WORDWRAP(label, expect, args) \ + { \ + vector result; \ + cout << label << "... "; \ + word_wrap args; \ + fails += compare_result(expect, result); \ + } + + TEST_WORDWRAP("Short line, no wrap", vector({"12345"}), (&result, "12345", 15)); + + return fails == 0 ? 0 : 1; +} From 6daa13ad518a98bb32580f900011effd6d89dd64 Mon Sep 17 00:00:00 2001 From: Tim Siegel Date: Thu, 21 Apr 2022 10:42:19 -0400 Subject: [PATCH 02/25] unit testing: Link test executables against libdfhack Note: Hard-coded "SDL" here is wrong, but requires some refactoring in top-level CMakeLists.txt to fix. --- library/CMakeLists.txt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 767a26c8a..881bdccc6 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -84,12 +84,10 @@ set(MAIN_SOURCES if(BUILD_TESTS) # TODO Make a function or macro for this - add_executable( - MiscUtils.test - MiscUtils.test.cpp - MiscUtils.cpp) - add_test(NAME MiscUtils.test - COMMAND MiscUtils.test) + add_executable(MiscUtils.test MiscUtils.test.cpp) + # FIXME Hard-coded "SDL" works on Linux, but probably isn't portable + target_link_libraries(MiscUtils.test dfhack SDL) + add_test(NAME MiscUtils.test COMMAND MiscUtils.test) endif() if(WIN32) From b56f57c074cc2dc6a5bbb4a104ec17bf11111f44 Mon Sep 17 00:00:00 2001 From: Tim Siegel Date: Thu, 21 Apr 2022 18:33:53 -0400 Subject: [PATCH 03/25] cmake: Add SDL dep for Linux dfhack; deprecate BUILD_TESTS On Linux, libdfhack.so depends on libSDL.so, but that was not marked inside CMake. As it's only used via LD_PRELOAD, there was no problem. But when linking unit tests against it, this becomes necessary. It may be wise to add a find_package(SDL) to provide the user with more control, but just a hard-coded "SDL" should work for most installs. The CTest module creates a BUILD_TESTING option, which clashes (thematically, not in code) with the existing BUILD_TESTS option. Resolve it thus: - Deprecate BUILD_TESTS; it still works, but is marked as an advanced option so it doesn't show in the CMake UI by default. - Add a new BUILD_TEST_SCRIPTS that does what BUILD_TESTS used to do, but is a "dependent" option so it goes away if BUILD_TESTING=OFF. The up-shot is that, by default, the C++ unit tests will be built (BUILD_TESTING=ON) and the Lua integration tests are not installed (BUILD_TEST_SCRIPTS=OFF). --- CMakeLists.txt | 32 +++++++++++++++++++++++--------- library/CMakeLists.txt | 12 +++++++++--- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f8fbf3b39..48784ecb1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -497,16 +497,30 @@ if(BUILD_DOCS) install(FILES "README.html" DESTINATION "${DFHACK_DATA_DESTINATION}") endif() -option(BUILD_TESTS "Build 'test' target, and install integration tests in hack/scripts/test" OFF) -if(BUILD_TESTS) - include(CTest) - # Install Lua tests into the scripts folder - if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") - message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") +# Testing with CTest +include(CTest) + +include(CMakeDependentOption) +cmake_dependent_option( + BUILD_TEST_SCRIPTS "Install integration tests in hack/scripts/test" OFF + "BUILD_TESTING" OFF) +mark_as_advanced(FORCE BUILD_TESTS) + +# Handle deprecated BUILD_TESTS option +option(BUILD_TESTS "Deprecated option; please use BUILD_TEST_SCRIPTS=ON" OFF) +if(BUILD_TESTING AND BUILD_TESTS) + set(BUILD_TEST_SCRIPTS ON FORCE) +endif() + +if(BUILD_TESTING) + if(BUILD_TEST_SCRIPTS) + if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") + message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") + endif() + install(DIRECTORY ${dfhack_SOURCE_DIR}/test + DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) + install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) endif() - install(DIRECTORY ${dfhack_SOURCE_DIR}/test - DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) - install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) else() add_custom_target(test COMMENT "Nothing to do: CMake option BUILD_TESTS is OFF" diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 881bdccc6..b134b30df 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -82,12 +82,16 @@ set(MAIN_SOURCES RemoteTools.cpp ) -if(BUILD_TESTS) +if(BUILD_TESTING) # TODO Make a function or macro for this add_executable(MiscUtils.test MiscUtils.test.cpp) - # FIXME Hard-coded "SDL" works on Linux, but probably isn't portable - target_link_libraries(MiscUtils.test dfhack SDL) + target_link_libraries(MiscUtils.test dfhack) add_test(NAME MiscUtils.test COMMAND MiscUtils.test) + + # How to get `test` to ensure everything is up to date before running + # tests? This add_dependencies() fails with: + # Cannot add target-level dependencies to non-existent target "test". + #add_dependencies(test MiscUtils.test) endif() if(WIN32) @@ -421,6 +425,8 @@ if(APPLE) target_link_libraries(dfhack ncurses) set_target_properties(dfhack PROPERTIES VERSION 1.0.0) set_target_properties(dfhack PROPERTIES SOVERSION 1.0.0) +elseif(UNIX) + target_link_libraries(dfhack SDL) endif() target_link_libraries(dfhack protobuf-lite clsocket lua jsoncpp_lib_static dfhack-version ${PROJECT_LIBS}) From c5be87e381aeaadd8dfe0eadd728fafb2ec86bdb Mon Sep 17 00:00:00 2001 From: Tim Siegel Date: Wed, 20 Apr 2022 20:28:13 -0400 Subject: [PATCH 04/25] unit tests: Add CTest support, and a trivial first unit test If BUILD_TESTS=ON: - Adds a 'test' target for ninja - Adds a library/MiscUtils.test unit test executable --- .gitignore | 2 ++ CMakeLists.txt | 10 +++++++- library/CMakeLists.txt | 10 ++++++++ library/MiscUtils.test.cpp | 47 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 library/MiscUtils.test.cpp diff --git a/.gitignore b/.gitignore index e91dcab3b..9b78fa31f 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,8 @@ build/Makefile build/CMakeCache.txt build/cmake_install.cmake build/CMakeFiles +build/CTestTestfile.cmake +build/DartConfiguration.tcl build/data build/docs build/lua diff --git a/CMakeLists.txt b/CMakeLists.txt index c03037973..e36385c43 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -527,14 +527,22 @@ if(BUILD_DOCS) install(FILES "README.html" DESTINATION "${DFHACK_DATA_DESTINATION}") endif() -option(BUILD_TESTS "Include tests (currently just installs Lua tests into the scripts folder)" OFF) +option(BUILD_TESTS "Build 'test' target, and install integration tests in hack/scripts/test" OFF) if(BUILD_TESTS) + include(CTest) + # Install Lua tests into the scripts folder if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") endif() install(DIRECTORY ${dfhack_SOURCE_DIR}/test DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) +else() + add_custom_target(test + COMMENT "Nothing to do: CMake option BUILD_TESTS is OFF" + # Portable NOOP; need to put something here or the comment isn't displayed + COMMAND cd + ) endif() # Packaging with CPack! diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index f969ade92..640d17978 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -82,6 +82,16 @@ set(MAIN_SOURCES RemoteTools.cpp ) +if(BUILD_TESTS) + # TODO Make a function or macro for this + add_executable( + MiscUtils.test + MiscUtils.test.cpp + MiscUtils.cpp) + add_test(NAME MiscUtils.test + COMMAND MiscUtils.test) +endif() + if(WIN32) set(CONSOLE_SOURCES Console-windows.cpp) else() diff --git a/library/MiscUtils.test.cpp b/library/MiscUtils.test.cpp new file mode 100644 index 000000000..f3360b121 --- /dev/null +++ b/library/MiscUtils.test.cpp @@ -0,0 +1,47 @@ +#include "Internal.h" +#include "MiscUtils.h" + +#include +#include +using namespace std; + +int compare_result(const vector &expect, const vector &result) +{ + if (result == expect) + { + cout << "ok\n"; + return 0; + } + else { + cout << "not ok\n"; + auto e = expect.begin(); + auto r = result.begin(); + cout << "# " << setw(20) << left << "Expected" << " " << left << "Got\n"; + while (e < expect.end() || r < result.end()) + { + cout + << "# " + << setw(20) << left << ":" + (e < expect.end() ? *e++ : "") + ":" + << " " + << setw(20) << left << ":" + (r < result.end() ? *r++ : "") + ":" + << "\n"; + } + return 1; + } +} + +int main() +{ + int fails = 0; +#define TEST_WORDWRAP(label, expect, args) \ + { \ + vector result; \ + cout << label << "... "; \ + word_wrap args; \ + fails += compare_result(expect, result); \ + } + + TEST_WORDWRAP("Short line, no wrap", vector({"12345"}), (&result, "12345", 15)); + + return fails == 0 ? 0 : 1; +} From face558dd060161de5e8a9f6301e4876d572a087 Mon Sep 17 00:00:00 2001 From: Tim Siegel Date: Thu, 21 Apr 2022 10:42:19 -0400 Subject: [PATCH 05/25] unit testing: Link test executables against libdfhack Note: Hard-coded "SDL" here is wrong, but requires some refactoring in top-level CMakeLists.txt to fix. --- library/CMakeLists.txt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 640d17978..dd9582b5b 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -84,12 +84,10 @@ set(MAIN_SOURCES if(BUILD_TESTS) # TODO Make a function or macro for this - add_executable( - MiscUtils.test - MiscUtils.test.cpp - MiscUtils.cpp) - add_test(NAME MiscUtils.test - COMMAND MiscUtils.test) + add_executable(MiscUtils.test MiscUtils.test.cpp) + # FIXME Hard-coded "SDL" works on Linux, but probably isn't portable + target_link_libraries(MiscUtils.test dfhack SDL) + add_test(NAME MiscUtils.test COMMAND MiscUtils.test) endif() if(WIN32) From 8e18d610f5f38df97dad4247b608c4abe320c7b8 Mon Sep 17 00:00:00 2001 From: Tim Siegel Date: Thu, 21 Apr 2022 18:33:53 -0400 Subject: [PATCH 06/25] cmake: Add SDL dep for Linux dfhack; deprecate BUILD_TESTS On Linux, libdfhack.so depends on libSDL.so, but that was not marked inside CMake. As it's only used via LD_PRELOAD, there was no problem. But when linking unit tests against it, this becomes necessary. It may be wise to add a find_package(SDL) to provide the user with more control, but just a hard-coded "SDL" should work for most installs. The CTest module creates a BUILD_TESTING option, which clashes (thematically, not in code) with the existing BUILD_TESTS option. Resolve it thus: - Deprecate BUILD_TESTS; it still works, but is marked as an advanced option so it doesn't show in the CMake UI by default. - Add a new BUILD_TEST_SCRIPTS that does what BUILD_TESTS used to do, but is a "dependent" option so it goes away if BUILD_TESTING=OFF. The up-shot is that, by default, the C++ unit tests will be built (BUILD_TESTING=ON) and the Lua integration tests are not installed (BUILD_TEST_SCRIPTS=OFF). --- CMakeLists.txt | 32 +++++++++++++++++++++++--------- library/CMakeLists.txt | 12 +++++++++--- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e36385c43..caf53c2a0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -527,16 +527,30 @@ if(BUILD_DOCS) install(FILES "README.html" DESTINATION "${DFHACK_DATA_DESTINATION}") endif() -option(BUILD_TESTS "Build 'test' target, and install integration tests in hack/scripts/test" OFF) -if(BUILD_TESTS) - include(CTest) - # Install Lua tests into the scripts folder - if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") - message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") +# Testing with CTest +include(CTest) + +include(CMakeDependentOption) +cmake_dependent_option( + BUILD_TEST_SCRIPTS "Install integration tests in hack/scripts/test" OFF + "BUILD_TESTING" OFF) +mark_as_advanced(FORCE BUILD_TESTS) + +# Handle deprecated BUILD_TESTS option +option(BUILD_TESTS "Deprecated option; please use BUILD_TEST_SCRIPTS=ON" OFF) +if(BUILD_TESTING AND BUILD_TESTS) + set(BUILD_TEST_SCRIPTS ON FORCE) +endif() + +if(BUILD_TESTING) + if(BUILD_TEST_SCRIPTS) + if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") + message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") + endif() + install(DIRECTORY ${dfhack_SOURCE_DIR}/test + DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) + install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) endif() - install(DIRECTORY ${dfhack_SOURCE_DIR}/test - DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) - install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) else() add_custom_target(test COMMENT "Nothing to do: CMake option BUILD_TESTS is OFF" diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index dd9582b5b..a62eb2fe0 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -82,12 +82,16 @@ set(MAIN_SOURCES RemoteTools.cpp ) -if(BUILD_TESTS) +if(BUILD_TESTING) # TODO Make a function or macro for this add_executable(MiscUtils.test MiscUtils.test.cpp) - # FIXME Hard-coded "SDL" works on Linux, but probably isn't portable - target_link_libraries(MiscUtils.test dfhack SDL) + target_link_libraries(MiscUtils.test dfhack) add_test(NAME MiscUtils.test COMMAND MiscUtils.test) + + # How to get `test` to ensure everything is up to date before running + # tests? This add_dependencies() fails with: + # Cannot add target-level dependencies to non-existent target "test". + #add_dependencies(test MiscUtils.test) endif() if(WIN32) @@ -400,6 +404,8 @@ if(APPLE) target_link_libraries(dfhack ncurses) set_target_properties(dfhack PROPERTIES VERSION 1.0.0) set_target_properties(dfhack PROPERTIES SOVERSION 1.0.0) +elseif(UNIX) + target_link_libraries(dfhack SDL) endif() target_link_libraries(dfhack protobuf-lite clsocket lua jsoncpp_static dfhack-version ${PROJECT_LIBS}) From 268719ed1f4d9a1fdf1731853bfd787639805e06 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 14:48:00 -0800 Subject: [PATCH 07/25] Integrates googletest --- .gitmodules | 3 ++ CMakeLists.txt | 65 +++++++++++++++++++------------------- depends/CMakeLists.txt | 3 ++ depends/googletest | 1 + library/CMakeLists.txt | 14 +++++--- library/MiscUtils.test.cpp | 57 +++++++++++---------------------- library/test.cpp | 6 ++++ 7 files changed, 74 insertions(+), 75 deletions(-) create mode 160000 depends/googletest create mode 100644 library/test.cpp diff --git a/.gitmodules b/.gitmodules index 9c5ac2d51..ee26c1926 100644 --- a/.gitmodules +++ b/.gitmodules @@ -28,3 +28,6 @@ [submodule "depends/luacov"] path = depends/luacov url = ../../DFHack/luacov.git +[submodule "depends/googletest"] + path = depends/googletest + url = https://github.com/google/googletest.git diff --git a/CMakeLists.txt b/CMakeLists.txt index caf53c2a0..40dc64b8f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -426,6 +426,39 @@ if(NOT GIT_FOUND) message(SEND_ERROR "could not find git") endif() +# Testing with CTest +include(CTest) + +include(CMakeDependentOption) +cmake_dependent_option( + BUILD_TEST_SCRIPTS "Install integration tests in hack/scripts/test" OFF + "BUILD_TESTING" OFF) +mark_as_advanced(FORCE BUILD_TESTS) + +# Handle deprecated BUILD_TESTS option +option(BUILD_TESTS "Deprecated option; please use BUILD_TEST_SCRIPTS=ON" OFF) +if(BUILD_TESTING AND BUILD_TESTS) + set(BUILD_TEST_SCRIPTS ON FORCE) +endif() + +if(BUILD_TESTING) + include_directories(depends/googletest/googletest/include) + if(BUILD_TEST_SCRIPTS) + if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") + message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") + endif() + install(DIRECTORY ${dfhack_SOURCE_DIR}/test + DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) + install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) + endif() +else() + add_custom_target(test + COMMENT "Nothing to do: CMake option BUILD_TESTS is OFF" + # Portable NOOP; need to put something here or the comment isn't displayed + COMMAND cd + ) +endif() + # build the lib itself if(BUILD_LIBRARY) add_subdirectory(library) @@ -527,38 +560,6 @@ if(BUILD_DOCS) install(FILES "README.html" DESTINATION "${DFHACK_DATA_DESTINATION}") endif() -# Testing with CTest -include(CTest) - -include(CMakeDependentOption) -cmake_dependent_option( - BUILD_TEST_SCRIPTS "Install integration tests in hack/scripts/test" OFF - "BUILD_TESTING" OFF) -mark_as_advanced(FORCE BUILD_TESTS) - -# Handle deprecated BUILD_TESTS option -option(BUILD_TESTS "Deprecated option; please use BUILD_TEST_SCRIPTS=ON" OFF) -if(BUILD_TESTING AND BUILD_TESTS) - set(BUILD_TEST_SCRIPTS ON FORCE) -endif() - -if(BUILD_TESTING) - if(BUILD_TEST_SCRIPTS) - if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") - message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") - endif() - install(DIRECTORY ${dfhack_SOURCE_DIR}/test - DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) - install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) - endif() -else() - add_custom_target(test - COMMENT "Nothing to do: CMake option BUILD_TESTS is OFF" - # Portable NOOP; need to put something here or the comment isn't displayed - COMMAND cd - ) -endif() - # Packaging with CPack! set(DFHACK_PACKAGE_SUFFIX "") if(UNIX) diff --git a/depends/CMakeLists.txt b/depends/CMakeLists.txt index 405a9555e..1d43fbf86 100644 --- a/depends/CMakeLists.txt +++ b/depends/CMakeLists.txt @@ -3,6 +3,9 @@ add_subdirectory(lodepng) add_subdirectory(lua) add_subdirectory(md5) add_subdirectory(protobuf) +if(BUILD_TESTING) + add_subdirectory(googletest) +endif() # Don't build tinyxml if it's being externally linked against. if(NOT TinyXML_FOUND) diff --git a/depends/googletest b/depends/googletest new file mode 160000 index 000000000..15460959c --- /dev/null +++ b/depends/googletest @@ -0,0 +1 @@ +Subproject commit 15460959cbbfa20e66ef0b5ab497367e47fc0a04 diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index a62eb2fe0..3c85d5861 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -82,11 +82,17 @@ set(MAIN_SOURCES RemoteTools.cpp ) +macro(dfhack_test name files) + message("dfhack_test files: ${files}") + add_executable(${name} test.cpp ${files}) + target_link_libraries(${name} dfhack gtest) + add_test(NAME ${name} COMMAND ${name}) +endmacro() + if(BUILD_TESTING) - # TODO Make a function or macro for this - add_executable(MiscUtils.test MiscUtils.test.cpp) - target_link_libraries(MiscUtils.test dfhack) - add_test(NAME MiscUtils.test COMMAND MiscUtils.test) + file(GLOB_RECURSE TEST_SOURCES LIST_DIRECTORIES false *.test.cpp) + dfhack_test(test-all ${TEST_SOURCES}) + dfhack_test(test-MiscUtils MiscUtils.test.cpp) # How to get `test` to ensure everything is up to date before running # tests? This add_dependencies() fails with: diff --git a/library/MiscUtils.test.cpp b/library/MiscUtils.test.cpp index f3360b121..e0cabe234 100644 --- a/library/MiscUtils.test.cpp +++ b/library/MiscUtils.test.cpp @@ -1,47 +1,26 @@ -#include "Internal.h" -#include "MiscUtils.h" +#include "MiscUtils.h" +#include #include #include -using namespace std; -int compare_result(const vector &expect, const vector &result) -{ - if (result == expect) - { - cout << "ok\n"; - return 0; - } - else { - cout << "not ok\n"; - auto e = expect.begin(); - auto r = result.begin(); - cout << "# " << setw(20) << left << "Expected" << " " << left << "Got\n"; - while (e < expect.end() || r < result.end()) - { - cout - << "# " - << setw(20) << left << ":" + (e < expect.end() ? *e++ : "") + ":" - << " " - << setw(20) << left << ":" + (r < result.end() ? *r++ : "") + ":" - << "\n"; - } - return 1; - } -} +TEST(MiscUtils, wordwrap) { + std::vector result; -int main() -{ - int fails = 0; -#define TEST_WORDWRAP(label, expect, args) \ - { \ - vector result; \ - cout << label << "... "; \ - word_wrap args; \ - fails += compare_result(expect, result); \ - } + std::cout << "MiscUtils.wordwrap: 0 wrap" << "... "; + word_wrap(&result, "123", 3); + ASSERT_EQ(result.size(), 1); + std::cout << "ok\n"; - TEST_WORDWRAP("Short line, no wrap", vector({"12345"}), (&result, "12345", 15)); + result.clear(); + std::cout << "MiscUtils.wordwrap: 1 wrap" << "... "; + word_wrap(&result, "12345", 3); + ASSERT_EQ(result.size(), 2); + std::cout << "ok\n"; - return fails == 0 ? 0 : 1; + result.clear(); + std::cout << "MiscUtils.wordwrap: 2 wrap" << "... "; + word_wrap(&result, "1234567", 3); + ASSERT_EQ(result.size(), 3); + std::cout << "ok\n"; } diff --git a/library/test.cpp b/library/test.cpp new file mode 100644 index 000000000..2dc3787b7 --- /dev/null +++ b/library/test.cpp @@ -0,0 +1,6 @@ +#include + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} \ No newline at end of file From e1f9a95d4f5b9b5492494359ed0911c73dd15aa3 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 14:54:18 -0800 Subject: [PATCH 08/25] Rolls back googletest to release-1.8.1 --- depends/googletest | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/depends/googletest b/depends/googletest index 15460959c..2fe3bd994 160000 --- a/depends/googletest +++ b/depends/googletest @@ -1 +1 @@ -Subproject commit 15460959cbbfa20e66ef0b5ab497367e47fc0a04 +Subproject commit 2fe3bd994b3189899d93f1d5a881e725e046fdc2 From 32b030e348be03aa1576cc2f64446e328b75f5cd Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 15:54:08 -0800 Subject: [PATCH 09/25] Adds -Wno-maybe-uninitialized to gtest target --- depends/CMakeLists.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/depends/CMakeLists.txt b/depends/CMakeLists.txt index 1d43fbf86..b7c47d6ca 100644 --- a/depends/CMakeLists.txt +++ b/depends/CMakeLists.txt @@ -4,7 +4,8 @@ add_subdirectory(lua) add_subdirectory(md5) add_subdirectory(protobuf) if(BUILD_TESTING) - add_subdirectory(googletest) + add_subdirectory(googletest EXCLUDE_FROM_ALL) + set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized") endif() # Don't build tinyxml if it's being externally linked against. From 9fdb2f7e475f1ff2d34e98360a7691e4aa564f2a Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 16:01:16 -0800 Subject: [PATCH 10/25] Adds -Wno-sign-compare to gtest target --- depends/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/depends/CMakeLists.txt b/depends/CMakeLists.txt index b7c47d6ca..13a8cc466 100644 --- a/depends/CMakeLists.txt +++ b/depends/CMakeLists.txt @@ -5,7 +5,7 @@ add_subdirectory(md5) add_subdirectory(protobuf) if(BUILD_TESTING) add_subdirectory(googletest EXCLUDE_FROM_ALL) - set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized") + set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized -Wno-sign-compare") endif() # Don't build tinyxml if it's being externally linked against. From de91fa7f28813b8ada2ebab8d12a155f6510402a Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 16:12:51 -0800 Subject: [PATCH 11/25] Adds -Wno-sign-compare to test targets --- library/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 3c85d5861..f8a60cfc0 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -87,6 +87,7 @@ macro(dfhack_test name files) add_executable(${name} test.cpp ${files}) target_link_libraries(${name} dfhack gtest) add_test(NAME ${name} COMMAND ${name}) + set_target_properties(${name} PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") endmacro() if(BUILD_TESTING) From 3e2d0f2ec2a762fd3794d60847ded42ab5ba5b21 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 16:41:06 -0800 Subject: [PATCH 12/25] Updates GithubActions yml file to use new BUILD_TESTING variable --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 83f3490da..d9af9a20e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -83,7 +83,7 @@ jobs: -B build-ci \ -G Ninja \ -DDFHACK_BUILD_ARCH=64 \ - -DBUILD_TESTS:BOOL=ON \ + -DBUILD_TESTING:BOOL=ON \ -DBUILD_DEV_PLUGINS:BOOL=${{ matrix.plugins == 'all' }} \ -DBUILD_SIZECHECK:BOOL=${{ matrix.plugins == 'all' }} \ -DBUILD_SKELETON:BOOL=${{ matrix.plugins == 'all' }} \ From 79551f7ef00efd9506f94ed1390dba476a9fc091 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 17:17:29 -0800 Subject: [PATCH 13/25] Updates CMake TESTING vars --- CMakeLists.txt | 48 ++++++++++++++++++++++++------------------ data/CMakeLists.txt | 2 +- depends/CMakeLists.txt | 2 +- library/CMakeLists.txt | 2 +- 4 files changed, 30 insertions(+), 24 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 40dc64b8f..c78c45bec 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -414,6 +414,15 @@ else() set(DFHACK_TINYXML "dfhack-tinyxml") endif() +if(BUILD_TESTING) + message("BUILD TESTS: Core, Scripts") + set(BUILD_SCRIPT_TESTS ON FORCE) + set(BUILD_CORE_TESTS ON FORCE) +endif() + +if(BUILD_CORE_TESTS) + include_directories(depends/googletest/googletest/include) +endif() include_directories(depends/lodepng) include_directories(depends/tthread) include_directories(${ZLIB_INCLUDE_DIRS}) @@ -421,44 +430,41 @@ include_directories(depends/clsocket/src) include_directories(depends/xlsxio/include) add_subdirectory(depends) -find_package(Git REQUIRED) -if(NOT GIT_FOUND) - message(SEND_ERROR "could not find git") -endif() # Testing with CTest -include(CTest) +if(BUILD_TESTING OR BUILD_CORE_TESTS) + message("Including CTest") + include(CTest) +endif() include(CMakeDependentOption) cmake_dependent_option( - BUILD_TEST_SCRIPTS "Install integration tests in hack/scripts/test" OFF + BUILD_SCRIPT_TESTS "Install integration tests in hack/scripts/test" OFF "BUILD_TESTING" OFF) mark_as_advanced(FORCE BUILD_TESTS) - # Handle deprecated BUILD_TESTS option -option(BUILD_TESTS "Deprecated option; please use BUILD_TEST_SCRIPTS=ON" OFF) -if(BUILD_TESTING AND BUILD_TESTS) - set(BUILD_TEST_SCRIPTS ON FORCE) -endif() +option(BUILD_TESTS "Deprecated option; please use BUILD_SCRIPT_TESTS=ON" OFF) -if(BUILD_TESTING) - include_directories(depends/googletest/googletest/include) - if(BUILD_TEST_SCRIPTS) - if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") - message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") - endif() - install(DIRECTORY ${dfhack_SOURCE_DIR}/test - DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) - install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) +if(BUILD_TESTING OR BUILD_SCRIPT_TESTS) + if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") + message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") endif() + install(DIRECTORY ${dfhack_SOURCE_DIR}/test + DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) + install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) else() add_custom_target(test - COMMENT "Nothing to do: CMake option BUILD_TESTS is OFF" + COMMENT "Nothing to do: CMake option BUILD_TESTING is OFF" # Portable NOOP; need to put something here or the comment isn't displayed COMMAND cd ) endif() +find_package(Git REQUIRED) +if(NOT GIT_FOUND) + message(SEND_ERROR "could not find git") +endif() + # build the lib itself if(BUILD_LIBRARY) add_subdirectory(library) diff --git a/data/CMakeLists.txt b/data/CMakeLists.txt index 412ffa347..8cf6bb2ca 100644 --- a/data/CMakeLists.txt +++ b/data/CMakeLists.txt @@ -21,7 +21,7 @@ install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/blueprints/ FILES_MATCHING PATTERN "*" PATTERN blueprints/library/test EXCLUDE) -if(BUILD_TESTS) +if(BUILD_SCRIPT_TESTS) install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/blueprints/library/test/ DESTINATION blueprints/library/test ) diff --git a/depends/CMakeLists.txt b/depends/CMakeLists.txt index 13a8cc466..f5e77ad8a 100644 --- a/depends/CMakeLists.txt +++ b/depends/CMakeLists.txt @@ -3,7 +3,7 @@ add_subdirectory(lodepng) add_subdirectory(lua) add_subdirectory(md5) add_subdirectory(protobuf) -if(BUILD_TESTING) +if(BUILD_CORE_TESTS) add_subdirectory(googletest EXCLUDE_FROM_ALL) set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized -Wno-sign-compare") endif() diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index f8a60cfc0..035fcda73 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -90,7 +90,7 @@ macro(dfhack_test name files) set_target_properties(${name} PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") endmacro() -if(BUILD_TESTING) +if(BUILD_CORE_TESTS) file(GLOB_RECURSE TEST_SOURCES LIST_DIRECTORIES false *.test.cpp) dfhack_test(test-all ${TEST_SOURCES}) dfhack_test(test-MiscUtils MiscUtils.test.cpp) From 69d988332c129fc5719a95160924dffb5e61ab9b Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 17:36:45 -0800 Subject: [PATCH 14/25] Updates build.yml to call test-all --- .github/workflows/build.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d9af9a20e..984a7622e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -100,12 +100,15 @@ jobs: id: run_tests run: | export TERM=dumb - status=0 - script -qe -c "python ci/run-tests.py --headless --keep-status \"$DF_FOLDER\"" || status=$((status + 1)) - python ci/check-rpc.py "$DF_FOLDER/dfhack-rpc.txt" || status=$((status + 2)) - mkdir -p artifacts - cp "$DF_FOLDER"/test*.json "$DF_FOLDER"/*.log artifacts || status=$((status + 4)) - exit $status + if build-ci/library/test-all; then + status=0 + script -qe -c "python ci/run-tests.py --headless --keep-status \"$DF_FOLDER\"" || status=$((status + 1)) + python ci/check-rpc.py "$DF_FOLDER/dfhack-rpc.txt" || status=$((status + 2)) + mkdir -p artifacts + cp "$DF_FOLDER"/test*.json "$DF_FOLDER"/*.log artifacts || status=$((status + 4)) + exit $status + fi + exit 1 - name: Upload test artifacts uses: actions/upload-artifact@v1 if: (success() || failure()) && steps.run_tests.outcome != 'skipped' From 25f87306b427c39e14591e19754d56633e940b92 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 18:46:46 -0800 Subject: [PATCH 15/25] Removes separated unit test executables --- CMakeLists.txt | 19 +++++++++++++------ library/CMakeLists.txt | 17 +---------------- library/tests/CMakeLists.txt | 7 +++++++ library/{ => tests}/MiscUtils.test.cpp | 7 ------- library/{ => tests}/test.cpp | 2 +- 5 files changed, 22 insertions(+), 30 deletions(-) create mode 100644 library/tests/CMakeLists.txt rename library/{ => tests}/MiscUtils.test.cpp (59%) rename library/{ => tests}/test.cpp (98%) diff --git a/CMakeLists.txt b/CMakeLists.txt index c78c45bec..29103d495 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -394,11 +394,6 @@ else() endif() endif() -find_package(ZLIB REQUIRED) -include_directories(depends/protobuf) -include_directories(depends/lua/include) -include_directories(depends/md5) - # Support linking against external tinyxml # If we find an external tinyxml, set the DFHACK_TINYXML variable to "tinyxml" # Otherwise, set it to "dfhack-tinyxml" @@ -420,9 +415,15 @@ if(BUILD_TESTING) set(BUILD_CORE_TESTS ON FORCE) endif() +find_package(ZLIB REQUIRED) + if(BUILD_CORE_TESTS) include_directories(depends/googletest/googletest/include) endif() + +include_directories(depends/protobuf) +include_directories(depends/lua/include) +include_directories(depends/md5) include_directories(depends/lodepng) include_directories(depends/tthread) include_directories(${ZLIB_INCLUDE_DIRS}) @@ -433,7 +434,13 @@ add_subdirectory(depends) # Testing with CTest if(BUILD_TESTING OR BUILD_CORE_TESTS) - message("Including CTest") + macro(dfhack_test name files) + message("dfhack_test(${name}, ${files})") + add_executable(${name} ${files}) + target_link_libraries(${name} dfhack gtest) + add_test(NAME ${name} COMMAND ${name}) + set_target_properties(${name} PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") + endmacro() include(CTest) endif() diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 035fcda73..5e7488027 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -82,23 +82,8 @@ set(MAIN_SOURCES RemoteTools.cpp ) -macro(dfhack_test name files) - message("dfhack_test files: ${files}") - add_executable(${name} test.cpp ${files}) - target_link_libraries(${name} dfhack gtest) - add_test(NAME ${name} COMMAND ${name}) - set_target_properties(${name} PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") -endmacro() - if(BUILD_CORE_TESTS) - file(GLOB_RECURSE TEST_SOURCES LIST_DIRECTORIES false *.test.cpp) - dfhack_test(test-all ${TEST_SOURCES}) - dfhack_test(test-MiscUtils MiscUtils.test.cpp) - - # How to get `test` to ensure everything is up to date before running - # tests? This add_dependencies() fails with: - # Cannot add target-level dependencies to non-existent target "test". - #add_dependencies(test MiscUtils.test) + add_subdirectory(tests) endif() if(WIN32) diff --git a/library/tests/CMakeLists.txt b/library/tests/CMakeLists.txt new file mode 100644 index 000000000..3e5a2b3ca --- /dev/null +++ b/library/tests/CMakeLists.txt @@ -0,0 +1,7 @@ +file(GLOB_RECURSE TEST_SOURCES LIST_DIRECTORIES false *test.cpp) +dfhack_test(test-library "${TEST_SOURCES}") + +# How to get `test` to ensure everything is up to date before running +# tests? This add_dependencies() fails with: +# Cannot add target-level dependencies to non-existent target "test". +#add_dependencies(test MiscUtils.test) diff --git a/library/MiscUtils.test.cpp b/library/tests/MiscUtils.test.cpp similarity index 59% rename from library/MiscUtils.test.cpp rename to library/tests/MiscUtils.test.cpp index e0cabe234..033b226a7 100644 --- a/library/MiscUtils.test.cpp +++ b/library/tests/MiscUtils.test.cpp @@ -1,26 +1,19 @@ #include "MiscUtils.h" #include -#include #include TEST(MiscUtils, wordwrap) { std::vector result; - std::cout << "MiscUtils.wordwrap: 0 wrap" << "... "; word_wrap(&result, "123", 3); ASSERT_EQ(result.size(), 1); - std::cout << "ok\n"; result.clear(); - std::cout << "MiscUtils.wordwrap: 1 wrap" << "... "; word_wrap(&result, "12345", 3); ASSERT_EQ(result.size(), 2); - std::cout << "ok\n"; result.clear(); - std::cout << "MiscUtils.wordwrap: 2 wrap" << "... "; word_wrap(&result, "1234567", 3); ASSERT_EQ(result.size(), 3); - std::cout << "ok\n"; } diff --git a/library/test.cpp b/library/tests/test.cpp similarity index 98% rename from library/test.cpp rename to library/tests/test.cpp index 2dc3787b7..76f841f1b 100644 --- a/library/test.cpp +++ b/library/tests/test.cpp @@ -3,4 +3,4 @@ int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); -} \ No newline at end of file +} From a716b2796eb4598451dba3e0ea6ce3151dea3e59 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 18:49:52 -0800 Subject: [PATCH 16/25] Updates test binary name in build.yml --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 984a7622e..53bfca76e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -100,7 +100,7 @@ jobs: id: run_tests run: | export TERM=dumb - if build-ci/library/test-all; then + if build-ci/library/tests/test-library; then status=0 script -qe -c "python ci/run-tests.py --headless --keep-status \"$DF_FOLDER\"" || status=$((status + 1)) python ci/check-rpc.py "$DF_FOLDER/dfhack-rpc.txt" || status=$((status + 2)) From cd7fe8a213b20ac8a05ce631fadd078a16f90a1d Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 18:54:16 -0800 Subject: [PATCH 17/25] Update .gitmodules Co-authored-by: Alan --- .gitmodules | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitmodules b/.gitmodules index ee26c1926..c349de288 100644 --- a/.gitmodules +++ b/.gitmodules @@ -30,4 +30,4 @@ url = ../../DFHack/luacov.git [submodule "depends/googletest"] path = depends/googletest - url = https://github.com/google/googletest.git + url = ../../google/googletest.git From d03f93c0d7b57d17ee5929a052a5255d3acaf95b Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 11 Nov 2022 19:01:10 -0800 Subject: [PATCH 18/25] Adds a second test stage --- .github/workflows/build.yml | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 53bfca76e..1779e486e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -96,19 +96,20 @@ jobs: run: | ninja -C build-ci install ccache --show-stats - - name: Run tests - id: run_tests + - name: Run unit tests + id: run_tests1 + run: | + exit build-ci/library/tests/test-library + - name: Run lua tests + id: run_tests2 run: | export TERM=dumb - if build-ci/library/tests/test-library; then - status=0 - script -qe -c "python ci/run-tests.py --headless --keep-status \"$DF_FOLDER\"" || status=$((status + 1)) - python ci/check-rpc.py "$DF_FOLDER/dfhack-rpc.txt" || status=$((status + 2)) - mkdir -p artifacts - cp "$DF_FOLDER"/test*.json "$DF_FOLDER"/*.log artifacts || status=$((status + 4)) - exit $status - fi - exit 1 + status=0 + script -qe -c "python ci/run-tests.py --headless --keep-status \"$DF_FOLDER\"" || status=$((status + 1)) + python ci/check-rpc.py "$DF_FOLDER/dfhack-rpc.txt" || status=$((status + 2)) + mkdir -p artifacts + cp "$DF_FOLDER"/test*.json "$DF_FOLDER"/*.log artifacts || status=$((status + 4)) + exit $status - name: Upload test artifacts uses: actions/upload-artifact@v1 if: (success() || failure()) && steps.run_tests.outcome != 'skipped' From b11b1c3d5f4b9ea57874ca2b5006355f58679abe Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Sun, 13 Nov 2022 12:43:21 -0800 Subject: [PATCH 19/25] Updates build.yml & moves TEST variable setup --- .github/workflows/build.yml | 5 ++++- CMakeLists.txt | 12 ++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1779e486e..f8631bc4e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -99,7 +99,10 @@ jobs: - name: Run unit tests id: run_tests1 run: | - exit build-ci/library/tests/test-library + if build-ci/library/tests/test-library; then + exit 0 + fi + exit 1 - name: Run lua tests id: run_tests2 run: | diff --git a/CMakeLists.txt b/CMakeLists.txt index 29103d495..a1965fe3b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -394,6 +394,12 @@ else() endif() endif() +if(BUILD_TESTING) + message("BUILD TESTS: Core, Scripts") + set(BUILD_SCRIPT_TESTS ON FORCE) + set(BUILD_CORE_TESTS ON FORCE) +endif() + # Support linking against external tinyxml # If we find an external tinyxml, set the DFHACK_TINYXML variable to "tinyxml" # Otherwise, set it to "dfhack-tinyxml" @@ -409,12 +415,6 @@ else() set(DFHACK_TINYXML "dfhack-tinyxml") endif() -if(BUILD_TESTING) - message("BUILD TESTS: Core, Scripts") - set(BUILD_SCRIPT_TESTS ON FORCE) - set(BUILD_CORE_TESTS ON FORCE) -endif() - find_package(ZLIB REQUIRED) if(BUILD_CORE_TESTS) From 72ad7a1b0199b8a9664cc987d33a732876395847 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Fri, 25 Nov 2022 09:42:59 -0800 Subject: [PATCH 20/25] Adds googletest/include for test targets only --- CMakeLists.txt | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a1965fe3b..5a94274c7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -417,10 +417,6 @@ endif() find_package(ZLIB REQUIRED) -if(BUILD_CORE_TESTS) - include_directories(depends/googletest/googletest/include) -endif() - include_directories(depends/protobuf) include_directories(depends/lua/include) include_directories(depends/md5) @@ -437,9 +433,10 @@ if(BUILD_TESTING OR BUILD_CORE_TESTS) macro(dfhack_test name files) message("dfhack_test(${name}, ${files})") add_executable(${name} ${files}) + target_include_directories(${name} PUBLIC depends/googletest/googletest/include) target_link_libraries(${name} dfhack gtest) - add_test(NAME ${name} COMMAND ${name}) set_target_properties(${name} PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") + add_test(NAME ${name} COMMAND ${name}) endmacro() include(CTest) endif() From ae035d5836d0eff129e45508ec98c54b85388e51 Mon Sep 17 00:00:00 2001 From: myk002 Date: Mon, 28 Nov 2022 17:16:48 -0800 Subject: [PATCH 21/25] simplify unit testing setup --- .github/workflows/build.yml | 14 ++++---- CMakeLists.txt | 68 ++++++++++++------------------------ data/CMakeLists.txt | 2 +- depends/CMakeLists.txt | 6 ++-- library/CMakeLists.txt | 4 +-- library/tests/CMakeLists.txt | 9 ++--- 6 files changed, 36 insertions(+), 67 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f8631bc4e..39e332920 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -83,7 +83,7 @@ jobs: -B build-ci \ -G Ninja \ -DDFHACK_BUILD_ARCH=64 \ - -DBUILD_TESTING:BOOL=ON \ + -DBUILD_TESTS:BOOL=ON \ -DBUILD_DEV_PLUGINS:BOOL=${{ matrix.plugins == 'all' }} \ -DBUILD_SIZECHECK:BOOL=${{ matrix.plugins == 'all' }} \ -DBUILD_SKELETON:BOOL=${{ matrix.plugins == 'all' }} \ @@ -96,15 +96,13 @@ jobs: run: | ninja -C build-ci install ccache --show-stats - - name: Run unit tests - id: run_tests1 + - name: Run cpp unit tests + id: run_tests_cpp run: | - if build-ci/library/tests/test-library; then - exit 0 - fi - exit 1 + ninja -C build-ci && ninja -C build-ci test + exit $? - name: Run lua tests - id: run_tests2 + id: run_tests_lua run: | export TERM=dumb status=0 diff --git a/CMakeLists.txt b/CMakeLists.txt index 5a94274c7..e2b123120 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -394,11 +394,10 @@ else() endif() endif() -if(BUILD_TESTING) - message("BUILD TESTS: Core, Scripts") - set(BUILD_SCRIPT_TESTS ON FORCE) - set(BUILD_CORE_TESTS ON FORCE) -endif() +find_package(ZLIB REQUIRED) +include_directories(depends/protobuf) +include_directories(depends/lua/include) +include_directories(depends/md5) # Support linking against external tinyxml # If we find an external tinyxml, set the DFHACK_TINYXML variable to "tinyxml" @@ -415,11 +414,6 @@ else() set(DFHACK_TINYXML "dfhack-tinyxml") endif() -find_package(ZLIB REQUIRED) - -include_directories(depends/protobuf) -include_directories(depends/lua/include) -include_directories(depends/md5) include_directories(depends/lodepng) include_directories(depends/tthread) include_directories(${ZLIB_INCLUDE_DIRS}) @@ -427,42 +421,16 @@ include_directories(depends/clsocket/src) include_directories(depends/xlsxio/include) add_subdirectory(depends) - # Testing with CTest -if(BUILD_TESTING OR BUILD_CORE_TESTS) - macro(dfhack_test name files) - message("dfhack_test(${name}, ${files})") - add_executable(${name} ${files}) - target_include_directories(${name} PUBLIC depends/googletest/googletest/include) - target_link_libraries(${name} dfhack gtest) - set_target_properties(${name} PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") - add_test(NAME ${name} COMMAND ${name}) - endmacro() - include(CTest) -endif() - -include(CMakeDependentOption) -cmake_dependent_option( - BUILD_SCRIPT_TESTS "Install integration tests in hack/scripts/test" OFF - "BUILD_TESTING" OFF) -mark_as_advanced(FORCE BUILD_TESTS) -# Handle deprecated BUILD_TESTS option -option(BUILD_TESTS "Deprecated option; please use BUILD_SCRIPT_TESTS=ON" OFF) - -if(BUILD_TESTING OR BUILD_SCRIPT_TESTS) - if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") - message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") - endif() - install(DIRECTORY ${dfhack_SOURCE_DIR}/test - DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) - install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) -else() - add_custom_target(test - COMMENT "Nothing to do: CMake option BUILD_TESTING is OFF" - # Portable NOOP; need to put something here or the comment isn't displayed - COMMAND cd - ) -endif() +macro(dfhack_test name files) + message("dfhack_test(${name}, ${files})") + add_executable(${name} ${files}) + target_include_directories(${name} PUBLIC depends/googletest/googletest/include) + target_link_libraries(${name} dfhack gtest) + set_target_properties(${name} PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") + add_test(NAME ${name} COMMAND ${name}) +endmacro() +include(CTest) find_package(Git REQUIRED) if(NOT GIT_FOUND) @@ -570,6 +538,16 @@ if(BUILD_DOCS) install(FILES "README.html" DESTINATION "${DFHACK_DATA_DESTINATION}") endif() +option(BUILD_TESTS "Include tests (currently just installs Lua tests into the scripts folder)" OFF) +if(BUILD_TESTS) + if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts") + message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") + endif() + install(DIRECTORY ${dfhack_SOURCE_DIR}/test + DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) + install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) +endif() + # Packaging with CPack! set(DFHACK_PACKAGE_SUFFIX "") if(UNIX) diff --git a/data/CMakeLists.txt b/data/CMakeLists.txt index 8cf6bb2ca..412ffa347 100644 --- a/data/CMakeLists.txt +++ b/data/CMakeLists.txt @@ -21,7 +21,7 @@ install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/blueprints/ FILES_MATCHING PATTERN "*" PATTERN blueprints/library/test EXCLUDE) -if(BUILD_SCRIPT_TESTS) +if(BUILD_TESTS) install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/blueprints/library/test/ DESTINATION blueprints/library/test ) diff --git a/depends/CMakeLists.txt b/depends/CMakeLists.txt index f5e77ad8a..e1c07bd92 100644 --- a/depends/CMakeLists.txt +++ b/depends/CMakeLists.txt @@ -3,10 +3,8 @@ add_subdirectory(lodepng) add_subdirectory(lua) add_subdirectory(md5) add_subdirectory(protobuf) -if(BUILD_CORE_TESTS) - add_subdirectory(googletest EXCLUDE_FROM_ALL) - set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized -Wno-sign-compare") -endif() +add_subdirectory(googletest) +set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-Wno-maybe-uninitialized -Wno-sign-compare") # Don't build tinyxml if it's being externally linked against. if(NOT TinyXML_FOUND) diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 5e7488027..39c5d92d9 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -82,9 +82,7 @@ set(MAIN_SOURCES RemoteTools.cpp ) -if(BUILD_CORE_TESTS) - add_subdirectory(tests) -endif() +add_subdirectory(tests) if(WIN32) set(CONSOLE_SOURCES Console-windows.cpp) diff --git a/library/tests/CMakeLists.txt b/library/tests/CMakeLists.txt index 3e5a2b3ca..8bbc34065 100644 --- a/library/tests/CMakeLists.txt +++ b/library/tests/CMakeLists.txt @@ -1,7 +1,4 @@ -file(GLOB_RECURSE TEST_SOURCES LIST_DIRECTORIES false *test.cpp) +file(GLOB_RECURSE TEST_SOURCES + LIST_DIRECTORIES false + *test.cpp) dfhack_test(test-library "${TEST_SOURCES}") - -# How to get `test` to ensure everything is up to date before running -# tests? This add_dependencies() fails with: -# Cannot add target-level dependencies to non-existent target "test". -#add_dependencies(test MiscUtils.test) From a22e3117d396e1a2a10167cb17cb0fee9bf4757e Mon Sep 17 00:00:00 2001 From: myk002 Date: Mon, 28 Nov 2022 17:22:17 -0800 Subject: [PATCH 22/25] simplify the github action to just ninja test since ninja all was just run --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 39e332920..d7e8a52b8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -99,7 +99,7 @@ jobs: - name: Run cpp unit tests id: run_tests_cpp run: | - ninja -C build-ci && ninja -C build-ci test + ninja -C build-ci test exit $? - name: Run lua tests id: run_tests_lua From 88074dacf047f9002fdba88039f59c5e89c05c62 Mon Sep 17 00:00:00 2001 From: myk002 Date: Mon, 28 Nov 2022 17:31:10 -0800 Subject: [PATCH 23/25] move tests into the same dir as the main files --- library/CMakeLists.txt | 5 ++- library/MiscUtils.test.cpp | 52 ++++++----------------- library/{tests/test.cpp => main.test.cpp} | 0 library/tests/CMakeLists.txt | 4 -- library/tests/MiscUtils.test.cpp | 19 --------- 5 files changed, 16 insertions(+), 64 deletions(-) rename library/{tests/test.cpp => main.test.cpp} (100%) delete mode 100644 library/tests/CMakeLists.txt delete mode 100644 library/tests/MiscUtils.test.cpp diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index 39c5d92d9..d53f098f8 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -82,7 +82,10 @@ set(MAIN_SOURCES RemoteTools.cpp ) -add_subdirectory(tests) +file(GLOB_RECURSE TEST_SOURCES + LIST_DIRECTORIES false + *test.cpp) +dfhack_test(dfhack-test "${TEST_SOURCES}") if(WIN32) set(CONSOLE_SOURCES Console-windows.cpp) diff --git a/library/MiscUtils.test.cpp b/library/MiscUtils.test.cpp index f3360b121..033b226a7 100644 --- a/library/MiscUtils.test.cpp +++ b/library/MiscUtils.test.cpp @@ -1,47 +1,19 @@ -#include "Internal.h" -#include "MiscUtils.h" -#include +#include "MiscUtils.h" +#include #include -using namespace std; -int compare_result(const vector &expect, const vector &result) -{ - if (result == expect) - { - cout << "ok\n"; - return 0; - } - else { - cout << "not ok\n"; - auto e = expect.begin(); - auto r = result.begin(); - cout << "# " << setw(20) << left << "Expected" << " " << left << "Got\n"; - while (e < expect.end() || r < result.end()) - { - cout - << "# " - << setw(20) << left << ":" + (e < expect.end() ? *e++ : "") + ":" - << " " - << setw(20) << left << ":" + (r < result.end() ? *r++ : "") + ":" - << "\n"; - } - return 1; - } -} +TEST(MiscUtils, wordwrap) { + std::vector result; -int main() -{ - int fails = 0; -#define TEST_WORDWRAP(label, expect, args) \ - { \ - vector result; \ - cout << label << "... "; \ - word_wrap args; \ - fails += compare_result(expect, result); \ - } + word_wrap(&result, "123", 3); + ASSERT_EQ(result.size(), 1); - TEST_WORDWRAP("Short line, no wrap", vector({"12345"}), (&result, "12345", 15)); + result.clear(); + word_wrap(&result, "12345", 3); + ASSERT_EQ(result.size(), 2); - return fails == 0 ? 0 : 1; + result.clear(); + word_wrap(&result, "1234567", 3); + ASSERT_EQ(result.size(), 3); } diff --git a/library/tests/test.cpp b/library/main.test.cpp similarity index 100% rename from library/tests/test.cpp rename to library/main.test.cpp diff --git a/library/tests/CMakeLists.txt b/library/tests/CMakeLists.txt deleted file mode 100644 index 8bbc34065..000000000 --- a/library/tests/CMakeLists.txt +++ /dev/null @@ -1,4 +0,0 @@ -file(GLOB_RECURSE TEST_SOURCES - LIST_DIRECTORIES false - *test.cpp) -dfhack_test(test-library "${TEST_SOURCES}") diff --git a/library/tests/MiscUtils.test.cpp b/library/tests/MiscUtils.test.cpp deleted file mode 100644 index 033b226a7..000000000 --- a/library/tests/MiscUtils.test.cpp +++ /dev/null @@ -1,19 +0,0 @@ - -#include "MiscUtils.h" -#include -#include - -TEST(MiscUtils, wordwrap) { - std::vector result; - - word_wrap(&result, "123", 3); - ASSERT_EQ(result.size(), 1); - - result.clear(); - word_wrap(&result, "12345", 3); - ASSERT_EQ(result.size(), 2); - - result.clear(); - word_wrap(&result, "1234567", 3); - ASSERT_EQ(result.size(), 3); -} From d0a6a3e930c7309b4bf2ef5d8e56993719f17b76 Mon Sep 17 00:00:00 2001 From: myk002 Date: Mon, 28 Nov 2022 17:40:06 -0800 Subject: [PATCH 24/25] remove unnecessary target modifications --- CMakeLists.txt | 5 ++--- library/CMakeLists.txt | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e2b123120..9fde3e8f0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -426,8 +426,7 @@ macro(dfhack_test name files) message("dfhack_test(${name}, ${files})") add_executable(${name} ${files}) target_include_directories(${name} PUBLIC depends/googletest/googletest/include) - target_link_libraries(${name} dfhack gtest) - set_target_properties(${name} PROPERTIES COMPILE_FLAGS "-Wno-sign-compare") + target_link_libraries(${name} dfhack gtest SDL) add_test(NAME ${name} COMMAND ${name}) endmacro() include(CTest) @@ -544,7 +543,7 @@ if(BUILD_TESTS) message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.") endif() install(DIRECTORY ${dfhack_SOURCE_DIR}/test - DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) + DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts) endif() diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt index d53f098f8..aee5184c8 100644 --- a/library/CMakeLists.txt +++ b/library/CMakeLists.txt @@ -397,8 +397,6 @@ if(APPLE) target_link_libraries(dfhack ncurses) set_target_properties(dfhack PROPERTIES VERSION 1.0.0) set_target_properties(dfhack PROPERTIES SOVERSION 1.0.0) -elseif(UNIX) - target_link_libraries(dfhack SDL) endif() target_link_libraries(dfhack protobuf-lite clsocket lua jsoncpp_static dfhack-version ${PROJECT_LIBS}) From 757aa303b7f8a1168c744ed84022e00e68a91462 Mon Sep 17 00:00:00 2001 From: Myk Date: Tue, 29 Nov 2022 15:05:04 -0800 Subject: [PATCH 25/25] Update library/MiscUtils.test.cpp Co-authored-by: Josh Cooper --- library/MiscUtils.test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/MiscUtils.test.cpp b/library/MiscUtils.test.cpp index 033b226a7..d3410dad2 100644 --- a/library/MiscUtils.test.cpp +++ b/library/MiscUtils.test.cpp @@ -10,7 +10,7 @@ TEST(MiscUtils, wordwrap) { ASSERT_EQ(result.size(), 1); result.clear(); - word_wrap(&result, "12345", 3); + word_wrap(&result, "123456", 3); ASSERT_EQ(result.size(), 2); result.clear();