cmd/go: go test -failfast doesn't stop immediately with t.Parallel() #30522

timoha · 2019-03-01T19:40:49Z

What version of Go are you using (`go version`)?

$ go version
go version go1.10.7 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jenkins/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build781955494=/tmp/go-build -gno-record-gcc-switches"

What did you do?

func TestParallel1(t *testing.T) {
    t.Parallel()
    t.FailNow()
}

func TestParallel2(t *testing.T) {
    t.Parallel()
}

run with go test -v -failfast -parallel 1 returns:

=== RUN   TestParallel1
=== PAUSE TestParallel1
=== RUN   TestParallel2
=== PAUSE TestParallel2
=== CONT  TestParallel1
=== CONT  TestParallel2
--- PASS: TestParallel2 (0.00s)
--- FAIL: TestParallel1 (0.00s)
FAIL
exit status 1
FAIL	test	0.075s

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:

bcmills · 2019-03-01T20:34:14Z

CC @jayconrod

bcmills · 2019-03-01T20:34:35Z

I doubt we'll get to this in Go 1.13, but I plan to do some work on the testing package for 1.14.

gopherbot · 2019-07-12T00:39:59Z

Change https://golang.org/cl/185877 mentions this issue: integration_test.go: use custom parallel test code

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

gopherbot · 2022-06-20T07:58:13Z

Change https://go.dev/cl/413236 mentions this issue: testing: skip parallel tests when -failfast is enabled

tommie · 2022-08-13T15:18:11Z

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 Parallel() doesn't check shouldFailFast() after waitParallel() returns:

go/src/testing/testing.go

Line 1272 in 2f6783c

t.context.waitParallel()

It's only checked early in Run():

go/src/testing/testing.go

Line 1463 in 2f6783c

if !ok || shouldFailFast() {

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. :)

bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 1, 2019

bcmills changed the title ~~go test -failfast doesn't work with t.Parallel()~~ cmd/go: go test -failfast doesn't stop immediately with t.Parallel() Mar 1, 2019

bcmills added this to the Unplanned milestone Mar 1, 2019

bcmills added the Performance label Mar 1, 2019

bcmills mentioned this issue Jan 10, 2022

cmd/go: 'go test ./...' not support failfast flag #50507

Closed

rubensayshi mentioned this issue Jun 20, 2022

testing: skip parallel tests when -failfast is enabled #53460

Closed

dnephin mentioned this issue Oct 2, 2022

Error if -failfast is used with --rerun-fails gotestyourself/gotestsum#280

Merged

mixmasala mentioned this issue Jun 24, 2023

unit tests hang forever occaisionally katzenpost/katzenpost#339

Open

bcmills mentioned this issue Jun 27, 2023

proposal: testing: flag to continue testing after -failfast failure #61009

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: go test -failfast doesn't stop immediately with t.Parallel() #30522

cmd/go: go test -failfast doesn't stop immediately with t.Parallel() #30522

timoha commented Mar 1, 2019

bcmills commented Mar 1, 2019

bcmills commented Mar 1, 2019

gopherbot commented Jul 12, 2019

gopherbot commented Jun 20, 2022

tommie commented Aug 13, 2022 •

edited

cmd/go: go test -failfast doesn't stop immediately with t.Parallel() #30522

cmd/go: go test -failfast doesn't stop immediately with t.Parallel() #30522

Comments

timoha commented Mar 1, 2019

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

bcmills commented Mar 1, 2019

bcmills commented Mar 1, 2019

gopherbot commented Jul 12, 2019

gopherbot commented Jun 20, 2022

tommie commented Aug 13, 2022 • edited

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

tommie commented Aug 13, 2022 •

edited