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

CMake ParseAndAddCatchTest TEST_CASE_METHOD broken in 2.13.3 #2092

Closed
NeroBurner opened this issue Nov 3, 2020 · 8 comments
Closed

CMake ParseAndAddCatchTest TEST_CASE_METHOD broken in 2.13.3 #2092

NeroBurner opened this issue Nov 3, 2020 · 8 comments
Labels
Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration.

Comments

@NeroBurner
Copy link
Contributor

Describe the bug

The improved regex breaks test-fixtures (like TEST_CASE_METHOD(fixture, "testname")) detection of the ParseAndAddCatchTest helper script

Expected behavior
TEST_CASE_METHOD(fixture, "testcase") tests should be part of the detected test cases

Reproduction steps

add a testcase with a fixture like

TEST_CASE_METHOD(fixture, "testname")
{
    REQUIRE(true);
}

See that the current regex doesn't match the test case with fixture

https://regex101.com/r/j6Y4n9/1

Platform information:

  • Catch version: 2.13.3 (introduced in 2.13.2 with regex change for TEMPLATE_TEST_CASE() and not fixed by my new regex in 2.13.3)

Additional context

@horenmar
Copy link
Member

horenmar commented Nov 3, 2020

Nooooooooo

@NeroBurner
Copy link
Contributor Author

Unfortunately yes, and I understand your frustration 🙈

I'll try to be a bit more methodical, in this case starting with a full list of the Catch2 supported syntaxes.

I've extracted all the test case adding statements from the documentation I could find. Could you have a look if the syntax bits (especially the SIG and METHOD ones, those are created by me). Did I miss some methods?

// TEST_CASE( test name [, tags ] )
TEST_CASE("test name")
{
}
TEST_CASE("test name", "[tag]")
{
}
# SCENARIO( scenario name [, tags ] )
SCENARIO("test name")
{
}
SCENARIO("test name", "[tag]")
{
}

// Type parametrised test cases
// TEMPLATE_TEST_CASE( test name , tags, type1, type2, ..., typen )
TEMPLATE_TEST_CASE( "vectors can be sized and resized", "[vector][template]", int, std::string, (std::tuple<int,float>) )
{
}

// TEMPLATE_PRODUCT_TEST_CASE( test name , tags, (template-type1, template-type2, ..., template-typen), (template-arg1, template-arg2, ..., template-argm) )
TEMPLATE_PRODUCT_TEST_CASE("A Template product test case", "[template][product]", (std::vector, Foo), (int, float))
{
}

// TEMPLATE_LIST_TEST_CASE( test name, tags, type list )
TEMPLATE_LIST_TEST_CASE("Template test case with test types specified inside std::tuple", "[template][list]", MyTypes)
{
}

// Signature based parametrised test cases
// TEMPLATE_TEST_CASE_SIG( test name , tags, signature, type1, type2, ..., typen )
TEMPLATE_TEST_CASE_SIG("TemplateTestSig: arrays can be created from NTTP arguments", "[vector][template][nttp]",
     ((typename T, int V), T, V), (int,5), (float,4), (std::string,15), ((std::tuple<int, float>), 6))
{
}

// TEMPLATE_PRODUCT_TEST_CASE_SIG( test name , tags, signature, (template-type1, template-type2, ..., template-typen), (template-arg1, template-arg2, ..., template-argm) )
TEMPLATE_PRODUCT_TEST_CASE_SIG("A Template product test case with array signature", "[template][product][nttp]", ((typename T, size_t S), T, S), (std::array, Bar), ((int, 9), (float, 42)))
{
}

// Test fixtures
// TEST_CASE_METHOD( fixture, test name [, tags])
TEST_CASE_METHOD(UniqueTestsFixture, "Create Employee/No Name", "[create]")
{
}

// TEMPLATE_TEST_CASE_METHOD( fixture, test name , tags, type1, type2, ..., typen )
TEMPLATE_TEST_CASE_METHOD(Template_Fixture,"A TEMPLATE_TEST_CASE_METHOD based test run that succeeds", "[class][template]", int, float, double)
{
}

// Signature-based parametrised test fixtures
// TEMPLATE_TEST_CASE_METHOD_SIG( fixture, test name, tags, signature, type1, type2, ..., typen )
TEMPLATE_TEST_CASE_METHOD_SIG(Nttp_Fixture, "A TEMPLATE_TEST_CASE_METHOD_SIG based test run that succeeds", "[class][template][nttp]",((int V), V), 1, 3, 6)
{
}

// TEMPLATE_PRODUCT_TEST_CASE_METHOD_SIG( fixture, test name , tags, signature, (template-type1, template-type2, ..., template-typen), (template-arg1, template-arg2, ..., template-argm) )
TEMPLATE_PRODUCT_TEST_CASE_METHOD_SIG(Template_Fixture_2, "A TEMPLATE_PRODUCT_TEST_CASE_METHOD_SIG based test run that succeeds", "[class][template][product][nttp]", ((typename T, size_t S), T, S),(std::array, Template_Foo_2), ((int,2), (float,6)))
{
}

// Template fixtures with types specified in template type lists
// TEMPLATE_LIST_TEST_CASE_METHOD( fixture, test name, tags, type list)
TEMPLATE_LIST_TEST_CASE_METHOD(Template_Fixture, "Template test case method with test types specified inside std::tuple", "[class][template][list]", MyTypes)
{
}

// Test case related macros
// METHOD_AS_TEST_CASE( member-function-pointer, description )
METHOD_AS_TEST_CASE( TestClass::testCase, "Use class's method as a test case", "[class]" )

// REGISTER_TEST_CASE( function, description )
REGISTER_TEST_CASE( someFunction, "ManuallyRegistered", "[tags]" );

See regex link to see which ones do match (not many of the examples unfortunately)
https://regex101.com/r/vWFKrb/1

@horenmar
Copy link
Member

horenmar commented Nov 5, 2020

I should be able to look at these this week, but a simple idea first: instead of trying to extend the regex to handle all cases, let's have multiple ones.

@horenmar horenmar added the Extras Touches utility scripts outside of Catch2 proper, e.g. CMake integration. label Nov 5, 2020
@horenmar
Copy link
Member

horenmar commented Nov 7, 2020

More notes:

  • TEMPLATE_PRODUCT_TEST_CASE can be used in more complex ways, and, if one of the list is unitary, it doesn't have to be encased in parentheses. This means that the following are also valid
TEMPLATE_PRODUCT_TEST_CASE("Product with differing arities", "[template][product]", std::tuple, (int, (int, double), (int, double, float))) {
}
TEMPLATE_PRODUCT_TEST_CASE("Product with differing arities", "[template][product]", (std::vector, std::list), int) {
}
  • TEST_CASE can be anonymous
TEST_CASE() {
}

@NeroBurner
Copy link
Contributor Author

how can we handle the anonymous test cases? Those can't be reference by name, as they are anonymous

@horenmar
Copy link
Member

It has a name of Anonymous test case N, so I would do the same that is done for template test cases -- use a wildcard of Anonymous test case * and register them all as one CTest test.

norbertwenzel added a commit to NeroBurner/Catch2 that referenced this issue Dec 8, 2020
currently missing macros: signature variants of template test cases

For every macro only a single parameter set is used. This needs to test
all parameter combinations that are allowed for catchorg#2092.
norbertwenzel added a commit to NeroBurner/Catch2 that referenced this issue Dec 8, 2020
For every macro only a single parameter set is used. This needs to test
all parameter combinations that are allowed for catchorg#2092.
norbertwenzel added a commit to NeroBurner/Catch2 that referenced this issue Dec 8, 2020
For every macro only a single parameter set is used. This needs to test
all parameter combinations that are allowed for catchorg#2092.
@NeroBurner
Copy link
Contributor Author

NeroBurner commented Dec 17, 2020

@horenmar

TL;DR: I suggest deprecating (or preferably remove) ParseAndAddCatchTest.cmake helper scipt and completely replace it in the documentation with the Catch.cmake helper script

Long verson:

I tried to update the ParseAndAddCatchTest.cmake script. First with warnings on unsupported Catch-test-macros and then started to update the regexes and split them up. Some time after falling into this rabbit whole the nagging feeling got bigger and bigger: We can't be 100% sure to get the regex right for parsing C++ code and get ALL the test macros. We may get to 90%, but most probably silently fail with a few test cases. For a project these few test cases could be the ones catching a nasty bug.

In talks with a colleague we said to ourselves: we already build a binary with a defined interface, a defined output and the C++ source code parsed and checked correctly. Why not parse the test-binaries output. Luckily someone smarter than us came to the same conclusion and already added the contrib/Catch.cmake and contrib/CatchAddTests.cmake helper scripts.

With the Catch 3.x release coming up this would be the perfect time to introduce this breaking change (removing ParseAndAddCatchTest.cmake).

@horenmar
Copy link
Member

I suggest deprecating (or preferably remove) ParseAndAddCatchTest.cmake helper scipt and completely replace it in the documentation with the Catch.cmake helper script

I fundamentally agree, and have agreed for a long time. People just keep reporting bugs against it, suggesting that they use it for some reason 🤷

The better approach is already placed first in the documentation, maybe marking the Parse approach deprecated, and making the documentation more pushy about it would help.

NeroBurner added a commit to NeroBurner/Catch2 that referenced this issue Dec 17, 2020
Parsing C++ with regex in CMake is error prone and regularly leads to silently
dropped (not run) test cases.

Going forward the function `catch_discover_tests` from `contrib/CMake.cmake`
should be used.

For more information see catchorg#2092 (comment)
norbertwenzel pushed a commit to NeroBurner/Catch2 that referenced this issue Dec 20, 2020
Parsing C++ with regex in CMake is error prone and regularly leads to silently
dropped (not run) test cases.

Going forward the function `catch_discover_tests` from `contrib/CMake.cmake`
should be used.

For more information see catchorg#2092 (comment)
NeroBurner added a commit to NeroBurner/Catch2 that referenced this issue Dec 21, 2020
Parsing C++ with regex in CMake is error prone and regularly leads to silently
dropped (not run) test cases.

Going forward the function `catch_discover_tests` from `contrib/CMake.cmake`
should be used.

For more information see catchorg#2092 (comment)
horenmar pushed a commit that referenced this issue Dec 21, 2020
Parsing C++ with regex in CMake is error prone and regularly leads to silently
dropped (not run) test cases.

Going forward the function `catch_discover_tests` from `contrib/CMake.cmake`
should be used.

For more information see #2092 (comment)
horenmar pushed a commit that referenced this issue Apr 11, 2021
Parsing C++ with regex in CMake is error prone and regularly leads to silently
dropped (not run) test cases.

Going forward the function `catch_discover_tests` from `contrib/CMake.cmake`
should be used.

For more information see #2092 (comment)
horenmar pushed a commit that referenced this issue May 9, 2021
Parsing C++ with regex in CMake is error prone and regularly leads to silently
dropped (not run) test cases.

Going forward the function `catch_discover_tests` from `contrib/CMake.cmake`
should be used.

For more information see #2092 (comment)
horenmar pushed a commit that referenced this issue May 9, 2021
Parsing C++ with regex in CMake is error prone and regularly leads to silently
dropped (not run) test cases.

Going forward the function `catch_discover_tests` from `contrib/CMake.cmake`
should be used.

For more information see #2092 (comment)
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

No branches or pull requests

2 participants