testing: change -run regexp to make / have usual precedence #39904
Comments
If a root test case has failed subtests, only rerun those that have failed. A new hidden flag is added to restore the old behaviour of re-running the entire root test case if any subtest fails. Because of limitations of the 'go test -run' flag (golang/go#39904) this required running each test in a separate 'go test' process.
CC @mpvl |
If a root test case has failed subtests, only rerun those that have failed. A new hidden flag is added to restore the old behaviour of re-running the entire root test case if any subtest fails. Because of limitations of the 'go test -run' flag (github.com/golang/go/issues/39904) this required running each test in a separate 'go test' process.
I see there was some discussion about this in https://go-review.googlesource.com/c/go/+/19122/. I think the Maybe the same goal can be accomplished by adding |
The testing package must have some way to figure out whether the top-level test matches (and should be started), separate from the rest of the expression, so that's why it splits on unparenthesized slashes. But this changes the ordinary precedence of concatenation-with-slash to be higher than alternation (|), which was probably a mistake. Maybe the fix is just to split on |s first. That is, as shown above
is implicitly
and there's no way to override those implicit parens. Splitting on pipes first would essentially remove those implicit parens. I'd be willing to try that (splitting first on pipes) at the start of a cycle and see if it breaks anything. Thoughts? /cc @mpvl |
Wouldn't that break usage like this, where the intention is to run several of the
I'm not sure that this behaviour can be changed now for that reason. ... and if we decide that the behaviour is ok to be changed, for the record, I still think that things would be better if all these patterns were implicitly anchored (I often get bitten by several tests running when I only intended one because the test name As another possibility for fixing this issue, how about allowing multiple For example:
Another ability I'd like to see is the ability to exclude tests rather than include them, when there's a problematic test, but that's another proposal :) |
A new flag ( Allowing the I think splitting on
|
@rogpeppe, you're right that we'd be changing the meaning of that syntax, but The problem that this change would solve is that The suggestion solution - splitting on pipes before splitting on slashes - more closely respects the text matches, by making the pipe operator a looser binding than the implicit concatenation around slash. And when it's not what you want, parens can always be added. Pipe is the only operator that binds looser than concatenation, so it's the only instance of this problem that we'd need to address. An alternative would be to go back to exactly the plain text semantics, with the added rule that matching right up to a slash is allowed. That would require extra support in the regexp engine and would still be special, but even more differently special (for example, -run='^X$/^Y$' would not work anymore), so I'm not inclined to go down that road instead. The current syntax was a bit of a compromise, but it does work well in general, and so the general design might as well stay. But it still seems like we should fix this precedence problem that has no workaround. Yes, it does change the meaning of certain corner cases - by design - but it changes them in a way that can always be overridden with parens (in contrast to today). |
Does anyone object to making the change described in my comment above? |
Based on the discussion above, this seems like a likely accept |
No change in consensus, so accepted. |
The
go test -run
flag can be used to select multiple root test cases. This example will run the 3 tests which match the regex exactly:As far as I can tell, the logic for handling subtests makes it impossible to do the same with subtest cases. It seems to be possible to run multiple root test cases, with a single subtest case:
But adding anything after the
/
is assumed to be a sub-sub test, and does not match the same way.Some other things that I tried, which do not work:
Matches any
SubTestA
, not just the one fromTestThree
Does not match any subtests
Because of how parenthesis are handled in
splitRegex
Would it be reasonable to support this with the
-run
flag? Looking over the matching logic insrc/testing
it seems like this may not be a small change.The text was updated successfully, but these errors were encountered: