-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
fix: TEST_DL_PATHS
to multi-args
#2825
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #2825 +/- ##
==========================================
+ Coverage 91.19% 91.20% +0.01%
==========================================
Files 197 197
Lines 8374 8374
==========================================
+ Hits 7636 7637 +1
+ Misses 738 737 -1 |
TEST_DL_PATHS
to multi-argsTEST_DL_PATHS
to multi-args
Re tests: I am not sure whether the DL_PATHS property shows up under There is already a test that configures build, runs |
Cool I can add the test-case there. By default I think it should be using 2 DLLs, unless we need to add a dummy library target. I think functional tests like that should use the built-in |
8d5d93e
to
b133498
Compare
Signed-off-by: Cristian Le <cristian.le@mpsd.mpg.de>
b133498
to
2558001
Compare
I see you added the setting to the CMakeLists for the tests, but there is no check in the script that runs the test. Do you intend to pursue this further? |
It is partly implicitly added, i.e. by |
Description
As @daniel-slater mentioned in #2736, the issue was that
TEST_DL_PATHS
inCatchAddTests.cmake
should be a multi-argument variable. I have tested it against a generator-expression input like:example_run
I did not add proper tests to check for this issue because I couldn't find any functional tests that run
ctest --build-and-test
, and that would require some discussion on how to structure and place these.Also note that there is a much cleaner approach by automatically adding
TARGET_RUNTIME_DLL_DIRS
, which I will try to do in another PR.GitHub Issues
Closes #2736