From 4f47d1c6c1bf21e047fc4dd8a946740a2da2f673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Sat, 22 Jun 2019 20:26:03 +0200 Subject: [PATCH] Hidden tests now require positive filter match to be selected This also required some refactoring of how the pattern matching works. This means that the concepts of include and exclude patterns are no longer unified, with exclusion patterns working as just negation of an inclusion patterns (which led to including hidden tags by default, as they did not match the exclusion), but rather both include and exclude patterns are handled separately. The new logic is that given a filter and a test case, the test case must match _all_ include patterns and _no_ exclude patterns to be included by the filter. Furthermore, if the test case is hidden, then the filter must have at least one include pattern for the test case to be used. Closes #1184 --- docs/deprecations.md | 14 -------- docs/release-notes.md | 4 +++ include/internal/catch_test_spec.cpp | 33 +++++++++++-------- include/internal/catch_test_spec.h | 11 ++----- include/internal/catch_test_spec_parser.cpp | 2 +- include/internal/catch_test_spec_parser.h | 14 ++++---- projects/CMakeLists.txt | 3 ++ .../Baselines/compact.sw.approved.txt | 4 +-- .../Baselines/console.std.approved.txt | 2 +- .../Baselines/console.sw.approved.txt | 10 +++--- .../Baselines/console.swa4.approved.txt | 2 +- .../SelfTest/Baselines/junit.sw.approved.txt | 2 +- .../SelfTest/Baselines/xml.sw.approved.txt | 10 +++--- .../IntrospectiveTests/CmdLine.tests.cpp | 6 ++-- scripts/approvalTests.py | 15 +++++---- 15 files changed, 63 insertions(+), 69 deletions(-) diff --git a/docs/deprecations.md b/docs/deprecations.md index 2cf77279..643c73ce 100644 --- a/docs/deprecations.md +++ b/docs/deprecations.md @@ -27,20 +27,6 @@ will be added), which means that their failure will not fail the test, making the `else` actually useful. -### Change semantics of `[.]` and tag exclusion - -Currently, given these 2 tests -```cpp -TEST_CASE("A", "[.][foo]") {} -TEST_CASE("B", "[.][bar]") {} -``` -specifying `[foo]` as the testspec will run test "A" and specifying -`~[foo]` will run test "B", even though it is hidden. Also, specifying -`~[baz]` will run both tests. This behaviour is often surprising and will -be changed so that hidden tests are included in a run only if they -positively match a testspec. - - ### Console Colour API The API for Catch2's console colour will be changed to take an extra diff --git a/docs/release-notes.md b/docs/release-notes.md index 8be1bbd5..4d0527a0 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -42,6 +42,10 @@ * XmlReporter outputs a machine-parseable XML * `TEST_CASE` description support has been removed * If the second argument has text outside tags, the text will be ignored. +* Hidden test cases are no longer included just because they don't match an exclusion tag + * Previously, a `TEST_CASE("A", "[.foo]")` would be included by asking for `~[bar]`. + + ### Fixes * The `INFO` macro no longer contains superfluous semicolon (#1456) diff --git a/include/internal/catch_test_spec.cpp b/include/internal/catch_test_spec.cpp index 65d34d0e..4610c1a9 100644 --- a/include/internal/catch_test_spec.cpp +++ b/include/internal/catch_test_spec.cpp @@ -48,25 +48,30 @@ namespace Catch { m_tag) != end(testCase.lcaseTags); } - - TestSpec::ExcludedPattern::ExcludedPattern( PatternPtr const& underlyingPattern ) - : Pattern( underlyingPattern->name() ) - , m_underlyingPattern( underlyingPattern ) - {} - - bool TestSpec::ExcludedPattern::matches( TestCaseInfo const& testCase ) const { - return !m_underlyingPattern->matches( testCase ); - } - - bool TestSpec::Filter::matches( TestCaseInfo const& testCase ) const { - return std::all_of( m_patterns.begin(), m_patterns.end(), [&]( PatternPtr const& p ){ return p->matches( testCase ); } ); + bool should_use = !testCase.isHidden(); + for (auto const& pattern : m_required) { + should_use = true; + if (!pattern->matches(testCase)) { + return false; + } + } + for (auto const& pattern : m_forbidden) { + if (pattern->matches(testCase)) { + return false; + } + } + return should_use; } std::string TestSpec::Filter::name() const { std::string name; - for( auto const& p : m_patterns ) + for (auto const& p : m_required) { name += p->name(); + } + for (auto const& p : m_forbidden) { + name += p->name(); + } return name; } @@ -91,7 +96,7 @@ namespace Catch { } ); return matches; } - + const TestSpec::vectorStrings& TestSpec::getInvalidArgs() const{ return (m_invalidArgs); } diff --git a/include/internal/catch_test_spec.h b/include/internal/catch_test_spec.h index 2e546463..7196b7c5 100644 --- a/include/internal/catch_test_spec.h +++ b/include/internal/catch_test_spec.h @@ -52,16 +52,9 @@ namespace Catch { std::string m_tag; }; - class ExcludedPattern : public Pattern { - public: - explicit ExcludedPattern( PatternPtr const& underlyingPattern ); - bool matches( TestCaseInfo const& testCase ) const override; - private: - PatternPtr m_underlyingPattern; - }; - struct Filter { - std::vector m_patterns; + std::vector m_required; + std::vector m_forbidden; bool matches( TestCaseInfo const& testCase ) const; std::string name() const; diff --git a/include/internal/catch_test_spec_parser.cpp b/include/internal/catch_test_spec_parser.cpp index b872b191..dac9b584 100644 --- a/include/internal/catch_test_spec_parser.cpp +++ b/include/internal/catch_test_spec_parser.cpp @@ -147,7 +147,7 @@ namespace Catch { } void TestSpecParser::addFilter() { - if( !m_currentFilter.m_patterns.empty() ) { + if( !m_currentFilter.m_required.empty() || !m_currentFilter.m_forbidden.empty() ) { m_testSpec.m_filters.push_back( m_currentFilter ); m_currentFilter = TestSpec::Filter(); } diff --git a/include/internal/catch_test_spec_parser.h b/include/internal/catch_test_spec_parser.h index 179d3532..05882377 100644 --- a/include/internal/catch_test_spec_parser.h +++ b/include/internal/catch_test_spec_parser.h @@ -53,7 +53,7 @@ namespace Catch { void revertBackToLastMode(); void addFilter(); bool separate(); - + template void addPattern() { std::string token = m_patternName; @@ -66,22 +66,24 @@ namespace Catch { } if( !token.empty() ) { TestSpec::PatternPtr pattern = std::make_shared( token, m_substring ); - if( m_exclusion ) - pattern = std::make_shared( pattern ); - m_currentFilter.m_patterns.push_back( pattern ); + if (m_exclusion) { + m_currentFilter.m_forbidden.push_back(pattern); + } else { + m_currentFilter.m_required.push_back(pattern); + } } m_substring.clear(); m_patternName.clear(); m_exclusion = false; m_mode = None; } - + inline void addCharToPattern(char c) { m_substring += c; m_patternName += c; m_realPatternPos++; } - + }; TestSpec parseTestSpec( std::string const& arg ); diff --git a/projects/CMakeLists.txt b/projects/CMakeLists.txt index 6d068703..729edd2d 100644 --- a/projects/CMakeLists.txt +++ b/projects/CMakeLists.txt @@ -455,6 +455,9 @@ set_tests_properties(FilenameAsTagsTest PROPERTIES PASS_REGULAR_EXPRESSION "\\[# add_test(NAME EscapeSpecialCharactersInTestNames COMMAND $ "Test with special\\, characters \"in name") set_tests_properties(EscapeSpecialCharactersInTestNames PROPERTIES PASS_REGULAR_EXPRESSION "1 assertion in 1 test case") +add_test(NAME NegativeSpecNoHiddenTests COMMAND $ --list-tests ~[approval]) +set_tests_properties(NegativeSpecNoHiddenTests PROPERTIES FAIL_REGULAR_EXPRESSION "\\[\\.\\]") + add_test(NAME TestsInFile::SimpleSpecs COMMAND $ "-f ${CATCH_DIR}/projects/SelfTest/Misc/plain-old-tests.input") set_tests_properties(TestsInFile::SimpleSpecs PROPERTIES PASS_REGULAR_EXPRESSION "6 assertions in 2 test cases") diff --git a/projects/SelfTest/Baselines/compact.sw.approved.txt b/projects/SelfTest/Baselines/compact.sw.approved.txt index a65f15ab..bf3d16c6 100644 --- a/projects/SelfTest/Baselines/compact.sw.approved.txt +++ b/projects/SelfTest/Baselines/compact.sw.approved.txt @@ -962,7 +962,7 @@ CmdLine.tests.cpp:: passed: spec.matches( tcD ) == false for: false CmdLine.tests.cpp:: passed: spec.hasFilters() == true for: true == true CmdLine.tests.cpp:: passed: spec.matches( tcA ) == true for: true == true CmdLine.tests.cpp:: passed: spec.matches( tcB ) == false for: false == false -CmdLine.tests.cpp:: passed: spec.matches( tcC ) == true for: true == true +CmdLine.tests.cpp:: passed: spec.matches( tcC ) == false for: false == false CmdLine.tests.cpp:: passed: spec.hasFilters() == true for: true == true CmdLine.tests.cpp:: passed: spec.matches( tcA ) == false for: false == false CmdLine.tests.cpp:: passed: spec.matches( tcB ) == true for: true == true @@ -980,7 +980,7 @@ CmdLine.tests.cpp:: passed: spec.matches( tcD ) == true for: true = CmdLine.tests.cpp:: passed: spec.hasFilters() == true for: true == true CmdLine.tests.cpp:: passed: spec.matches( tcA ) == true for: true == true CmdLine.tests.cpp:: passed: spec.matches( tcB ) == false for: false == false -CmdLine.tests.cpp:: passed: spec.matches( tcC ) == true for: true == true +CmdLine.tests.cpp:: passed: spec.matches( tcC ) == false for: false == false CmdLine.tests.cpp:: passed: spec.matches( tcD ) == true for: true == true CmdLine.tests.cpp:: passed: spec.hasFilters() == true for: true == true CmdLine.tests.cpp:: passed: spec.matches( tcA ) == true for: true == true diff --git a/projects/SelfTest/Baselines/console.std.approved.txt b/projects/SelfTest/Baselines/console.std.approved.txt index d43063fe..2fb24b6f 100644 --- a/projects/SelfTest/Baselines/console.std.approved.txt +++ b/projects/SelfTest/Baselines/console.std.approved.txt @@ -1,4 +1,4 @@ -Filters: ~[!nonportable]~[!benchmark]~[approvals] +Filters: ~[!nonportable]~[!benchmark]~[approvals] * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ is a host application. diff --git a/projects/SelfTest/Baselines/console.sw.approved.txt b/projects/SelfTest/Baselines/console.sw.approved.txt index d321bd91..b7167e98 100644 --- a/projects/SelfTest/Baselines/console.sw.approved.txt +++ b/projects/SelfTest/Baselines/console.sw.approved.txt @@ -1,4 +1,4 @@ -Filters: ~[!nonportable]~[!benchmark]~[approvals] +Filters: ~[!nonportable]~[!benchmark]~[approvals] * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ is a host application. @@ -6946,9 +6946,9 @@ with expansion: false == false CmdLine.tests.cpp:: PASSED: - CHECK( spec.matches( tcC ) == true ) + CHECK( spec.matches( tcC ) == false ) with expansion: - true == true + false == false ------------------------------------------------------------------------------- Parse test names and tags @@ -7064,9 +7064,9 @@ with expansion: false == false CmdLine.tests.cpp:: PASSED: - CHECK( spec.matches( tcC ) == true ) + CHECK( spec.matches( tcC ) == false ) with expansion: - true == true + false == false CmdLine.tests.cpp:: PASSED: CHECK( spec.matches( tcD ) == true ) diff --git a/projects/SelfTest/Baselines/console.swa4.approved.txt b/projects/SelfTest/Baselines/console.swa4.approved.txt index 322b8158..0c1ab7b7 100644 --- a/projects/SelfTest/Baselines/console.swa4.approved.txt +++ b/projects/SelfTest/Baselines/console.swa4.approved.txt @@ -1,4 +1,4 @@ -Filters: ~[!nonportable]~[!benchmark]~[approvals] +Filters: ~[!nonportable]~[!benchmark]~[approvals] * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ is a host application. diff --git a/projects/SelfTest/Baselines/junit.sw.approved.txt b/projects/SelfTest/Baselines/junit.sw.approved.txt index 588c3a21..814d05ee 100644 --- a/projects/SelfTest/Baselines/junit.sw.approved.txt +++ b/projects/SelfTest/Baselines/junit.sw.approved.txt @@ -3,7 +3,7 @@ > - + diff --git a/projects/SelfTest/Baselines/xml.sw.approved.txt b/projects/SelfTest/Baselines/xml.sw.approved.txt index 35640250..8c95f95b 100644 --- a/projects/SelfTest/Baselines/xml.sw.approved.txt +++ b/projects/SelfTest/Baselines/xml.sw.approved.txt @@ -1,5 +1,5 @@ - + @@ -8720,10 +8720,10 @@ Nor would this - spec.matches( tcC ) == true + spec.matches( tcC ) == false - true == true + false == false @@ -8876,10 +8876,10 @@ Nor would this - spec.matches( tcC ) == true + spec.matches( tcC ) == false - true == true + false == false diff --git a/projects/SelfTest/IntrospectiveTests/CmdLine.tests.cpp b/projects/SelfTest/IntrospectiveTests/CmdLine.tests.cpp index 60eb97af..8fccc0ab 100644 --- a/projects/SelfTest/IntrospectiveTests/CmdLine.tests.cpp +++ b/projects/SelfTest/IntrospectiveTests/CmdLine.tests.cpp @@ -173,7 +173,7 @@ TEST_CASE( "Parse test names and tags" ) { CHECK( spec.hasFilters() == true ); CHECK( spec.matches( tcA ) == true ); CHECK( spec.matches( tcB ) == false ); - CHECK( spec.matches( tcC ) == true ); + CHECK( spec.matches( tcC ) == false ); } SECTION( "One tag exclusion and one tag inclusion" ) { TestSpec spec = parseTestSpec( "~[two][x]" ); @@ -203,7 +203,7 @@ TEST_CASE( "Parse test names and tags" ) { CHECK( spec.hasFilters() == true ); CHECK( spec.matches( tcA ) == true ); CHECK( spec.matches( tcB ) == false ); - CHECK( spec.matches( tcC ) == true ); + CHECK( spec.matches( tcC ) == false ); CHECK( spec.matches( tcD ) == true ); } SECTION( "wildcarded name exclusion" ) { @@ -486,7 +486,7 @@ TEST_CASE( "Process can be configured on command line", "[config][command-line]" REQUIRE(config.benchmarkSamples == 200); } - + SECTION("resamples") { CHECK(cli.parse({ "test", "--benchmark-resamples=20000" })); diff --git a/scripts/approvalTests.py b/scripts/approvalTests.py index dad2a96b..e4858178 100755 --- a/scripts/approvalTests.py +++ b/scripts/approvalTests.py @@ -195,19 +195,20 @@ print(" " + cmdPath) # ## Keep default reporters here ## # Standard console reporter -approve("console.std", ["~[!nonportable]~[!benchmark]~[approvals]", "--order", "lex", "--rng-seed", "1"]) +approve("console.std", ["~[!nonportable]~[!benchmark]~[approvals] *", "--order", "lex", "--rng-seed", "1"]) # console reporter, include passes, warn about No Assertions -approve("console.sw", ["~[!nonportable]~[!benchmark]~[approvals]", "-s", "-w", "NoAssertions", "--order", "lex", "--rng-seed", "1"]) +approve("console.sw", ["~[!nonportable]~[!benchmark]~[approvals] *", "-s", "-w", "NoAssertions", "--order", "lex", "--rng-seed", "1"]) # console reporter, include passes, warn about No Assertions, limit failures to first 4 -approve("console.swa4", ["~[!nonportable]~[!benchmark]~[approvals]", "-s", "-w", "NoAssertions", "-x", "4", "--order", "lex", "--rng-seed", "1"]) +approve("console.swa4", ["~[!nonportable]~[!benchmark]~[approvals] *", "-s", "-w", "NoAssertions", "-x", "4", "--order", "lex", "--rng-seed", "1"]) # junit reporter, include passes, warn about No Assertions -approve("junit.sw", ["~[!nonportable]~[!benchmark]~[approvals]", "-s", "-w", "NoAssertions", "-r", "junit", "--order", "lex", "--rng-seed", "1"]) +approve("junit.sw", ["~[!nonportable]~[!benchmark]~[approvals] *", "-s", "-w", "NoAssertions", "-r", "junit", "--order", "lex", "--rng-seed", "1"]) # xml reporter, include passes, warn about No Assertions -approve("xml.sw", ["~[!nonportable]~[!benchmark]~[approvals]", "-s", "-w", "NoAssertions", "-r", "xml", "--order", "lex", "--rng-seed", "1"]) +approve("xml.sw", ["~[!nonportable]~[!benchmark]~[approvals] *", "-s", "-w", "NoAssertions", "-r", "xml", "--order", "lex", "--rng-seed", "1"]) # compact reporter, include passes, warn about No Assertions -approve('compact.sw', ['~[!nonportable]~[!benchmark]~[approvals]', '-s', '-w', 'NoAssertions', '-r', 'compact', '--order', 'lex', "--rng-seed", "1"]) +approve('compact.sw', ['~[!nonportable]~[!benchmark]~[approvals] *', '-s', '-w', 'NoAssertions', '-r', 'compact', '--order', 'lex', "--rng-seed", "1"]) # sonarqube reporter, include passes, warn about No Assertions -approve("sonarqube.sw", ["~[!nonportable]~[!benchmark]~[approvals]", "-s", "-w", "NoAssertions", "-r", "sonarqube", "--order", "lex", "--rng-seed", "1"]) +approve("sonarqube.sw", ["~[!nonportable]~[!benchmark]~[approvals] *", "-s", "-w", "NoAssertions", "-r", "sonarqube", "--order", "lex", "--rng-seed", "1"]) + if overallResult != 0: print("If these differences are expected, run approve.py to approve new baselines.")