go
go copied to clipboard
golang
cmd/go: add go test -skip to skip specific tests
t.Skip() is an option but it requires changing the test code.
It's hard to write a correct regex to pass to the -run flag in order to skip specific tests. This is due to the lack of lookaheads in the regexp library. Previous discussion here: https://groups.google.com/g/golang-nuts/c/7qgSDWPIh_E?pli=1
Do you have a specific suggestion?
To run specific tests you can already write go test -run="TestOne|TestTwo|TestThree".
Do you have a specific suggestion?
I didn't want to prescribe a solution but here's some off the top of my head
-
expand run flag functionality -
-run [regex]to allow look aheads - thus negation- pros
- most people are familiar with this flag
- cons
- requires expanded regex implementation
- pros
-
new flag
-skip [regex]- pros
- mirrors run flag
- may be easier to use than a negative regex
- cons
- requires definition of precedence between
-skip&-run - it'd be nice to see skipped test output - that might require more work to implement
- requires definition of precedence between
- pros
To run specific tests you can already write go test -run="TestOne|TestTwo|TestThree".
This doesn't scale if I have O(10s) of tests in a package and want to skip one or two
hey @ianlancetaylor - do you have any additional questions or want me to clarify anything above?
No, I'm good.
This issue is in the proposal process now and will get into the proposal committee in due course. The committee looks at a lot of issues (see minutes at #33502), so it won't be immediately, but it shouldn't take too long.
Thanks.
Almost no one on earth understands how to write negation regexps correctly, so I would lean toward -skip if we do this.
requires definition of precedence between
-skip&-run
I can see only two reasonable definitions.
- Define that the default value of
-runmatches everything, and therefore-skiptakes precedence over-run. - Define that the default value of
-runimposes no constraints, and therefore it is an error to specify values of-runand-skipthat both match the same test.
I have a slight preference for (1), because it treats the default value of the -run flag the same as other values.
Does precedence mean the order the filters are applied in or which is used if both are given?
If it's the order, then -run followed by -skip makes the most sense to me and seems more useful than disallowing the combination.
Yeah, it seems especially useful in the context of subtests:
go test foo -run=TestBar -skip=TestBar/flaky_subtest
Does precedence mean the order the filters are applied in or which is used if both are given?
The latter in my opinion
I have almost moved some subtests into their own top level test function on a few occasions exactly because there was not a good way to skip some subset of them. I have also had to run go test -list iterations just to figure out a regex that would run all tests except a select few, and the resulting regex is often overly long given the use case of wanting to skip a small subset when a package has a lot of tests.
It sounds like people are generally in favor of adding -skip with a default of "matches nothing at all".
The -skip setting applies after the -run setting (which already defaults to "match everything")
and only applies to tests (not benchmarks).
Do I have that right? Does anyone object to adding -skip as described?
The
-skipsetting … only applies to tests (not benchmarks).
Hmm. It would be unfortunate to have an easy way to skip specific tests but not specific benchmarks.
I think it would make sense to either have -skip apply to both tests and benchmarks, or to add a separate -benchskip or similar flag for those. (It seems a bit simpler to me to make -skip also apply to benchmarks, but I don't have a strong preference either way.)
Do I have that right? Does anyone object to adding -skip as described?
Sounds good to me
Hmm. It would be unfortunate to have an easy way to skip specific tests but not specific benchmarks.
Expanding the scope of -skip to cover benchmarks makes sense to me. I don't mind if this comes later if it's not a trivial thing to do.
A way to skip benchmarks would be welcome as well.
The -run and -bench flags have different defaults. In my experience go test -run=^$ -bench=. is pretty common today, but would people start trying to use go test -skip=. -bench=. to mean the same thing? It seems to me a separate -benchskip flag would avoid cross pollinating the two sets and reduce the chances for surprise.
Instead of introducing a new flag, could we add some syntax to the existing flag? Let a pattern starting with ! invert the sense of the match. So go test -run=!Foo|Bar would run all tests that don't match Foo or Bar, same as the proposed -skip flag.
This avoids introducing new flags (-skip and -benchskip) and avoids issues about whether -run or -skip takes precedence. -run already has special handling for / so it is already not a straight regexp.
-runalready has special handling for / so it is already not a straight regexp.
Truthfully I was tripped up by this when I first encountered it
@magical, further complications in the syntax for the -skip flag would be pretty awkward for the “run a specific test but skip a specific subtest” use-case mentioned previously.
@ChrisHines, I think the question is, would someone running go test -bench=BenchmarkFoo -skip=BenchmarkFoo/some_subbench expect it to skip BenchmarkFoo/some_subbench?
I would argue that pretty much everyone would expect that to skip the sub-benchmark, so I suspect that a unified -skip flag would be less confusing than a separate -benchskip flag.
The -run and -bench flags have different defaults. In my experience go test -run=^$ -bench=. is pretty common today
FWIW this is usually a bad idea, at least interactively, since you inevitably end up benchmarking broken code. (And boy can you make broken code run fast!)
@magical, more magic in -run is not a good idea. The slash was not great either but is done.
The -run and -bench flags have different defaults. In my experience go test -run=^$ -bench=. is pretty common today
FWIW this is usually a bad idea, at least interactively, since you inevitably end up benchmarking broken code. (And boy can you make broken code run fast!)
That's a fair point. I guess it depends on one's workflow. In my experience testing and benchmarking are distinct steps of the workflow. If the tests fail I don't want to wait for benchmarks to run since they would be meaningless. Once the tests pass and I am benchmarking I don't want to waste time rerunning the tests before getting the benchmark data.
-list already applies to everything - tests, benchmarks, examples - so I think it's probably OK to have -skip do the same (apply to benchmarks; tests and examples were already covered since they are controlled by -run).
Does anyone object to doing this?
No objection
On Wed, Oct 21, 2020 at 14:36 Chris Hines [email protected] wrote:
No objection here.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/golang/go/issues/41583#issuecomment-713785061, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAERAQIEIIMXURBJQH2EMDSL4S3XANCNFSM4RXF7DKQ .
I hope it's okay that I removed the FinalCommentPeriod and WaitingForInfo labels; I don't think either applies at this point anymore.
Is this issue up for grabs? I'd like to use this feature, and there's still a bit of time before the 1.17 freeze is upon us.
I'm still working on this, but the change is non-trivial and the freeze is almost here. I don't think it's fair to rush the review and merge for 1.17, even if I was to finish the change with tests today.
Worth noting that go tool dist test seems to support a variant of this:
$ go tool dist test -h
usage: go tool dist test [options]
[...]
-run string
run only those tests matching the regular expression; empty means to run all. Special exception: if the string begins with '!', the match is inverted.
I wonder if we should also move that to -skip for the same reasons we decided against inverted regexes here. Or if it's fine to leave as is, given that most users shouldn't use go tool dist test directly.
Users shouldn't use go tool dist test directly, but the default value of the -run flag is $GOTESTONLY.
@mvdan This is in the Go 1.18 milestone. Is it likely to happen for 1.18? Thanks.
@ianlancetaylor thanks for the ping; I intend to pick up the patch again, but the change is too invasive for 1.18 at this point even if I finished the change this week.
I still intend to get to this for 1.19. There's no more need for more comments to just say it would be nice to have :) See https://github.com/golang/go/wiki/NoPlusOne.
Can you make it support skipping multiple tests from different packages? Like:
go test -skip "pkg1.TestFoo|pkg2.TestBar" ./...
It would be useful to disable flaky tests in different packages on CI
@linzhp that sounds like it should be a separate issue or proposal, because it should also affect the -run flag, and this issue does not cover changing the -run flag in any way. The two flags will take the same form of pattern in this proposal, and I don't see a reason why we should make them inconsistent.
I clearly did not get to this for 1.19, so I'm unassigning myself for now. There is no plan or estimate right now; the issue is in the backlog milestone.
Change https://go.dev/cl/421439 mentions this issue: cmd/go, testing: add go test -skip flag
Woah, next release can I finally stop sed-ing individual tests to skip? 🎉🎊
