Skip to content
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

testing: panics during a test quietly interrupt additional test runs #47525

Closed
saracen opened this issue Aug 4, 2021 · 3 comments
Closed

testing: panics during a test quietly interrupt additional test runs #47525

saracen opened this issue Aug 4, 2021 · 3 comments

Comments

@saracen
Copy link
Contributor

@saracen saracen commented Aug 4, 2021

81a74b4 introduced a fix that provided additional test information should a test panic and ensured that test output was flushed.

A panicked test is output as a failed test item like any other failure. However, what is not clear from this output or the exit code, is that whilst a failure occurred, no additional tests will then be run.

Consider:

func TestWithPanic(t *testing.T) {
	panic("panic during test")
}

func TestEclipsedByPanic(t *testing.T) {
	// success, failure?
}

Running go test here will immediately exit after TestWithPanic. We know that this test failed, but we don't know that it paniced nor caused additional tests (TestEclipsedByPanic) to not run without delving deeper into the test's output.

Ordinarily, when a test failure occurs, additional tests are still run. By not making it clear that a panic took place, a test reporting tool might assume that the test output contains all successes and failures, but could in fact be missing tests.

A possible solution to this would be to use a different exit code other than 1 to indicate that there wasn't just a regular test failure present, but a completely irrecoverable test run.

@bcmills
Copy link
Member

@bcmills bcmills commented Aug 4, 2021

Ordinarily, when a test failure occurs, additional tests are still run. By not making it clear that a panic took place, a test reporting tool might assume that the test output contains all successes and failures, but could in fact be missing tests.

If all tests are run, then the test emits a final FAIL line before exiting. That final line is omitted if a test terminates by panicking or calling os.Exit:

$ go test
--- FAIL: TestWithPanic (0.00s)
panic: panic during test [recovered]
        panic: panic during test

goroutine 6 [running]:
testing.tRunner.func1.2({0x4f0100, 0x534de8})
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1209 +0x24e
testing.tRunner.func1()
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1212 +0x218
panic({0x4f0100, 0x534de8})
        /usr/local/google/home/bcmills/sdk/gotip/src/runtime/panic.go:1038 +0x215
example.com/m.TestWithPanic(0x0)
        /tmp/tmp.cbe4PylsVE/main_test.go:6 +0x27
testing.tRunner(0xc0001221a0, 0x517260)
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1259 +0x102
created by testing.(*T).Run
        /usr/local/google/home/bcmills/sdk/gotip/src/testing/testing.go:1306 +0x35a
exit status 2
FAIL    example.com/m   0.013s

-- main_test.go --
package main

import "testing"

func TestWithPanic(t *testing.T) {
	panic("panic during test")
}

func TestEclipsedByPanic(t *testing.T) {
	// success, failure?
}
$ go test
--- FAIL: TestWithoutPanic (0.00s)
    main_test.go:6: just a failure
FAIL
exit status 1
FAIL    example.com/m   0.014s

-- main_test.go --
package main

import "testing"

func TestWithoutPanic(t *testing.T) {
	t.Errorf("just a failure")
}

func TestEclipsedByPanic(t *testing.T) {
	// success, failure?
}

Loading

@saracen
Copy link
Contributor Author

@saracen saracen commented Aug 4, 2021

Thank you @bcmills

It appears that test2json adds the FAIL to the package's output. I guess that's easy enough to parse. 👍

{"Time":"2021-08-04T18:52:47.136312+01:00","Action":"output","Package":"example.com/m","Output":"FAIL\n"}

If using go test ./..., you get FAIL at the very end for both failure and panic. This doesn't appear to be parsed by test2json. But that's not a problem, I just found it odd.

Loading

@seankhliao
Copy link
Contributor

@seankhliao seankhliao commented Aug 5, 2021

Closing as there doesn't seem to be anything to be done.

Note test2json only handles the output of a single package

Loading

@seankhliao seankhliao closed this Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants