search

TheLartians / ModernCppStarter

🚀 Kick-start your C++! A template for modern C++ projects using CMake, CI, code coverage, clang-format, reproducible dependency management and much more.
https://thelartians.github.io/ModernCppStarter
The Unlicense
4.45k stars 388 forks source link

cxxopts tests are leaked in the all project #90

Closed TheLartians closed 3 years ago

TheLartians commented 3 years ago

Introduced by using the new shorthand syntax which excludes inner targets. However, this doesn't prevent cxxopts from adding its tests the main tests target. We should therefore add the flag to disable tests again.

Originally posted by @ClausKlein in https://github.com/TheLartians/ModernCppStarter/pull/88#issuecomment-788682734

ClausKlein commented 3 years ago

@TheLartians IMHO the problem is caused by doctest.

see https://github.com/TheLartians/ModernCppStarter/pull/83/commits/75c10df2d84948737ba4bebbabb85aa539a55697#diff-33394812ba204689144fd2f80832db83853ba1cb32403edb4e15fe4893e675fdR61

ClausKlein commented 3 years ago

@TheLartians IMHO the problem is caused by doctest.

see 75c10df#diff-33394812ba204689144fd2f80832db83853ba1cb32403edb4e15fe4893e675fdR61

No!

ClausKlein commented 3 years ago

IMHO we have to prevent the extern project do run their tests at all

bash-3.2$ ninja options_test 
[0/2] Re-checking globbed directories...
[3/3] Linking CXX executable _deps/cxxopts-build/test/options_test
bash-3.2$ 
bash-3.2$ ctest
Test project /Users/clausklein/Workspace/cpp/ModernCppStarter/build/all
    Start 1: options
1/4 Test #1: options ..........................   Passed    0.01 sec
    Start 2: find-package-test
2/4 Test #2: find-package-test ................   Passed    3.01 sec
    Start 3: add-subdirectory-test
3/4 Test #3: add-subdirectory-test ............   Passed    3.03 sec
    Start 4: greeterTests
4/4 Test #4: greeterTests .....................   Passed    0.01 sec

100% tests passed, 0 tests failed out of 4

Total Test time (real) =   6.05 sec
ClausKlein commented 3 years ago

it is caused by CXXOPTS_BUILD_TESTS

TheLartians commented 3 years ago

Fixed by #91