Fix section filtering to make sense

Specifically, this commit makes the `-c`/`--section` parameter
strictly ordered and hierarchical, unlike how it behaved before,
which was a huge mess -- see #3038 for details.

Closes #3038

6 files changed, 208 insertions, 47 deletions

diff --git a/src/catch2/internal/catch_run_context.cpp b/src/catch2/internal/catch_run_context.cpp
index 0029bf56..37045f60 100644
--- a/src/catch2/internal/catch_run_context.cpp
+++ b/src/catch2/internal/catch_run_context.cpp
@@ -129,12 +129,8 @@ namespace Catch {
 
                         for ( auto const& child : m_children ) {
                             if ( child->isSectionTracker() &&
-                                 std::find( filters.begin(),
-                                            filters.end(),
-                                            static_cast<SectionTracker const&>(
-                                                *child )
-                                                .trimmedName() ) !=
-                                     filters.end() ) {
+                                 static_cast<SectionTracker const&>( *child )
+                                         .trimmedName() == filters[0] ) {
                                 return true;
                             }
                         }
diff --git a/src/catch2/internal/catch_test_case_tracker.cpp b/src/catch2/internal/catch_test_case_tracker.cpp
index 1470b91c..541910d0 100644
--- a/src/catch2/internal/catch_test_case_tracker.cpp
+++ b/src/catch2/internal/catch_test_case_tracker.cpp
@@ -172,9 +172,9 @@ namespace TestCaseTracking {
     bool SectionTracker::isComplete() const {
         bool complete = true;
 
-        if (m_filters.empty()
+        if ( m_filters.empty()
             || m_filters[0].empty()
-            || std::find(m_filters.begin(), m_filters.end(), m_trimmed_name) != m_filters.end()) {
+            || m_filters[0] == m_trimmed_name ) {
             complete = TrackerBase::isComplete();
         }
         return complete;
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 651ac835..5bac7090 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -310,38 +310,23 @@ set_tests_properties(UnmatchedOutputFilter
     PASS_REGULAR_EXPRESSION "No test cases matched '\\[this-tag-does-not-exist\\]'"
 )
 
-add_test(NAME FilteredSection-1 COMMAND $<TARGET_FILE:SelfTest> \#1394 -c RunSection)
-set_tests_properties(FilteredSection-1 PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran")
-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 FilteredSections::SimpleExample::1 COMMAND $<TARGET_FILE:SelfTest> \#1394 -c RunSection)
+set_tests_properties(FilteredSections::SimpleExample::1 PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran")
+add_test(NAME FilteredSections::SimpleExample::2 COMMAND $<TARGET_FILE:SelfTest> \#1394\ nested -c NestedRunSection -c s1)
+set_tests_properties(FilteredSections::SimpleExample::2 PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran")
 
 add_test(
   NAME
-    FilteredSection::GeneratorsDontCauseInfiniteLoop-1
+    FilteredSections::GeneratorsDontCauseInfiniteLoop
   COMMAND
     $<TARGET_FILE:SelfTest> "#2025: original repro" -c "fov_0"
 )
-set_tests_properties(FilteredSection::GeneratorsDontCauseInfiniteLoop-1
+set_tests_properties(FilteredSections::GeneratorsDontCauseInfiniteLoop
   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"
-    --colour-mode none
-)
-set_tests_properties(FilteredSection::GeneratorsDontCauseInfiniteLoop-2
-  PROPERTIES
-    PASS_REGULAR_EXPRESSION "All tests passed \\(4 assertions in 1 test case\\)"
-)
-
 add_test(NAME ApprovalTests
   COMMAND
     Python3::Interpreter
@@ -694,6 +679,14 @@ set_tests_properties("Bazel::RngSeedEnvVar::MalformedValueIsIgnored"
     PASS_REGULAR_EXPRESSION "Randomness seeded to: 17171717"
 )
 
+add_test(NAME "FilteredSections::DifferentSimpleFilters"
+  COMMAND
+      Python3::Interpreter "${CMAKE_CURRENT_LIST_DIR}/TestScripts/testSectionFiltering.py" $<TARGET_FILE:SelfTest>
+)
+set_tests_properties("FilteredSections::DifferentSimpleFilters"
+  PROPERTIES
+    LABELS "uses-python"
+)
 
 list(APPEND CATCH_TEST_TARGETS SelfTest)
 set(CATCH_TEST_TARGETS ${CATCH_TEST_TARGETS} PARENT_SCOPE)
diff --git a/tests/SelfTest/IntrospectiveTests/Details.tests.cpp b/tests/SelfTest/IntrospectiveTests/Details.tests.cpp
index 5566bb59..d968d942 100644
--- a/tests/SelfTest/IntrospectiveTests/Details.tests.cpp
+++ b/tests/SelfTest/IntrospectiveTests/Details.tests.cpp
@@ -7,6 +7,7 @@
 // SPDX-License-Identifier: BSL-1.0
 
 #include <catch2/catch_test_macros.hpp>
+#include <catch2/generators/catch_generators.hpp>
 #include <catch2/internal/catch_enforce.hpp>
 #include <catch2/internal/catch_case_insensitive_comparisons.hpp>
 #include <catch2/internal/catch_optional.hpp>
@@ -170,3 +171,64 @@ TEST_CASE( "Decomposer checks that the argument is 0 when handling "
 
     CATCH_INTERNAL_STOP_WARNINGS_SUPPRESSION
 }
+
+
+TEST_CASE( "foo", "[approvals]" ) {
+    SECTION( "A" ) {
+        SECTION( "B1" ) { REQUIRE( true ); }
+        SECTION( "B2" ) { REQUIRE( true ); }
+        SECTION( "B3" ) { REQUIRE( true ); }
+    }
+}
+
+TEST_CASE( "bar", "[approvals]" ) {
+    REQUIRE( true );
+    SECTION( "A" ) {
+        SECTION( "B1" ) { REQUIRE( true ); }
+        SECTION( "B2" ) { REQUIRE( true ); }
+        SECTION( "B3" ) { REQUIRE( true ); }
+    }
+    REQUIRE( true );
+}
+
+TEST_CASE( "baz", "[approvals]" ) {
+    SECTION( "A" ) { REQUIRE( true ); }
+    auto _ = GENERATE( 1, 2, 3 );
+    (void)_;
+    SECTION( "B" ) { REQUIRE( true ); }
+}
+
+TEST_CASE( "qux", "[approvals]" ) {
+    REQUIRE( true );
+    SECTION( "A" ) { REQUIRE( true ); }
+    auto _ = GENERATE( 1, 2, 3 );
+    (void)_;
+    SECTION( "B" ) { REQUIRE( true ); }
+    REQUIRE( true );
+}
+
+TEST_CASE( "corge", "[approvals]" ) {
+    REQUIRE( true );
+    SECTION( "A" ) {
+        REQUIRE( true );
+    }
+    auto i = GENERATE( 1, 2, 3 );
+    DYNAMIC_SECTION( "i=" << i ) {
+        REQUIRE( true );
+    }
+    REQUIRE( true );
+}
+
+TEST_CASE("grault", "[approvals]") {
+    REQUIRE( true );
+    SECTION( "A" ) {
+        REQUIRE( true );
+    }
+    SECTION("B") {
+        auto i = GENERATE( 1, 2, 3 );
+        DYNAMIC_SECTION( "i=" << i ) {
+            REQUIRE( true );
+        }
+    }
+    REQUIRE( true );
+}
diff --git a/tests/SelfTest/UsageTests/Tricky.tests.cpp b/tests/SelfTest/UsageTests/Tricky.tests.cpp
index 041d7867..fe93e8cf 100644
--- a/tests/SelfTest/UsageTests/Tricky.tests.cpp
+++ b/tests/SelfTest/UsageTests/Tricky.tests.cpp
@@ -354,27 +354,9 @@ TEST_CASE("#1514: stderr/stdout is not captured in tests aborted by an 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);
-    }
-}
diff --git a/tests/TestScripts/testSectionFiltering.py b/tests/TestScripts/testSectionFiltering.py
new file mode 100755
index 00000000..e4f33ad0
--- /dev/null
+++ b/tests/TestScripts/testSectionFiltering.py
@@ -0,0 +1,128 @@
+#!/usr/bin/env python3
+
+#              Copyright Catch2 Authors
+# Distributed under the Boost Software License, Version 1.0.
+#   (See accompanying file LICENSE.txt or copy at
+#        https://www.boost.org/LICENSE_1_0.txt)
+
+# SPDX-License-Identifier: BSL-1.0
+
+"""
+This test script verifies the behaviour of the legacy section filtering
+using `-c`, `--section` CLI parameters.
+
+This is done by having a hardcoded set of test filter + section filter
+combinations, together with the expected number of assertions that will
+be run inside the test for given filter combo.
+"""
+
+import os
+import subprocess
+import sys
+from typing import Tuple, List
+import xml.etree.ElementTree as ET
+
+def make_cli_filter(section_names: Tuple[str, ...]) -> List[str]:
+    final = []
+    for name in section_names:
+        final.append('--section')
+        final.append(name)
+    return final
+
+def run_one_test(binary_path: str, test_name: str, section_names: Tuple[str, ...], expected_assertions: int):
+    cmd = [
+      binary_path,
+      '--reporter', 'xml',
+      test_name
+    ]
+    cmd.extend(make_cli_filter(section_names))
+    try:
+        ret = subprocess.run(
+            cmd,
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            check=True,
+            universal_newlines=True,
+        )
+        stdout = ret.stdout
+    except subprocess.SubprocessError as ex:
+        print('Could not run "{}"'.format(cmd))
+        print("Return code: {}".format(ex.returncode))
+        print("stdout: {}".format(ex.stdout))
+        print("stderr: {}".format(ex.stderr))
+        raise
+
+    try:
+        tree = ET.fromstring(stdout)
+    except ET.ParseError as ex:
+        print("Invalid XML: '{}'".format(ex))
+        raise
+
+    # Validate that we ran exactly 1 test case, and it passed
+    test_case_stats = tree.find('OverallResultsCases')
+    expected_testcases = {'successes' : '1', 'failures' : '0', 'expectedFailures': '0', 'skips': '0'}
+    assert test_case_stats.attrib == expected_testcases, f'We did not run single passing test case as expected. {test_name}: {test_case_stats.attrib}'
+
+    # Validate that we got exactly the expected number of passing assertions
+    expected_assertions = {'successes' : str(expected_assertions), 'failures' : '0', 'expectedFailures': '0', 'skips': '0'}
+    assertion_stats = tree.find('OverallResults')
+    assert assertion_stats.attrib == expected_assertions, f'"{test_name}": {assertion_stats.attrib} vs {expected_assertions}'
+
+
+# Inputs taken from issue #3038
+tests = {
+  'foo': (
+    ((), 3),
+    (('A',), 3),
+    (('A', 'B'), 0),
+    (('A', 'B1'), 1),
+    (('A', 'B2'), 1),
+    (('A', 'B1', 'B2'), 1),
+    (('A', 'B2', 'XXXX'), 1),
+  ),
+  'bar': (
+    ((), 9),
+    (('A',), 9),
+    (('A', 'B1'), 3),
+    (('XXXX',), 2),
+    (('B1',), 2),
+    (('A', 'B1', 'B2'), 3),
+  ),
+  'baz': (
+    ((), 4),
+    (('A',), 1),
+    (('A', 'B'), 1),
+    (('A', 'XXXX'), 1),
+    (('B',), 3),
+    (('XXXX',), 0),
+  ),
+  'qux': (
+    ((), 12),
+    (('A',), 7),
+    (('B',), 9),
+    (('B', 'XXXX'), 9),
+    (('XXXX',), 6),
+  ),
+  'corge': (
+    ((), 12),
+    (('i=2',), 7),
+    (('i=3',), 7),
+  ),
+  'grault': (
+    ((), 12),
+    (('A',), 3),
+    (('B',), 9),
+    (('B', 'i=1'), 7),
+    (('B', 'XXXX'), 6),
+  ),
+}
+
+if len(sys.argv) != 2:
+    print("Wrong number of arguments, expected just the path to Catch2 SelfTest binary")
+    exit(1)
+
+bin_path = os.path.abspath(sys.argv[1])
+
+for test_filter, specs in tests.items():
+    for section_path, expected_assertions in specs:
+        run_one_test(bin_path, test_filter, section_path, expected_assertions)