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

How to specify configuration in catch_discover_tests #1595

Closed
jdumas opened this issue Apr 8, 2019 · 11 comments
Closed

How to specify configuration in catch_discover_tests #1595

jdumas opened this issue Apr 8, 2019 · 11 comments
Labels
Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration. Resolved - pending review Issue waiting for feedback from the original author

Comments

@jdumas
Copy link

jdumas commented Apr 8, 2019

So, using contrib/Catch.cmake we have catch_discover_tests() which is nice. However, for Windows users with multiple configurations it would be nice to be able to register tests in ctest using the appropriate CONFIGURATIONS for each test.

I tried to fiddle a bit with the contrib/CatchAddTests.cmake but I couldn't figure out how to make it work (it keeps naming all my tests NAME if I try to use the first signature of add_test(NAME ...) in the ctest script).

Anyone has suggestion on how to achieve this?

@horenmar horenmar added the Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration. label Apr 10, 2019
@MaciejPatro
Copy link
Contributor

I'm not sure how helpful it is but looking at add_test(NAME ...) vs add_test(...)

Add a test called <name>. The test name may not contain spaces, quotes, or other characters special in CMake syntax. The options are:

versus

Add a test called <name> with the given command-line. Unlike the above NAME signature no transformation is performed on the command-line to support target names or generator expressions.

It might be that the test name you provide is not proper for this command.

This property of add_test(NAME ...) will require parsing original test names and modifying them for ctest. I'm not sure how big consequences this might have for other users.

@qak
Copy link

qak commented Apr 27, 2023

I've been trying to get Catch2 (specifically catch_discover_tests()) to work correctly with the Ninja multi-config generator. The problem is that catch_discover_tests() writes information about tests to ${CMAKE_CURRENT_BINARY_DIR}/${TARGET}_include-${args_hash}.cmake and ${CMAKE_CURRENT_BINARY_DIR}/${TARGET}_tests-${args_hash}.cmake, which are identical files for all enabled configurations. As a result, building one configuration overwrites the mentioned .cmake files of the other enabled configurations. This causes unnecessary rebuilds because the build tool (Ninja in my case) wants to relink the tests executable every time the .cmake files created by catch_discover_tests() are modified. I guess this is why the creator of this issue opened it in the first place.

My question is, is there really no way to work around this issue using CMake? I've tried modifying the name of the TARGET (which represents the tests executable) passed to catch_discover_tests() to include the name of the current configuration using a generator expression, but CMake doesn't allow this.

If the only way to resolve this issue is to modify Catch2, I think this means catch_discover_tests() is incompatible with all multi-config generators in CMake. Should I open a new issue for this?

@AndreGosselink
Copy link

AndreGosselink commented Aug 6, 2023

I stumbled upon the same thing. I was hoping, to be able to squeeze the config parameter in somewhere... But seemingly this is not possible, given @qak s comment. Only alternative is to use config as name pre or postfixes then. There is no other workaround?

@be-sc
Copy link

be-sc commented Dec 6, 2023

@AndreGosselink Can you explain why you struck out the config name pre/postfix idea? What’s the problem with it? I was thinking along the same lines for a fix:

  1. Check if the generator is multi-config.
  2. If it is: Create one include.cmake and tests.cmake file pair for each active build config, probably using the configs’ dedicated subdirectories in the build dir.
  3. If it is not: Do the same thing as implemented now.

@qak
Copy link

qak commented Dec 9, 2023

@be-sc My impression is that you've suggested a solution to the problem that involves altering the internals of Catch2, whereas it appears @AndreGosselink was attempting to work around the problem by passing appropriate arguments to catch_discover_tests() from a project that uses Catch2.

@be-sc
Copy link

be-sc commented Dec 9, 2023

I just realized multi-config support for PRE_TEST is already present on the devel branch (#2739). It works great for Ninja Multi-Config.

@qak
Copy link

qak commented Dec 12, 2023

It looks like support for multi-config CMake generators has been added to Catch2 version 3.5.0, according to the changelog. It'll take me a while to test this out (maybe up to several weeks), so if somebody gets to try out the new functionality, please don't hesitate to follow up in this thread.

@AndreGosselink
Copy link

@be-sc My impression is that you've suggested a solution to the problem that involves altering the internals of Catch2, whereas it appears @AndreGosselink was attempting to work around the problem by passing appropriate arguments to catch_discover_tests() from a project that uses Catch2.

I can't remember exactly what I was trying, but I somewhat wanted to hand down a configuration value to ctests add_test(... CONFIGURATIONS).

It looks like support for multi-config CMake generators has been added to Catch2 version 3.5.0, according to the changelog. It'll take me a while to test this out (maybe up to several weeks), so if somebody gets to try out the new functionality, please don't hesitate to follow up in this thread.

Will try it, cheers

@horenmar
Copy link
Member

As I understand it, this should be solved. Both XCode and MSVC projects can now use discover_tests, which are both multi-config build systems.

@horenmar horenmar added the Resolved - pending review Issue waiting for feedback from the original author label Dec 12, 2023
@qak
Copy link

qak commented Dec 19, 2023

I've tested using catch_discover_tests(… DISCOVERY_MODE "PRE_TEST") with Catch2 3.5.0, and I can confirm it now works correctly with the Ninja multi-config generator on GNU/Linux.

@danra
Copy link

danra commented Feb 29, 2024

PRE_TEST mode indeed works great now for me with the Xcode multi-config generator (resolving #2746), but it seems that the default POST_BUILD discovery mode still doesn't: ctest --build-config <config> will run the tests with the most recently built configuration, regardless of the sepcified <config>. I suppose this is because of the reason already mentioned above:

building one configuration overwrites the mentioned .cmake files of the other enabled configurations.

(I noticed this issue when I saw that the CI for one of my projects, which builds first the Debug and then the Release configuration and then attempts to test both configurations via ctest --build-config Debug and ``ctest --build-config Release, actually runs Release` build tests twice.)

In case it's technically feasible to fix the behavior of CMake multi-config generators with Catch2's default POST_BUILD discovery mode, I suggest reopening the issue to make the fix.

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. Resolved - pending review Issue waiting for feedback from the original author
Projects
None yet
Development

No branches or pull requests

7 participants