cmd/go: add go test -skip to skip specific tests

[dprotaso]

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

[ianlancetaylor]

Do you have a specific suggestion?

To run specific tests you can already write go test -run="TestOne|TestTwo|TestThree".

[dprotaso]

Do you have a specific suggestion?

I didn't want to prescribe a solution but here's some off the top of my head

1. 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

2. 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

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

[dprotaso]

hey @ianlancetaylor - do you have any additional questions or want me to clarify anything above?

[ianlancetaylor]

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.

[rsc]

Almost no one on earth understands how to write negation regexps correctly, so I would lean toward -skip if we do this.

[bcmills]

requires definition of precedence between -skip & -run

I can see only two reasonable definitions.

1. Define that the default value of -run matches everything, and therefore -skip takes precedence over -run.
2. Define that the default value of -run imposes no constraints, and therefore it is an error to specify values of -run and -skip that 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.

[jimmyfrasche]

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.

[bcmills]

Yeah, it seems especially useful in the context of subtests:

go test foo -run=TestBar -skip=TestBar/flaky_subtest

[dprotaso]

Does precedence mean the order the filters are applied in or which is used if both are given?

The latter in my opinion

[ChrisHines]

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.

[rsc]

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?

[bcmills]

The -skip setting … 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.)

[dprotaso]

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.

[ChrisHines]

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.

[magical]

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.

[dprotaso]

-run already has special handling for / so it is already not a straight regexp.

Truthfully I was tripped up by this when I first encountered it

[bcmills]

@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.

[bcmills]

@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.

[rsc]

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!)

[rsc]

@magical, more magic in -run is not a good idea. The slash was not great either but is done.

[ChrisHines]

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.

[rsc]

-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?

[ChrisHines]

No objection here.

[dprotaso]

No objection

…

On Wed, Oct 21, 2020 at 14:36 Chris Hines ***@***.***> wrote: No objection here. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#41583 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAERAQIEIIMXURBJQH2EMDSL4S3XANCNFSM4RXF7DKQ> .

[rsc]

Based on the discussion, this seems like a likely accept.

[rsc]

No change in consensus, so accepted.

[mvdan]

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.

[mvdan]

Is this issue up for grabs?

I'll take the silence and reactions as a yes :)

[mvdan]

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.

[mvdan]

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.

[perillo]

Users shouldn't use go tool dist test directly, but the default value of the -run flag is $GOTESTONLY.