Fix the test filter dropping tests it should run - #190
Merged
sheredom merged 1 commit intoAug 18, 2026
Conversation
utest_should_filter_test got two things wrong, and both silently skip a test rather than report anything. A trailing wildcard never matched the empty remainder, so --filter=*bar* did not run foo.bar although --filter=*bar and --filter=*ba* both did. The name being exhausted was treated as a mismatch even with only wildcards left to consume. Worse, a wildcard could not give characters back. On a mismatch the filter position was reset to the wildcard but the name position was not, so the retry resumed midway through the name. That is not an edge case: --filter=*o.b* does not match foo.bar today. Replaced with the usual iterative glob match, which records where to resume and backtracks properly. It handles both, and is 24 lines shorter than what it replaces. Six tests added for the filter, covering exact names, leading and trailing wildcards, literals after an exhausted name, and backtracking. They call the function directly rather than spawning a binary, so unlike the utest_cmdline suite they also run on MinGW. Against the old implementation five of the six fail; the rest of the suite is unchanged at 2161 tests, 2167 with these.
Owner
|
Great find! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
utest_should_filter_testgets two things wrong. Both fail the same way: thetest is silently skipped, and nothing is reported — the run just comes back
green with fewer tests than you asked for.
A trailing wildcard never matches the empty remainder
--filter=*bar runs foo.bar
--filter=ba runs foo.bar
--filter=bar does not
Once the name is exhausted, a wildcard still left in the filter is treated as a mismatch rather than as standing for nothing. It bites in two places: when the inner wildcard loop runs out of name, and when the outer loop does.
A wildcard cannot give characters back
On a mismatch the filter position is reset to the wildcard, but the name position is not, so the retry resumes partway through the name instead of one character on from where the attempt began. This is not an edge case:
--filter=o.b does not match foo.bar
The change
Replaced with the usual iterative glob match — remember the wildcard and the position to resume from, and on a mismatch let the wildcard swallow one more character. It handles both defects, and
utest.hcomes out 24 lines shorter than before.Measured over 21 filter/name pairs: the current implementation answers 6 of them wrongly, the replacement none. Patching only the trailing-wildcard case still leaves 3 wrong, which is why this replaces the function rather than guarding it.
Tests
Six tests covering exact names, leading and trailing wildcards, literals after an exhausted name, and backtracking. They call the function directly instead of spawning a binary, so unlike the
utest_cmdlinesuite they also run under MinGW.Five of the six fail against the current implementation — checked, so they are known to have teeth rather than assumed to.
mainIndependent of #188 and #189; applies to
mainon its own.