Fix --warn InfiniteGenerators firing even if the generator was filtered

Author: horenmar

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;
         }

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<std::string>( generatorName ), lineInfo );
         auto& currentTracker = m_trackerContext.currentTracker();
         assert(
@@ -563,11 +554,24 @@ namespace Catch {
                 m_trackerContext,
                 &currentTracker,
                 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) {

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)))

src/catch2/internal/catch_test_case_tracker.hpp

@@ -12,6 +12,7 @@
 #include <catch2/internal/catch_source_line_info.hpp>
 #include <catch2/internal/catch_unique_ptr.hpp>
 #include <catch2/internal/catch_stringref.hpp>
+#include <catch2/internal/catch_path_filter.hpp>
 
 #include <string>
 #include <vector>
@@ -81,6 +82,8 @@ namespace TestCaseTracking {
 
         using Children = std::vector<ITrackerPtr>;
 
+        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 );
 

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 $<TARGET_FILE:InfiniteGenerators> --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 $<TARGET_FILE:InfiniteGenerators>
+    --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 $<TARGET_FILE:InfiniteGenerators>
+    --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 $<TARGET_FILE:InfiniteGenerators>
+    --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