go: cmd/go: fail tests that invoke os.Exit(0) explicitly

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

$ go version
go version go1.11.2 darwin/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="/Users/rhysd/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/rhysd/.go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.2/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/9t/jwm1hlr905g_wlnzrmbnb3cr0000gn/T/go-build385579133=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Here is a code snip to explain this issue:

package foo

import (
	"os"
	"testing"
)

type Parsed struct{}

func parseArguments(args []string) (Parsed, error) {
	// parse arguments
	if len(args) == 0 {
		// Show help message
		os.Exit(0)
	}

	// check parsed arguments

	return Parsed{}, nil
}

func TestParse(t *testing.T) {
	_, err := parseArguments([]string{})
	if err != nil {
		t.Fatal(err)
	}
	// test parse result
}

func TestOther(t *testing.T) {
	t.Fatal()
}

Please write above code to some Go file and run:

$ go test
$ echo $?

It outputs 0. So it means that test is ok. However, actually test has stopped at the middle of execution since os.Exit(0) is accidentally called. I’m not sure that this is a bug. It may be intended behavior. But when calling os.Exit(0) in tests accidentally (for example, due to lack of understanding of API), I may not notice the tests are wrongly run since it exits successfully. CI also cannot detect it.

What did you expect to see?

IMO, go test exiting with non-zero exit status when the tests exit at the middle of execution by os.Exit() would solve this issue.

What did you see instead?

echo $? echoes 0 and test exited successfully

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 8
Comments: 45 (36 by maintainers)

Commits related to this issue

Revert "cmd/go: fail if a test binary exits with no output" This reverts CL 184457. Reason for revert: introduced failures in the regression test for #18153. Fixes #34791 Updates #29062 Change-Id:... — committed to golang/go by bcmills 5 years ago
misc: log 'ok' from 'go run' tests on success Otherwise, these tests produce no output, which can make the overall output of all.bash a bit tricky to decipher. Updates #30316 Updates #29062 Change-... — committed to golang/go by bcmills 5 years ago
cmd/go: make go test -json report failures for panicking/exiting tests 'go test -json' should report that a test failed if the test binary did not exit normally with status 0. This covers panics, non... — committed to golang/go by deleted user 4 years ago
[release-branch.go1.14] cmd/go: make go test -json report failures for panicking/exiting tests 'go test -json' should report that a test failed if the test binary did not exit normally with status 0.... — committed to golang/go by deleted user 4 years ago
testing: restore os.Exit(0) after every call to (*M).Run cmd/go.TestScript/test_main_twice demonstrates a program that invokes (*M).Run twice in a row. If we only restore os.Exit(0) in m.afterOnce, w... — committed to golang/go by bcmills 4 years ago
cmd/test2json: document passing -test.paniconexit0 For #29062 Fixes #43263 Change-Id: I160197c94cc4f936967cc22c82cec01663a14fe6 Reviewed-on: https://go-review.googlesource.com/c/go/+/283873 Trust: I... — committed to golang/go by ianlancetaylor 3 years ago

Most upvoted comments

I would normally agree, but note that an os.Exit(0) silently skips the rest of the tests, doesn’t let TestMain exit properly, and could hide real test failures. This issue is not about reasonable uses of os.Exit(0), it’s about unexpected or unintended ones.

I’d argue that os.Exit should only be used as part of TestMain and never within a test, which covers the example given in the original post here, and which is the direction we were trying to take go test in.

Even if we choose to not do anything for the original issue here, I’m still strongly opposed to having go test sometimes provide tests with a real terminal, though.

mvdan on Oct 11, 2019

Exiting 0 is how you indicate success. If the test goes out of its way to do that, I am not convinced we should second-guess it.

rsc on Oct 11, 2019

Thanks for taking care of this @bcmills. I do fundamentally disagree with the premise of #18153. For example, it makes tests brittle, and requires them to be run with a real terminal, and no arguments:

$ cat f_test.go
package foo

import (
        "os"
        "testing"

        "golang.org/x/crypto/ssh/terminal"
)

func TestFoo(t *testing.T) {
        if !terminal.IsTerminal(int(os.Stdout.Fd())) {
                t.Fatal("not a terminal!")
        }
}
$ go test
PASS
ok      test.tld/foo    0.002s
$ go test | cat
--- FAIL: TestFoo (0.00s)
    f_test.go:12: not a terminal!
FAIL
exit status 1
FAIL    test.tld/foo    0.002s
$ go test .
--- FAIL: TestFoo (0.00s)
    f_test.go:12: not a terminal!
FAIL
FAIL    test.tld/foo    0.002s
FAIL

go test -v with no arguments fails, too.

Moreover, it makes it very hard (or impossible, I suspect) to inspect or modify the output in any way, like we needed to do here.

Do you reckon a proposal is necessary to make a proper decision here?

mvdan on Oct 10, 2019

Change https://golang.org/cl/250977 mentions this issue: cmd/go, testing, os: fail test that calls os.Exit(0)

gopherbot on Aug 27, 2020

This is marked as early-in-cycle but we have about three weeks left before the freeze. Assuming you plan to continue work on this, @mvdan, I’m moving to Backlog. If not, please move to Unplanned. Thanks

andybons on Oct 15, 2019

This thread started going back to the original os.Exit issue, so I’ve started a new proposal about go test and terminal outputs at https://github.com/golang/go/issues/34877.

mvdan on Oct 13, 2019

Certainly the fix I implemented (and got reverted) wouldn’t catch all scenarios. The intent was basically to catch honest mistakes, not tests trying to do harmful things. If a test is trying really hard to do harmful stuff, it could do much worse 😃

I personally think that simple rules around “the test binary’s output stopped unexpectedly” are simpler and better in the long run than doing magic things with linkname. And we probably don’t need linkname to catch the most common honest mistakes.