Closed dennisklein closed 4 months ago
The updates encompass improving the CMake configuration for test directories by conditionally including subdirectories based on CMake version and integrating DL_PATHS ${LD_LIBRARY_PATH}
into catch_discover_tests
function across various example modules. These changes aim to enhance test discovery and execution by ensuring correct dynamic library paths are set, demonstrating a structured approach to maintaining testing compatibility and functionality.
File Path | Change Summary |
---|---|
tests/CMakeLists.txt |
Conditionally adds base and geobase directories based on CMake version (3.22+). |
tests/base/CMakeLists.txt |
Updates copyright year; adds DL_PATHS option to catch_discover_tests . |
tests/examples/.../pixelDetector/CMakeLists.txt |
Adds DL_PATHS ${LD_LIBRARY_PATH} to catch_discover_tests . |
tests/examples/advanced/.../CMakeLists.txt |
Adds DL_PATHS ${LD_LIBRARY_PATH} to catch_discover_tests in Tutorial3, propagator modules. |
tests/examples/simulation/.../CMakeLists.txt |
Adds DL_PATHS ${LD_LIBRARY_PATH} to catch_discover_tests in Tutorial1, 2, 4 modules. |
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
This could benefit from factoring the catch_discover_tests
call into a new macro since it should probably handle the case when ${LD_LIBRARY_PATH}
is empty. :stuck_out_tongue_winking_eye: at @ChristianTackeGSI - feel free to push to this branch, if you like
@dennisklein,
you should remove the added versions of Catch2 which were probably added by accident,
@dennisklein,
you should remove the added versions of Catch2 which were probably added by accident,
Hm, not sure I understand?
@dennisklein, you should remove the added versions of Catch2 which were probably added by accident,
Hm, not sure I understand?
If I check the changes on
https://github.com/FairRootGroup/FairRoot/pull/1525
I see 574 changed files.
@dennisklein, you should remove the added versions of Catch2 which were probably added by accident,
Hm, not sure I understand?
If I check the changes on
1525
I see 574 changed files.
Yes, that is expected as Catch2 is copied into the source tree.
@dennisklein, you should remove the added versions of Catch2 which were probably added by accident,
Hm, not sure I understand?
If I check the changes on
1525
I see 574 changed files.
Yes, that is expected as Catch2 is copied into the source tree.
Okay. I see. I though Catch2 is only copied when used.
I though Catch2 is only copied when used.
The FetchContent mechanism of CMake could do that, but I believe it is not a good idea to have the configure and build steps do internet downloads. The git clone should be the only step to require one to be online. Otherwise, the project would become a nightmare to package. A packager would have to generate extra Catch2 source treee assets etc
Typically, instead of carrying a copy, one could also solve this by using git submodules. But those impose a new workflow on FairRoot developers which I decided to avoid so far. But I am open to changing that, if you feel dealing with git submodules would be acceptable. They come with their own trade-offs.
Largely based on the research into the problem done by @fuhlig1, thx!
Co-authored-by: @fuhlig1 Co-authored-by: @ChristianTackeGSI