-
Notifications
You must be signed in to change notification settings - Fork 17.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/go: go test -failfast doesn't stop immediately with t.Parallel() #30522
Comments
CC @jayconrod |
I doubt we'll get to this in Go 1.13, but I plan to do some work on the |
Change https://golang.org/cl/185877 mentions this issue: |
The t.Parallel method does not interact well with the -failfast flag. See golang/go#30522. Instead, start off tests ourselves in a goroutine. Doing it this way allows the -failfast flag to take effect sooner. One nice benefit of this change is that the test output is cleaner: Before: <<< === RUN Test === RUN Test/Go1.9.7 === RUN Test/Go1.9.7/TestNormal === PAUSE Test/Go1.9.7/TestNormal === RUN Test/Go1.9.7/TestPureGo === PAUSE Test/Go1.9.7/TestPureGo === RUN Test/Go1.9.7/TestReflect === PAUSE Test/Go1.9.7/TestReflect === CONT Test/Go1.9.7/TestNormal === CONT Test/Go1.9.7/TestReflect === CONT Test/Go1.9.7/TestPureGo === RUN Test/Go1.10.8 === RUN Test/Go1.10.8/TestNormal === PAUSE Test/Go1.10.8/TestNormal === RUN Test/Go1.10.8/TestPureGo === PAUSE Test/Go1.10.8/TestPureGo === RUN Test/Go1.10.8/TestReflect === PAUSE Test/Go1.10.8/TestReflect === CONT Test/Go1.10.8/TestNormal === CONT Test/Go1.10.8/TestReflect === CONT Test/Go1.10.8/TestPureGo === RUN Test/Go1.11.6 === RUN Test/Go1.11.6/TestNormal === PAUSE Test/Go1.11.6/TestNormal === RUN Test/Go1.11.6/TestPureGo === PAUSE Test/Go1.11.6/TestPureGo === RUN Test/Go1.11.6/TestReflect === PAUSE Test/Go1.11.6/TestReflect === CONT Test/Go1.11.6/TestNormal === CONT Test/Go1.11.6/TestReflect === CONT Test/Go1.11.6/TestPureGo === RUN Test/Go1.12.1 === RUN Test/Go1.12.1/TestNormal === PAUSE Test/Go1.12.1/TestNormal === RUN Test/Go1.12.1/TestPureGo === PAUSE Test/Go1.12.1/TestPureGo === RUN Test/Go1.12.1/TestReflect === PAUSE Test/Go1.12.1/TestReflect === RUN Test/Go1.12.1/TestProto1Legacy === PAUSE Test/Go1.12.1/TestProto1Legacy === RUN Test/Go1.12.1/TestProtocGenGo === PAUSE Test/Go1.12.1/TestProtocGenGo === RUN Test/Go1.12.1/TestProtocGenGoGRPC === PAUSE Test/Go1.12.1/TestProtocGenGoGRPC === CONT Test/Go1.12.1/TestNormal === CONT Test/Go1.12.1/TestProto1Legacy === CONT Test/Go1.12.1/TestProtocGenGoGRPC === CONT Test/Go1.12.1/TestProtocGenGo === CONT Test/Go1.12.1/TestReflect === CONT Test/Go1.12.1/TestPureGo === RUN Test/ConformanceTests === RUN Test/GeneratedGoFiles === RUN Test/FormattedGoFiles === RUN Test/CommittedGitChanges === RUN Test/TrackedGitFiles --- PASS: Test (509.59s) --- PASS: Test/Go1.9.7 (0.00s) --- PASS: Test/Go1.9.7/TestReflect (100.81s) --- PASS: Test/Go1.9.7/TestNormal (100.88s) --- PASS: Test/Go1.9.7/TestPureGo (56.93s) --- PASS: Test/Go1.10.8 (0.00s) --- PASS: Test/Go1.10.8/TestNormal (64.85s) --- PASS: Test/Go1.10.8/TestReflect (65.20s) --- PASS: Test/Go1.10.8/TestPureGo (36.14s) --- PASS: Test/Go1.11.6 (0.00s) --- PASS: Test/Go1.11.6/TestReflect (59.69s) --- PASS: Test/Go1.11.6/TestNormal (60.25s) --- PASS: Test/Go1.11.6/TestPureGo (34.17s) --- PASS: Test/Go1.12.1 (0.00s) --- PASS: Test/Go1.12.1/TestProto1Legacy (59.41s) --- PASS: Test/Go1.12.1/TestNormal (59.87s) --- PASS: Test/Go1.12.1/TestProtocGenGo (3.55s) --- PASS: Test/Go1.12.1/TestProtocGenGoGRPC (4.12s) --- PASS: Test/Go1.12.1/TestReflect (60.10s) --- PASS: Test/Go1.12.1/TestPureGo (60.66s) --- PASS: Test/ConformanceTests (0.78s) --- PASS: Test/GeneratedGoFiles (8.99s) --- PASS: Test/FormattedGoFiles (2.64s) --- PASS: Test/CommittedGitChanges (0.10s) --- PASS: Test/TrackedGitFiles (0.00s) PASS >>> After: <<< === RUN Test === RUN Test/Go1.9.7/PureGo === RUN Test/Go1.9.7/Normal === RUN Test/Go1.9.7/Reflect === RUN Test/Go1.10.8/Normal === RUN Test/Go1.10.8/PureGo === RUN Test/Go1.10.8/Reflect === RUN Test/Go1.11.6/Normal === RUN Test/Go1.11.6/PureGo === RUN Test/Go1.11.6/Reflect === RUN Test/Go1.12.1/Normal === RUN Test/Go1.12.1/PureGo === RUN Test/Go1.12.1/Reflect === RUN Test/Go1.12.1/Proto1Legacy === RUN Test/Go1.12.1/ProtocGenGo === RUN Test/Go1.12.1/ProtocGenGoGRPC === RUN Test/ConformanceTests === RUN Test/GeneratedGoFiles === RUN Test/FormattedGoFiles === RUN Test/CommittedGitChanges --- PASS: Test (182.87s) --- PASS: Test/Go1.9.7/PureGo (72.17s) --- PASS: Test/Go1.9.7/Normal (72.59s) --- PASS: Test/Go1.10.8/Normal (8.03s) --- PASS: Test/Go1.10.8/PureGo (8.03s) --- PASS: Test/Go1.10.8/Reflect (8.34s) --- PASS: Test/Go1.11.6/Normal (10.70s) --- PASS: Test/Go1.11.6/PureGo (8.21s) --- PASS: Test/Go1.11.6/Reflect (8.77s) --- PASS: Test/Go1.12.1/Normal (7.41s) --- PASS: Test/Go1.12.1/PureGo (6.52s) --- PASS: Test/Go1.12.1/Reflect (6.04s) --- PASS: Test/Go1.12.1/Proto1Legacy (6.96s) --- PASS: Test/Go1.12.1/ProtocGenGo (1.15s) --- PASS: Test/Go1.12.1/ProtocGenGoGRPC (2.43s) --- PASS: Test/Go1.9.7/Reflect (88.47s) --- PASS: Test/ConformanceTests (1.81s) --- PASS: Test/GeneratedGoFiles (16.28s) --- PASS: Test/FormattedGoFiles (3.17s) --- PASS: Test/CommittedGitChanges (0.17s) PASS >>> Change-Id: I777ffe3f4d5f6407c87e3866cbaa890017204cba Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185877 Reviewed-by: Herbie Ong <herbie@google.com>
The t.Parallel method does not interact well with the -failfast flag. See golang#30522. With -failfast is enabled tests blocking on t.Parallel() can be skipped after the first test failure. This won't immediately kill any parallel tests which are already running but it will prevent more tests from starting.
The t.Parallel method does not interact well with the -failfast flag. See golang#30522. With -failfast is enabled tests blocking on t.Parallel() can be skipped after the first test failure. This won't immediately kill any parallel tests which are already running but it will prevent more tests from starting.
The t.Parallel method does not interact well with the -failfast flag. See golang#30522. With -failfast is enabled tests blocking on t.Parallel() can be skipped after the first test failure. This won't immediately kill any parallel tests which are already running but it will prevent more tests from starting.
The t.Parallel method does not interact well with the -failfast flag. See golang#30522. With -failfast enabled tests blocking on t.Parallel() can be skipped after the first test failure. This won't immediately kill any parallel tests which are already running but it will prevent more tests from starting.
The t.Parallel method does not interact well with the -failfast flag. See golang#30522. With -failfast enabled tests blocking on t.Parallel() can be skipped after the first test failure. This won't immediately kill any parallel tests which are already running but it will prevent more tests from starting.
Change https://go.dev/cl/413236 mentions this issue: |
Another repro case, making this clearer: package q
import "testing"
func TestA(t *testing.T) {
t.Parallel()
t.Fatal("a")
}
func TestB(t *testing.T) {
t.Parallel()
t.Fatal("b")
} Yields: $ go test -v -parallel=1 -failfast ./testing_test.go
=== RUN TestA
=== PAUSE TestA
=== RUN TestB
=== PAUSE TestB
=== CONT TestA
testing_test.go:7: a
--- FAIL: TestA (0.00s)
=== CONT TestB
testing_test.go:12: b
--- FAIL: TestB (0.00s)
FAIL
FAIL command-line-arguments 0.051s
FAIL Though I would only expect one test to be marked "FAIL". I think the issue is that Line 1272 in 2f6783c
It's only checked early in Line 1463 in 2f6783c
Considering this as the solution: t.context.waitParallel()
if shouldFailFast() {
if t.chatty != nil {
t.chatty.Updatef(t.name, "=== ABORT %s\n", t.name)
}
t.SkipNow()
return
} Edit Tests completed: $ ../bin/go test -v -parallel=1 -failfast ./testing_test.go
=== RUN TestA
=== PAUSE TestA
=== RUN TestB
=== PAUSE TestB
=== CONT TestA
testing_test.go:7: a
--- FAIL: TestA (0.00s)
=== ABORT TestB
--- SKIP: TestB (0.00s)
FAIL
FAIL command-line-arguments 0.003s
FAIL It would be nice to not have this "skipped", but it's better than actually running the test code. :) |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes.
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
run with
go test -v -failfast -parallel 1
returns:What did you expect to see?
TestParallel2
not continue.What did you see instead?
TestParallel2
continued execution.The text was updated successfully, but these errors were encountered: