From 5d32ce26f4c18588f9eac3342aea122a38b8fed3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sun, 12 Apr 2020 18:44:42 +0200 Subject: [PATCH] Fix bug in test spec parser handling of escaping in ORed patterns It did not clear out all of its internal state when switching from one pattern to another, so when it should've escaped `,`, it took its position from its position in the original user-provided string, rather than its position in the current pattern. Fixes #1905 --- include/internal/catch_test_spec_parser.cpp | 2 ++ .../Baselines/compact.sw.approved.txt | 3 ++ .../Baselines/console.std.approved.txt | 4 +-- .../Baselines/console.sw.approved.txt | 25 +++++++++++++-- .../Baselines/console.swa4.approved.txt | 25 +++++++++++++-- .../SelfTest/Baselines/junit.sw.approved.txt | 3 +- .../Baselines/sonarqube.sw.approved.txt | 1 + .../SelfTest/Baselines/xml.sw.approved.txt | 31 +++++++++++++++++-- .../IntrospectiveTests/CmdLine.tests.cpp | 11 +++++++ 9 files changed, 96 insertions(+), 9 deletions(-) diff --git a/include/internal/catch_test_spec_parser.cpp b/include/internal/catch_test_spec_parser.cpp index dad15c01..40fcdcc3 100644 --- a/include/internal/catch_test_spec_parser.cpp +++ b/include/internal/catch_test_spec_parser.cpp @@ -168,6 +168,7 @@ namespace Catch { m_pos = m_arg.size(); m_substring.clear(); m_patternName.clear(); + m_realPatternPos = 0; return false; } endMode(); @@ -186,6 +187,7 @@ namespace Catch { } m_patternName.clear(); + m_realPatternPos = 0; return token; } diff --git a/projects/SelfTest/Baselines/compact.sw.approved.txt b/projects/SelfTest/Baselines/compact.sw.approved.txt index eef23b5a..10fcad66 100644 --- a/projects/SelfTest/Baselines/compact.sw.approved.txt +++ b/projects/SelfTest/Baselines/compact.sw.approved.txt @@ -23,6 +23,9 @@ This would not be caught previously Nor would this Tricky.tests.cpp:: failed: explicitly with 1 message: '1514' Compilation.tests.cpp:: passed: std::is_same, TypeList>::value for: true +CmdLine.tests.cpp:: passed: spec.matches(fakeTestCase("spec . char")) for: true +CmdLine.tests.cpp:: passed: spec.matches(fakeTestCase("spec , char")) for: true +CmdLine.tests.cpp:: passed: !(spec.matches(fakeTestCase(R"(spec \, char)"))) for: !false Exception.tests.cpp:: failed: unexpected exception with message: 'answer := 42' with 1 message: 'expected exception' Exception.tests.cpp:: failed: unexpected exception with message: 'answer := 42'; expression was: thisThrows() with 1 message: 'expected exception' Exception.tests.cpp:: passed: thisThrows() with 1 message: 'answer := 42' diff --git a/projects/SelfTest/Baselines/console.std.approved.txt b/projects/SelfTest/Baselines/console.std.approved.txt index c0357adb..d3a733d2 100644 --- a/projects/SelfTest/Baselines/console.std.approved.txt +++ b/projects/SelfTest/Baselines/console.std.approved.txt @@ -1380,6 +1380,6 @@ due to unexpected exception with message: Why would you throw a std::string? =============================================================================== -test cases: 307 | 233 passed | 70 failed | 4 failed as expected -assertions: 1677 | 1525 passed | 131 failed | 21 failed as expected +test cases: 308 | 234 passed | 70 failed | 4 failed as expected +assertions: 1680 | 1528 passed | 131 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/console.sw.approved.txt b/projects/SelfTest/Baselines/console.sw.approved.txt index 1b0a8ce2..9a5b7eb3 100644 --- a/projects/SelfTest/Baselines/console.sw.approved.txt +++ b/projects/SelfTest/Baselines/console.sw.approved.txt @@ -188,6 +188,27 @@ Compilation.tests.cpp:: PASSED: with expansion: true +------------------------------------------------------------------------------- +#1905 -- test spec parser properly clears internal state between compound tests +------------------------------------------------------------------------------- +CmdLine.tests.cpp: +............................................................................... + +CmdLine.tests.cpp:: PASSED: + REQUIRE( spec.matches(fakeTestCase("spec . char")) ) +with expansion: + true + +CmdLine.tests.cpp:: PASSED: + REQUIRE( spec.matches(fakeTestCase("spec , char")) ) +with expansion: + true + +CmdLine.tests.cpp:: PASSED: + REQUIRE_FALSE( spec.matches(fakeTestCase(R"(spec \, char)")) ) +with expansion: + !false + ------------------------------------------------------------------------------- #748 - captures with unexpected exceptions outside assertions @@ -13427,6 +13448,6 @@ Misc.tests.cpp: Misc.tests.cpp:: PASSED: =============================================================================== -test cases: 307 | 217 passed | 86 failed | 4 failed as expected -assertions: 1694 | 1525 passed | 148 failed | 21 failed as expected +test cases: 308 | 218 passed | 86 failed | 4 failed as expected +assertions: 1697 | 1528 passed | 148 failed | 21 failed as expected diff --git a/projects/SelfTest/Baselines/console.swa4.approved.txt b/projects/SelfTest/Baselines/console.swa4.approved.txt index 322b8158..e32fd88f 100644 --- a/projects/SelfTest/Baselines/console.swa4.approved.txt +++ b/projects/SelfTest/Baselines/console.swa4.approved.txt @@ -188,6 +188,27 @@ Compilation.tests.cpp:: PASSED: with expansion: true +------------------------------------------------------------------------------- +#1905 -- test spec parser properly clears internal state between compound tests +------------------------------------------------------------------------------- +CmdLine.tests.cpp: +............................................................................... + +CmdLine.tests.cpp:: PASSED: + REQUIRE( spec.matches(fakeTestCase("spec . char")) ) +with expansion: + true + +CmdLine.tests.cpp:: PASSED: + REQUIRE( spec.matches(fakeTestCase("spec , char")) ) +with expansion: + true + +CmdLine.tests.cpp:: PASSED: + REQUIRE_FALSE( spec.matches(fakeTestCase(R"(spec \, char)")) ) +with expansion: + !false + ------------------------------------------------------------------------------- #748 - captures with unexpected exceptions outside assertions @@ -368,6 +389,6 @@ Condition.tests.cpp:: FAILED: CHECK( true != true ) =============================================================================== -test cases: 19 | 14 passed | 3 failed | 2 failed as expected -assertions: 42 | 35 passed | 4 failed | 3 failed as expected +test cases: 20 | 15 passed | 3 failed | 2 failed as expected +assertions: 45 | 38 passed | 4 failed | 3 failed as expected diff --git a/projects/SelfTest/Baselines/junit.sw.approved.txt b/projects/SelfTest/Baselines/junit.sw.approved.txt index e8f7c09f..8ac1236d 100644 --- a/projects/SelfTest/Baselines/junit.sw.approved.txt +++ b/projects/SelfTest/Baselines/junit.sw.approved.txt @@ -1,7 +1,7 @@ - + @@ -30,6 +30,7 @@ Nor would this + FAILED: diff --git a/projects/SelfTest/Baselines/sonarqube.sw.approved.txt b/projects/SelfTest/Baselines/sonarqube.sw.approved.txt index 2f11cf5b..72fdd780 100644 --- a/projects/SelfTest/Baselines/sonarqube.sw.approved.txt +++ b/projects/SelfTest/Baselines/sonarqube.sw.approved.txt @@ -2,6 +2,7 @@ + diff --git a/projects/SelfTest/Baselines/xml.sw.approved.txt b/projects/SelfTest/Baselines/xml.sw.approved.txt index cc90e0cb..fff44abd 100644 --- a/projects/SelfTest/Baselines/xml.sw.approved.txt +++ b/projects/SelfTest/Baselines/xml.sw.approved.txt @@ -196,6 +196,33 @@ Nor would this + + + + spec.matches(fakeTestCase("spec . char")) + + + true + + + + + spec.matches(fakeTestCase("spec , char")) + + + true + + + + + !(spec.matches(fakeTestCase(R"(spec \, char)"))) + + + !false + + + +
@@ -16048,7 +16075,7 @@ loose text artifact
- + - + diff --git a/projects/SelfTest/IntrospectiveTests/CmdLine.tests.cpp b/projects/SelfTest/IntrospectiveTests/CmdLine.tests.cpp index 6e590326..0d8232e0 100644 --- a/projects/SelfTest/IntrospectiveTests/CmdLine.tests.cpp +++ b/projects/SelfTest/IntrospectiveTests/CmdLine.tests.cpp @@ -292,6 +292,17 @@ TEST_CASE( "Parse test names and tags", "[command-line][test-spec]" ) { } } +TEST_CASE("#1905 -- test spec parser properly clears internal state between compound tests", "[command-line][test-spec]") { + using Catch::parseTestSpec; + using Catch::TestSpec; + // We ask for one of 2 different tests and the latter one of them has a , in name that needs escaping + TestSpec spec = parseTestSpec(R"("spec . char","spec \, char")"); + + REQUIRE(spec.matches(fakeTestCase("spec . char"))); + REQUIRE(spec.matches(fakeTestCase("spec , char"))); + REQUIRE_FALSE(spec.matches(fakeTestCase(R"(spec \, char)"))); +} + TEST_CASE( "Process can be configured on command line", "[config][command-line]" ) { #ifndef CATCH_CONFIG_DISABLE_MATCHERS