go test treats exited suite as overall "pass"

[grasparv]

go version go1.14.3 linux/amd64

I maintain testie that uses go test -json.

Here is an example reproducing unexpected results:

$ ls -l
total 8.0K
-rw-r--r-- 1 js js 332 May 16 11:52 bad_test.go
-rw-r--r-- 1 js js  22 May 16 11:46 go.mod

bad_test.go

package bad

import (
        "os"
        "testing"
)

func productionFunction1() {
        os.Exit(0)
}

func productionFunction2() bool {
        return true
}

func TestHelloOne(t *testing.T) {
        productionFunction1()
        if productionFunction2() == true {
                t.Fail()
        }
}

func TestHelloTwo(t *testing.T) {
        if productionFunction2() == true {
                t.Fail()
        }
}

$ go test -json ./...
{"Time":"2020-05-16T11:52:37.303091346+02:00","Action":"run","Package":"hello","Test":"TestHelloOne"}
{"Time":"2020-05-16T11:52:37.303398456+02:00","Action":"output","Package":"hello","Test":"TestHelloOne","Output":"=== RUN   TestHelloOne\n"}
{"Time":"2020-05-16T11:52:37.303932387+02:00","Action":"output","Package":"hello","Test":"TestHelloOne","Output":"ok  \thello\t0.005s\n"}
{"Time":"2020-05-16T11:52:37.304042213+02:00","Action":"pass","Package":"hello","Test":"TestHelloOne","Elapsed":0.006}
$

Now it appears as if the test suite overall result was OK just because the production code (incorrectly) reached an os.Exit(0).

I would have expected go test to:

1. Find all test cases
2. Execute from that list of test cases

Rationale: At the point in time where the go test command has begun execution and established a list of all tests that should run, any failures to execute must be reported as such. By specifying ./..., I believe I am referring to all tests in this directory and sub-directories. It translates to the test specification: Run TestHelloOne and run TestHelloTwo. If go test fails to what I told it to, it violates the test specification. Go can not choose on its own accord to not execute the test due to behaviour in test subject.

Alternatives?: Saying that the production is not allowed to os.Exit(0) incorrectly in production code is obviously not a solution, and it seems unreasonable that users should have to add some secondary layer of tools on top of or inside the test runner to determine whether all tests were really executed (in this case I'd like to have a new flag for go test with this behaviour!) - if this is even possible with the limited information available (e.g. how does a test runner list all tests to execute for ./... for comparing it with go test -json output)?

The behaviour actually seems to be the same for go test without the -json switch, but I am reporting it here as a user of go test -json without trying to pinpoint where the issue lies.

Thank you!

[ALTree]

This is a dup of the proposal at #29062, which was already accepted. No one did the work for the 1.15 code freeze deadline, so now it'll have to wait for the 1.16 development phase. Contributions are welcome, so if you care about this and want to contribute, feel free to do so.

I'm closing here, since this is a dup of a proposal that was already accepted. Additional discussion about this should be placed in the thread at #29062. Thank you.

[grasparv]

Oh, I did not see it. Thanks.