Fix line filters running tests from multiple runnables. #23456
Merged
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.
derive_regexp
was written with the assumption that we were run from ablank slate — that if the filter didn't match we might as well return it
because it was nil.
This isn't the case because minitest calls
run
on every runnable. Whichis any subclass of Minitest::Runnable, such as ActiveSupport::TestCase,
ActionDispatch::IntegrationTest as well as any inheriting from those.
Thus after the first
run
we'd have put in a composite filter inoptions[:filter]
making the nextrun
create a linked list when itfailed to match the regexp and put the composite filter as the head.
Every runnable would accumulate more and more of the same filters,
which effectively acted like an expanding whitelist and we ran tests
from other runnables.
Clog the accumulation by returning nil if there's no filter to derive
a regexp from.
Note: we pass a seed in the tests because Minitest shuffles the runnables
to ensure the whitelist is expanded enough that the failure is triggered.
r? @kaspth