Skip to content
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

catch_discover_tests() DL_PATHS can't handle two or more paths #2736

Closed
SolomidHero opened this issue Aug 22, 2023 · 2 comments · Fixed by #2825
Closed

catch_discover_tests() DL_PATHS can't handle two or more paths #2736

SolomidHero opened this issue Aug 22, 2023 · 2 comments · Fixed by #2825
Labels
Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration.

Comments

@SolomidHero
Copy link

Describe the bug
I am trying to specify shared libs paths to tests. It was fine for 1 library to add it to DL_PATHS of catch_discover_tests(), but adding more paths has no effect.

catch_discover_tests(Tests
    DL_PATHS
        "${path_to_A}/lib"
        "${path_to_B}/lib"  # test executable won't use it
)

I understood it with some tricky logic:

  • Library A is 3rd party and no such on system.
  • Library B is 3rd party too, but system has it analog with a little changed functionality (say older version). So without providing path B my executable runs, but fails internally.

When running ctest --verbose I saw:

...
1: Working Directory: C:/Users/Administrator/Project/build
1: Environment variable modifications:
1:  PATH=path_list_prepend:C:\Users\Administrator\Project\build\path\to\A\lib 
...

see only one lib in PATH, but DL_PATHS provided with 2.

Expected behavior

Test executable would have PATH prefixed with all paths provided in DL_PATHS parameter of catch_discover_tests()

Reproduction steps
You can try having small repro with such

enable_testing()

include(FetchContent)
FetchContent_Declare(
    Catch2
    GIT_REPOSITORY https://github.com/catchorg/Catch2.git
    GIT_PROGRESS TRUE
    GIT_SHALLOW TRUE
    GIT_TAG v3.4.0 # I used this version
)
FetchContent_MakeAvailable(Catch2)

# define test executable and link shared/imported libs
...


catch_discover_tests(Tests
    EXTRA_ARGS ${TEST_ARGS}
    DL_PATHS
        "$Env{PATH}"
        "${path_to_A}/lib")

i.e. I added needed library after some placeholder. It won't work either

Platform information:

  • OS: Windows 10.0.20348.0
  • Compiler+version: MSVC 17 19.36.32532.0
  • Catch version: v3.4.0
  • cmake 3.26.4

Additional context
I see DL_PATHS passed somewhere on POST_BUILD branch:

-D "TEST_DL_PATHS=${_DL_PATHS}"
@daniel-slater
Copy link

This happens because the call to cmake_parse_arguments() inside function(catch_discover_tests_impl) in CatchAddTests.cmake has TEST_DL_PATHS in the one-value keywords list, instead of the multi-value keywords list.

@LecrisUT
Copy link
Contributor

LecrisUT commented Feb 26, 2024

@daniel-slater I am not sure about that, in CatchAddTests.cmake, it is parsed inside as:

cmake_path(CONVERT "${dl_paths}" TO_NATIVE_PATH_LIST paths)

So dl_paths should have escaped ; or :. I am not sure if it gets double escaped in this case.

But btw, there is a more easier approach to DL_PATHS using TARGET_RUNTIME_DLL_DIRS. I will write a simple PR to add this for CMake >= 3.27

Edit: Actually you are right about the multi-value, I did a quick test with it and indeed it succeeded. It even worked as a gen-ex: DL_PATHS "$<TARGET_RUNTIME_DLL_DIRS:test-suite>"

@horenmar horenmar added the Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration. label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants