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

Make catch tags work with ctest labels #2696

Closed
wants to merge 5 commits into from
Closed

Conversation

Bidski
Copy link

@Bidski Bidski commented Jun 1, 2023

Description

Developed independently of #1590 but with the same goal in mind. Extract the tags from each test and apply them as labels to the generated CTest tests.

This is achieved by adding a --verbosity quiet filter to the --list-tags command line so that only the tags are output without any other information (similar to --verbosity quiet for --list-tests). The cmake script then creates a list out of these tags and sets them as labels

GitHub Issues

Closes #1590

This PR also achieves the same (or a similar) end results as #2690 (I failed to see this PR before I made this one). Since these two PRs achieve the same thing in a different way I will leave this PR open so that it trade offs to the two approaches can be discussed.

@Bidski Bidski changed the title Open Make catch tags work with ctest labels Make catch tags work with ctest labels Jun 1, 2023
@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Merging #2696 (9e31c7b) into devel (0631b60) will decrease coverage by 0.08%.
The diff coverage is 33.33%.

❗ Current head 9e31c7b differs from pull request most recent head b719527. Consider uploading reports for the commit b719527 to get more accurate results

@@            Coverage Diff             @@
##            devel    #2696      +/-   ##
==========================================
- Coverage   91.19%   91.11%   -0.08%     
==========================================
  Files         192      192              
  Lines        7843     7852       +9     
==========================================
+ Hits         7152     7154       +2     
- Misses        691      698       +7     

@horenmar horenmar added the Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration. label Jun 15, 2023
@horenmar
Copy link
Member

horenmar commented Jun 15, 2023

Thanks, but any approach that has to call the binary per test is unusable for the final product.

Assuming 500 tests discovered (this is bit more than Catch2 has in the SelfTest binary, about half of what I've had in various work projects), and process creation overhead between 5 ms (no AV/no process creation filters) and 20 ms (various hooked processes), the tag retrieval will add between 2.5s and 10s overhead (before counting any time spent by Catch2 to actually retrieve the tests).

The only usable approach for adding tags as labels in CTest is to use the output from --list-tests and parse the tags from that.

---edit---

I did some extra tests for the performance overhead numbers

  • The whole of SelfTest executes serially in 1.6s on my unplugged notebook. The discovery would literally add more overhead than if all the tests are run serially. With parallel execution, there is no contest.
  • Tests for "Workproject P" execute in 18s in parallel. The discovery would add about 25% overhead.
  • Tests for "Workproject G" execute in 2 minutes in parallel. The discovery would add negligible overhead in most cases.

@LecrisUT
Copy link
Contributor

Is #2690 superseding this one then?

@horenmar
Copy link
Member

Yes

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 this pull request may close these issues.

Make catch tags work with ctest labels
3 participants