Skip to content

Section path filtering is hilarious broken #3038

@horenmar

Description

@horenmar

When things work as expected

The test in question

TEST_CASE( "TRY FILTERS" ) {
    SECTION( "A" ) {
        SECTION( "B" ) {
            SECTION( "C1" ) {
                SECTION( "D" ) { REQUIRE( true ); }
            }
            SECTION( "C2" ) {
                SECTION( "D" ) { REQUIRE( true ); }
            }
        }
    }
}

Usage examples

Section path prefix filter

./tests/SelfTest "TRY FILTERS" -c "A" -c "B"
Filters: "TRY FILTERS"
Randomness seeded to: 2698325630
===============================================================================
All tests passed (2 assertions in 1 test case)

This works as expected, that is, we run both assertions, because the section
path prefix we specified contains both of them.

Disambiguated section path prefix filter

./tests/SelfTest "TRY FILTERS" -c "A" -c "B" -c "C1"
Filters: "TRY FILTERS"
Randomness seeded to: 4049078214
===============================================================================
All tests passed (1 assertion in 1 test case)

We disambiguated the two "C" sections, and only run 1 assertion.

Bad section name in section filter

$ ./tests/SelfTest "TRY FILTERS" -c "A" -c "ZZZZZZZZ" -c "C1"
Filters: "TRY FILTERS"
Randomness seeded to: 4083994241
===============================================================================
test cases: 1 | 1 passed
assertions: - none -

No assertions run, because the "ZZZZZZZZ" filter doesn't pass through
the "B" section.

Missing part of the section filter

$ ./tests/SelfTest "TRY FILTERS" -c "A" -c "C1"
Filters: "TRY FILTERS"
Randomness seeded to: 134702392
===============================================================================
test cases: 1 | 1 passed
assertions: - none -

No assertions run, because there is no "B" in our filter.

So far, so good. So what are the broken parts?

When things get broken

We are still using the same example test case:

TEST_CASE( "TRY FILTERS" ) {
    SECTION( "A" ) {
        SECTION( "B" ) {
            SECTION( "C1" ) {
                SECTION( "D" ) { REQUIRE( true ); }
            }
            SECTION( "C2" ) {
                SECTION( "D" ) { REQUIRE( true ); }
            }
        }
    }
}

Overly long section filter

$ ./tests/SelfTest "TRY FILTERS" -c "A" -c "B" -c "C1" -c "C2" -c "D"
Filters: "TRY FILTERS"
Randomness seeded to: 3171617314
===============================================================================
All tests passed (2 assertions in 1 test case)

We ended up running 2 assertions, because both of the "C" sections are
entered, even though they do not form a linear path.

Note that for extra confusion, omitting the -c "D" at the end runs no
assertions, contrary to the prefix-path behaviour from the previous section:

$ ./tests/SelfTest "TRY FILTERS" -c "A" -c "B" -c "C1" -c "C2"
Filters: "TRY FILTERS"
Randomness seeded to: 2012568784
===============================================================================
test cases: 1 | 1 passed
assertions: - none -

Random noise in the section filter

$ ./tests/SelfTest "TRY FILTERS" -c "A" -c "B" -c "C1"  -c "LOL" -c "LMAO" -c "ROFL" -c "D"
Filters: "TRY FILTERS"
Randomness seeded to: 2513554073
===============================================================================
All tests passed (1 assertion in 1 test case)

The "LOL", "LMAO", and "ROFL" filters are 'ignored' and the assertion
inside the "C1" section is run. Note that as before, the -c "D" filter
at the end is required for the assertion to be run.

Section filter in wrong order

$ ./tests/SelfTest "TRY FILTERS" -c "LOL" -c "LMAO" -c "ROFL" -c "D" -c "C1" -c "B" -c "A"
Filters: "TRY FILTERS"
Randomness seeded to: 3370816543
===============================================================================
All tests passed (1 assertion in 1 test case)

Yes, that also runs the assertion inside the "C1" section. Note that
laughing less stops the assertion from running, at least for this order
of the "real" filters.

Why is it broken

Current filtering implementation is weird, and the result is that
the section filtering is partially order dependent, and also partially
isn't. The filters specified by user via -c are strictly ordered,
and every time Catch2 enters a section, it removes the first one and then
passes the rest down for further filtering.

However, the filtering check on whether a section should be entered is
done via std::find over all remaining filters. So e.g. in the last example,
when we try to enter the "A" section, we check whether there is a "A"
filter, and then we pop "LOL" filter from the filter stack. Then, when
entering "B" section, we check whether there is "B" filter somewhere
in the stack, and then pop "LMAO" filter... Thus by laughing enough, we
keep the "D" filter in stack long enough to enter the section and execute
the assertion.

How do we fix it

Instead of checking all remaining filters for the name of the section that
we are trying to enter, check just the first filter. This will remove
all of the surprising behaviour above, at the cost of disallowing the user
to pick sections "A1" and "A2", but not "A3" in the example below.

TEST_CASE() {
    SECTION( "A1" ) {}
    SECTION( "A2" ) {}
    SECTION( "A3" ) {}
}

With the current implementation, they could do this by abusing the check
logic with -c "A1" -c "A2" section filter spec. If we want to support
this behaviour in the future, we should extend the section filter specs
to support similar set of operators as test filters do.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions