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

Fix CMake regex in ParseAndAddCatchTest helper #2056

Merged
merged 2 commits into from
Oct 20, 2020

Conversation

NeroBurner
Copy link
Contributor

Description

Fix regex that requires two string arguments in the form of
TEST_CASE("a", "b") resulting in not finding TEST_CASE("a") entries.

See regex: https://regex101.com/r/rQe6Jd/1

GitHub Issues

Fixes: #2055

@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #2056 into v2.x will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             v2.x    #2056   +/-   ##
=======================================
  Coverage   88.75%   88.75%           
=======================================
  Files         138      138           
  Lines        5651     5651           
=======================================
  Hits         5015     5015           
  Misses        636      636           

@horenmar
Copy link
Member

Honestly this calls for a proper parser, because the new regex breaks TEMPLATE_TEST_CASE instead:

https://regex101.com/r/rQe6Jd/2

@horenmar horenmar added the Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration. label Oct 19, 2020
@NeroBurner
Copy link
Contributor Author

Do you maybe also have an idea on how we could add tests for the parser, that such oversights are covered by the test cases?

@horenmar
Copy link
Member

I'd go with having a separate CMake project that uses the parser for a known source file, and a Python script that first configures the project, and then runs ctest -N and checks that the output lists all tests.

Fix regex that requires two string arguments in the form of
TEST_CASE("a", "b") resulting in not finding TEST_CASE("a") entries.

See https://regex101.com/r/JygOND/1

Fixes: catchorg#2055
@NeroBurner NeroBurner force-pushed the fix_cmake_parse_test_regex branch from 975d25c to ff234db Compare October 20, 2020 06:15
@NeroBurner
Copy link
Contributor Author

updated the regex to cover the template cases https://regex101.com/r/JygOND/1

@horenmar
Copy link
Member

Okay, looks fine.

However, I can't help but think of the regex, two problems saying. 😃

@horenmar
Copy link
Member

I just noticed that you target master branch. Don't, it is dead, I just kept it around for a while in case something broke too terribly after I changed the default branch.

For v2 branch target v2.x, for current development target devel.

Don't worry about the already merged PR, I'll cherry-pick it into the right branch later.

@NeroBurner NeroBurner changed the base branch from master to v2.x October 20, 2020 11:06
@NeroBurner
Copy link
Contributor Author

changed it, thanks for the heads up

@horenmar horenmar merged commit 77b2a7d into catchorg:v2.x Oct 20, 2020
@NeroBurner NeroBurner deleted the fix_cmake_parse_test_regex branch October 20, 2020 11:51
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.

3 participants