Adapt unit tests #9
Conversation
…faq.html#testing Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
andistorm
force-pushed
the
ah-adapt-unit-tests
branch
from
1b6990d to
442904e
Compare
CMakeLists.txt
Outdated
| include(CTest) | ||
| enable_testing() |
There was a problem hiding this comment.
https://cmake.org/cmake/help/latest/command/enable_testing.html
should not be necessary as we're requiring BUILD_TESTING
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
| @@ -39,7 +40,7 @@ endif() | |||
| # | |||
| # tests | |||
| # | |||
| if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME AND BUILD_TESTING) | |||
| if((${CMAKE_PROJECT_NAME} STREQUAL ${PROJECT_NAME} OR ${PROJECT_NAME}_BUILD_TESTING) AND BUILD_TESTING) | |||
There was a problem hiding this comment.
not sure about the ... AND BUILD_TESTING here, if this is required, we probably will also trigger building tests in our external requirements like libcurl and maybe others
There was a problem hiding this comment.
This line is the result of the discussion here: EVerest/EVerest#129 and and in the WG CI/CD & Testing.
I understand your concern, could be handled by using the options field in dependencies.yaml (setting BUILD_TESTING=OFF)
There was a problem hiding this comment.
Has the discussion, you mentioned, taken into account, that requiring BUILD_TESTING would trigger unit tests in the dependencies?
Concerning the setting BUILD_TESTING in the options field - I'm not sure if this will work - because it is an option with global scope. So it can be only on or off, but not on for one project and off for another one.
| @@ -16,20 +16,21 @@ if (CODE_COVERAGE) | |||
| evc_include(CodeCoverage) | |||
| endif() | |||
|
|
|||
| add_executable(test_state_allocator state_allocator.cpp) | |||
| target_link_libraries(test_state_allocator | |||
| set(TEST_TARGET_NAME ${PROJECT_NAME}_tests) | |||
There was a problem hiding this comment.
TEST_TARGET_NAME is to general here, especially when having multiple tests. There should be a TEST_PREFIX or something similar, which gets prepended to state_allocator
There was a problem hiding this comment.
In other repos we re-use that TEST_TARGET_NAME variable for multiple test cases, so setting it to something like
set(TEST_TARGET_NAME ${PROJECT_NAME}_state_allocator_tests)and later
set(TEST_TARGET_NAME ${PROJECT_NAME}_some_other_target_name_tests)If we just use a TEST_PREFIX we still have to repeat the appended test name, but an argument could be made for unique variable names
Adapt unit tests according https://everest.github.io/nightly/general/faq.html#testing
See EVerest/EVerest#129