mirror of
				https://github.com/catchorg/Catch2.git
				synced 2025-11-04 05:59:32 +01:00 
			
		
		
		
	Fix potential infinite loops in generators combined with section filter
The problem was that under specific circumstances, namely that none
of their children progressed, `GeneratorTracker` will not progress.
This was changed recently, to allow for code like this, where a
`SECTION` follows a `GENERATE` at the same level:
```cpp
SECTION("A") {}
auto a = GENERATE(1, 2);
SECTION("B") {}
```
However, this interacted badly with `SECTION` filters (`-c foo`),
as they could deactivate all `SECTION`s below a generator, and thus
stop it from progressing forever. This commit makes GeneratorTracker
check whether there are any filters active, and if they are, it checks
whether its section-children can ever run.
Fixes #2025
			
			
This commit is contained in:
		@@ -71,13 +71,53 @@ namespace Catch {
 | 
			
		||||
                // `SECTION`s.
 | 
			
		||||
                // **The check for m_children.empty cannot be removed**.
 | 
			
		||||
                // doing so would break `GENERATE` _not_ followed by `SECTION`s.
 | 
			
		||||
                const bool should_wait_for_child =
 | 
			
		||||
                    !m_children.empty() &&
 | 
			
		||||
                    std::find_if( m_children.begin(),
 | 
			
		||||
                                  m_children.end(),
 | 
			
		||||
                                  []( TestCaseTracking::ITrackerPtr tracker ) {
 | 
			
		||||
                                      return tracker->hasStarted();
 | 
			
		||||
                                  } ) == m_children.end();
 | 
			
		||||
                const bool should_wait_for_child = [&]() {
 | 
			
		||||
                    // No children -> nobody to wait for
 | 
			
		||||
                    if ( m_children.empty() ) {
 | 
			
		||||
                        return false;
 | 
			
		||||
                    }
 | 
			
		||||
                    // If at least one child started executing, don't wait
 | 
			
		||||
                    if ( std::find_if(
 | 
			
		||||
                             m_children.begin(),
 | 
			
		||||
                             m_children.end(),
 | 
			
		||||
                             []( TestCaseTracking::ITrackerPtr tracker ) {
 | 
			
		||||
                                 return tracker->hasStarted();
 | 
			
		||||
                             } ) != m_children.end() ) {
 | 
			
		||||
                        return false;
 | 
			
		||||
                    }
 | 
			
		||||
 | 
			
		||||
                    // No children have started. We need to check if they _can_
 | 
			
		||||
                    // start, and thus we should wait for them, or they cannot
 | 
			
		||||
                    // start (due to filters), and we shouldn't wait for them
 | 
			
		||||
                    auto* parent = m_parent;
 | 
			
		||||
                    // This is safe: there is always at least one section
 | 
			
		||||
                    // tracker in a test case tracking tree
 | 
			
		||||
                    while ( !parent->isSectionTracker() ) {
 | 
			
		||||
                        parent = &( parent->parent() );
 | 
			
		||||
                    }
 | 
			
		||||
                    assert( parent &&
 | 
			
		||||
                            "Missing root (test case) level section" );
 | 
			
		||||
 | 
			
		||||
                    auto const& parentSection =
 | 
			
		||||
                        static_cast<SectionTracker&>( *parent );
 | 
			
		||||
                    auto const& filters = parentSection.getFilters();
 | 
			
		||||
                    // No filters -> no restrictions on running sections
 | 
			
		||||
                    if ( filters.empty() ) {
 | 
			
		||||
                        return true;
 | 
			
		||||
                    }
 | 
			
		||||
 | 
			
		||||
                    for ( auto const& child : m_children ) {
 | 
			
		||||
                        if ( child->isSectionTracker() &&
 | 
			
		||||
                             std::find( filters.begin(),
 | 
			
		||||
                                        filters.end(),
 | 
			
		||||
                                        static_cast<SectionTracker&>( *child )
 | 
			
		||||
                                            .trimmedName() ) !=
 | 
			
		||||
                                 filters.end() ) {
 | 
			
		||||
                            return true;
 | 
			
		||||
                        }
 | 
			
		||||
                    }
 | 
			
		||||
                    return false;
 | 
			
		||||
                }();
 | 
			
		||||
 | 
			
		||||
                // This check is a bit tricky, because m_generator->next()
 | 
			
		||||
                // has a side-effect, where it consumes generator's current
 | 
			
		||||
 
 | 
			
		||||
@@ -233,6 +233,14 @@ namespace TestCaseTracking {
 | 
			
		||||
            m_filters.insert( m_filters.end(), filters.begin()+1, filters.end() );
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    std::vector<std::string> const& SectionTracker::getFilters() const {
 | 
			
		||||
        return m_filters;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    std::string const& SectionTracker::trimmedName() const {
 | 
			
		||||
        return m_trimmed_name;
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
} // namespace TestCaseTracking
 | 
			
		||||
 | 
			
		||||
using TestCaseTracking::ITracker;
 | 
			
		||||
 
 | 
			
		||||
@@ -163,6 +163,10 @@ namespace TestCaseTracking {
 | 
			
		||||
 | 
			
		||||
        void addInitialFilters( std::vector<std::string> const& filters );
 | 
			
		||||
        void addNextFilters( std::vector<std::string> const& filters );
 | 
			
		||||
        //! Returns filters active in this tracker
 | 
			
		||||
        std::vector<std::string> const& getFilters() const;
 | 
			
		||||
        //! Returns whitespace-trimmed name of the tracked section
 | 
			
		||||
        std::string const& trimmedName() const;
 | 
			
		||||
    };
 | 
			
		||||
 | 
			
		||||
} // namespace TestCaseTracking
 | 
			
		||||
 
 | 
			
		||||
@@ -419,6 +419,33 @@ set_tests_properties(FilteredSection-1 PROPERTIES FAIL_REGULAR_EXPRESSION "No te
 | 
			
		||||
add_test(NAME FilteredSection-2 COMMAND $<TARGET_FILE:SelfTest> \#1394\ nested -c NestedRunSection -c s1)
 | 
			
		||||
set_tests_properties(FilteredSection-2 PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran")
 | 
			
		||||
 | 
			
		||||
add_test(
 | 
			
		||||
  NAME
 | 
			
		||||
    FilteredSection::GeneratorsDontCauseInfiniteLoop-1
 | 
			
		||||
  COMMAND
 | 
			
		||||
    $<TARGET_FILE:SelfTest> "#2025: original repro" -c "fov_0"
 | 
			
		||||
)
 | 
			
		||||
set_tests_properties(FilteredSection::GeneratorsDontCauseInfiniteLoop-1
 | 
			
		||||
  PROPERTIES
 | 
			
		||||
    PASS_REGULAR_EXPRESSION "inside with fov: 0" # This should happen
 | 
			
		||||
    FAIL_REGULAR_EXPRESSION "inside with fov: 1" # This would mean there was no filtering
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
# GENERATE between filtered sections (both are selected)
 | 
			
		||||
add_test(
 | 
			
		||||
  NAME
 | 
			
		||||
    FilteredSection::GeneratorsDontCauseInfiniteLoop-2
 | 
			
		||||
  COMMAND
 | 
			
		||||
    $<TARGET_FILE:SelfTest> "#2025: same-level sections"
 | 
			
		||||
    -c "A"
 | 
			
		||||
    -c "B"
 | 
			
		||||
)
 | 
			
		||||
set_tests_properties(FilteredSection::GeneratorsDontCauseInfiniteLoop-2
 | 
			
		||||
  PROPERTIES
 | 
			
		||||
    PASS_REGULAR_EXPRESSION "All tests passed \\(4 assertions in 1 test case\\)"
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
# AppVeyor has a Python 2.7 in path, but doesn't have .py files as autorunnable
 | 
			
		||||
add_test(NAME ApprovalTests COMMAND ${PYTHON_EXECUTABLE} ${CATCH_DIR}/scripts/approvalTests.py $<TARGET_FILE:SelfTest>)
 | 
			
		||||
set_tests_properties(ApprovalTests PROPERTIES FAIL_REGULAR_EXPRESSION "Results differed")
 | 
			
		||||
 
 | 
			
		||||
@@ -402,3 +402,28 @@ TEST_CASE("#1514: stderr/stdout is not captured in tests aborted by an exception
 | 
			
		||||
    // FAIL aborts the test by throwing a Catch exception
 | 
			
		||||
    FAIL("1514");
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
TEST_CASE( "#2025: -c shouldn't cause infinite loop", "[sections][generators][regression][.approvals]" ) {
 | 
			
		||||
    SECTION( "Check cursor from buffer offset" ) {
 | 
			
		||||
        auto bufPos = GENERATE_REF( range( 0, 44 ) );
 | 
			
		||||
        WHEN( "Buffer position is " << bufPos ) { REQUIRE( 1 == 1 ); }
 | 
			
		||||
    }
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
TEST_CASE("#2025: original repro", "[sections][generators][regression][.approvals]") {
 | 
			
		||||
    auto fov = GENERATE(true, false);
 | 
			
		||||
    DYNAMIC_SECTION("fov_" << fov) {
 | 
			
		||||
        std::cout << "inside with fov: " << fov << '\n';
 | 
			
		||||
    }
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
TEST_CASE("#2025: same-level sections", "[sections][generators][regression][.approvals]") {
 | 
			
		||||
    SECTION("A") {
 | 
			
		||||
        SUCCEED();
 | 
			
		||||
    }
 | 
			
		||||
    auto i = GENERATE(1, 2, 3);
 | 
			
		||||
    SECTION("B") {
 | 
			
		||||
        REQUIRE(i < 4);
 | 
			
		||||
    }
 | 
			
		||||
}
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user