From 49d79e9e9c40e644304945dd5a8e4e78d0c61d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Tue, 14 Oct 2025 22:14:47 +0200 Subject: [PATCH] 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 --- src/catch2/internal/catch_run_context.cpp | 8 +- .../internal/catch_test_case_tracker.cpp | 4 +- tests/CMakeLists.txt | 35 ++--- .../IntrospectiveTests/Details.tests.cpp | 62 +++++++++ tests/SelfTest/UsageTests/Tricky.tests.cpp | 18 --- tests/TestScripts/testSectionFiltering.py | 128 ++++++++++++++++++ 6 files changed, 208 insertions(+), 47 deletions(-) create mode 100755 tests/TestScripts/testSectionFiltering.py 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( - *child ) - .trimmedName() ) != - filters.end() ) { + static_cast( *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 $ \#1394 -c RunSection) -set_tests_properties(FilteredSection-1 PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran") -add_test(NAME FilteredSection-2 COMMAND $ \#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 $ \#1394 -c RunSection) +set_tests_properties(FilteredSections::SimpleExample::1 PROPERTIES FAIL_REGULAR_EXPRESSION "No tests ran") +add_test(NAME FilteredSections::SimpleExample::2 COMMAND $ \#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 $ "#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 - $ "#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" $ +) +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 +#include #include #include #include @@ -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)