diff --git a/src/catch2/generators/catch_generators.hpp b/src/catch2/generators/catch_generators.hpp index afb39f86..63bcbf2f 100644 --- a/src/catch2/generators/catch_generators.hpp +++ b/src/catch2/generators/catch_generators.hpp @@ -179,8 +179,7 @@ namespace Generators { bool isFinite() const override { for (auto const& gen : m_generators) { - if (!gen.isFinite()) { return false; - } + if (!gen.isFinite()) { return false; } } return true; } diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp index a435170f..18e01f75 100644 --- a/src/catch2/internal/catch_run_context.cpp +++ b/src/catch2/internal/catch_run_context.cpp @@ -196,6 +196,8 @@ namespace Catch { auto getGenerator() const -> GeneratorBasePtr const& override { return m_generator; } + + bool isFilteredImpl() const override { return m_isFiltered; } }; } // namespace } @@ -540,17 +542,6 @@ namespace Catch { SourceLineInfo lineInfo, Generators::GeneratorBasePtr&& generator ) { - // TBD: Do we want to avoid the warning if the generator is filtered? - if ( m_config->warnAboutInfiniteGenerators() && - !generator->isFinite() ) { - // We want the semantics of `FAIL()`, but we inline it - // to avoid issues with conditionally prefixed macros - INTERNAL_CATCH_MSG( "FAIL", - Catch::ResultWas::ExplicitFailure, - Catch::ResultDisposition::Normal, - "GENERATE() would run infinitely" ); - } - auto nameAndLoc = TestCaseTracking::NameAndLocation( static_cast( generatorName ), lineInfo ); auto& currentTracker = m_trackerContext.currentTracker(); assert( @@ -563,11 +554,24 @@ namespace Catch { m_trackerContext, ¤tTracker, CATCH_MOVE( generator ) ); - auto ret = newTracker.get(); + + // The warning shouldn't fire if the generator is infinite, **but** filtered down. + if ( m_config->warnAboutInfiniteGenerators() && + !newTracker->m_generator->isFinite() && + !newTracker->isFiltered() ) { + // We want the semantics of `FAIL()`, but we inline it + // to avoid issues with conditionally prefixed macros + INTERNAL_CATCH_MSG( "FAIL", + Catch::ResultWas::ExplicitFailure, + Catch::ResultDisposition::Normal, + "GENERATE() would run infinitely" ); + } + + auto returnPtr = newTracker.get(); currentTracker.addChild( CATCH_MOVE( newTracker ) ); - ret->open(); - return ret; + returnPtr->open(); + return returnPtr; } bool RunContext::testForMissingAssertions(Counts& assertions) { diff --git a/src/catch2/internal/catch_test_case_tracker.cpp b/src/catch2/internal/catch_test_case_tracker.cpp index 05bfff8e..864cf11b 100644 --- a/src/catch2/internal/catch_test_case_tracker.cpp +++ b/src/catch2/internal/catch_test_case_tracker.cpp @@ -167,6 +167,28 @@ namespace TestCaseTracking { m_ctx.setCurrentTracker( this ); } + bool SectionTracker::isFilteredImpl() const { + // TBD: This is currently _very_ similar to the block in `isComplete`. + // Is this neccessarily that way, or just accident of current semantics? + const size_t filterIndex = + m_newStyleFilters ? m_allTrackerDepth : m_sectionOnlyDepth; + + if ( filterIndex < m_filterRef->size() ) { + // 1) New style filter must explicitly target section + if ( m_newStyleFilters && ( *m_filterRef )[filterIndex].type != + PathFilter::For::Section ) { + return true; + } + // 2) Both style filters must match the trimmed name exactly + if ( m_trimmed_name != + StringRef( ( *m_filterRef )[filterIndex].filter ) ) { + return true; + } + } + + return false; + } + SectionTracker::SectionTracker( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent ) : TrackerBase( CATCH_MOVE(nameAndLocation), ctx, parent ), m_trimmed_name(trim(StringRef(ITracker::nameAndLocation().name))) diff --git a/src/catch2/internal/catch_test_case_tracker.hpp b/src/catch2/internal/catch_test_case_tracker.hpp index 48700e99..56be0ae1 100644 --- a/src/catch2/internal/catch_test_case_tracker.hpp +++ b/src/catch2/internal/catch_test_case_tracker.hpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -81,6 +82,8 @@ namespace TestCaseTracking { using Children = std::vector; + virtual bool isFilteredImpl() const = 0; + protected: enum CycleState { NotStarted, @@ -178,6 +181,20 @@ namespace TestCaseTracking { * for internal debug checks. */ virtual bool isGeneratorTracker() const; + + /** + * Returns true if the concrete tracker instance has a filter that applies to it. + */ + bool isFiltered() const { + // Fast path: are there even filters for tracker in this position? + const size_t filter_depth = + m_newStyleFilters ? m_allTrackerDepth : m_sectionOnlyDepth; + if ( m_filterRef->size() <= filter_depth ) { return false; } + + // Slow path: If there are filters, ask the concrete tracker. + // This handles things like match-all filters for that tracker. + return isFilteredImpl(); + } }; class TrackerContext { @@ -233,6 +250,8 @@ namespace TestCaseTracking { // to not own the name, the name still has to outlive the `ITracker` parent, so // this should still be safe. StringRef m_trimmed_name; + + bool isFilteredImpl() const override; public: SectionTracker( NameAndLocation&& nameAndLocation, TrackerContext& ctx, ITracker* parent ); diff --git a/tests/ExtraTests/CMakeLists.txt b/tests/ExtraTests/CMakeLists.txt index 5ee07cca..21075a5b 100644 --- a/tests/ExtraTests/CMakeLists.txt +++ b/tests/ExtraTests/CMakeLists.txt @@ -583,16 +583,56 @@ add_executable(InfiniteGenerators ${TESTS_DIR}/X95-InfiniteGenerators.cpp) target_link_libraries(InfiniteGenerators PRIVATE Catch2::Catch2WithMain) add_test( - NAME Warnings::InfiniteGenerators + NAME Warnings::InfiniteGenerators::NoFilterWarns COMMAND $ --warn InfiniteGenerators ) -set_tests_properties(Warnings::InfiniteGenerators +set_tests_properties(Warnings::InfiniteGenerators::NoFilterWarns PROPERTIES # One test case fails with infinite generator, but the other one runs PASS_REGULAR_EXPRESSION "test cases: 2 \\| 1 passed \\| 1 failed" TIMEOUT 5 ) +add_test( + NAME Warnings::InfiniteGenerators::MatchAllFilterWarns + COMMAND $ + --warn InfiniteGenerators + --path-filter g:* +) +set_tests_properties(Warnings::InfiniteGenerators::MatchAllFilterWarns + PROPERTIES + # One test case fails with infinite generator, but the other one runs + PASS_REGULAR_EXPRESSION "test cases: 2 \\| 1 passed \\| 1 failed" + TIMEOUT 5 +) + +add_test( + NAME Warnings::InfiniteGenerators::MatchOneFilterDoesntWarn + COMMAND $ + --warn InfiniteGenerators + --path-filter g:1 +) +set_tests_properties(Warnings::InfiniteGenerators::MatchOneFilterDoesntWarn + PROPERTIES + # One test case fails with infinite generator, but the other one runs + PASS_REGULAR_EXPRESSION "All tests passed \\(1 assertion in 2 test cases\\)" + TIMEOUT 5 +) + +add_test( + NAME Warnings::InfiniteGenerators::SectionFilterWarns + COMMAND $ + --warn InfiniteGenerators + --section FooBarBaz +) +set_tests_properties(Warnings::InfiniteGenerators::SectionFilterWarns + PROPERTIES + # One test case fails with infinite generator, but the other one runs + PASS_REGULAR_EXPRESSION "test cases: 2 \\| 1 passed \\| 1 failed" + TIMEOUT 5 +) + + set(EXTRA_TEST_BINARIES AllSkipped PrefixedMacros