Repository: gotestyourself/gotestsum
Pull request #192 (2021-04-17 20:35:12)

Avoid rerun of failed tests when previous run had a panic
Mitigates the upstream go bug: https://github.com/golang/go/issues/45508

Since re-running tests requires knowledge of all tests, attempting a re-run after a panic is not safe. `gotestsum` will not be aware of any of the tests that did not run, so many tests could be skipped if a rerun is attempted.

Ideally the exit code of the `go test` process would be 2 in this case, but until that changes we are forced to rely on string comparison of the output.


Patch:
diff --git a/cmd/rerunfails.go b/cmd/rerunfails.go
index 966c67d..4676301 100644
--- a/cmd/rerunfails.go
+++ b/cmd/rerunfails.go
@@ -94,6 +94,8 @@ func hasErrors(err error, exec *testjson.Execution) error {
 	// Exit code 0 and 1 are expected.
 	case ExitCodeWithDefault(err) > 1:
 		return fmt.Errorf("unexpected go test exit code: %v", err)
+	case exec.HasPanic():
+		return fmt.Errorf("rerun aborted because previous run had a suspected panic and some test may not have run")
 	default:
 		return nil
 	}


Test patch:
diff --git a/cmd/main_test.go b/cmd/main_test.go
index 4e794f4..ad33aec 100644
--- a/cmd/main_test.go
+++ b/cmd/main_test.go
@@ -367,3 +367,35 @@ func TestRun_RerunFails_BuildErrorPreventsRerun(t *testing.T) {
 // type checking of os/exec.ExitError is done in a test file so that users
 // installing from source can continue to use versions prior to go1.12.
 var _ exitCoder = &exec.ExitError{}
+
+func TestRun_RerunFails_PanicPreventsRerun(t *testing.T) {
+	jsonFailed := `{"Package": "pkg", "Action": "run"}
+{"Package": "pkg", "Test": "TestOne", "Action": "run"}
+{"Package": "pkg", "Test": "TestOne", "Action": "output","Output":"panic: something went wrong\n"}
+{"Package": "pkg", "Action": "fail"}
+`
+
+	fn := func(args []string) proc {
+		return proc{
+			cmd:    fakeWaiter{result: newExitCode("failed", 1)},
+			stdout: strings.NewReader(jsonFailed),
+			stderr: bytes.NewReader(nil),
+		}
+	}
+	reset := patchStartGoTestFn(fn)
+	defer reset()
+
+	out := new(bytes.Buffer)
+	opts := &options{
+		rawCommand:                   true,
+		args:                         []string{"./test.test"},
+		format:                       "testname",
+		rerunFailsMaxAttempts:        3,
+		rerunFailsMaxInitialFailures: 1,
+		stdout:                       out,
+		stderr:                       os.Stderr,
+		hideSummary:                  newHideSummaryValue(),
+	}
+	err := run(opts)
+	assert.ErrorContains(t, err, "rerun aborted because previous run had a suspected panic", out.String())
+}
diff --git a/testjson/execution.go b/testjson/execution.go
index ded622c..a43dfce 100644
--- a/testjson/execution.go
+++ b/testjson/execution.go
@@ -99,6 +99,11 @@ type Package struct {
 	action Action
 	// cached is true if the package was marked as (cached)
 	cached bool
+	// panicked is true if the package, or one of the tests in the package,
+	// contained output that looked like a panic. This is used to mitigate
+	// github.com/golang/go/issues/45508. This field may be removed in the future
+	// if the issue is fixed in Go.
+	panicked bool
 }
 
 // Result returns if the package passed, failed, or was skipped because there
@@ -172,6 +177,9 @@ func (p *Package) OutputLines(tc TestCase) []string {
 }
 
 func (p *Package) addOutput(id int, output string) {
+	if strings.HasPrefix(output, "panic: ") {
+		p.panicked = true
+	}
 	// TODO: limit size of buffered test output
 	p.output[id] = append(p.output[id], output)
 }
@@ -506,6 +514,17 @@ func (e *Execution) Errors() []string {
 	return e.errors
 }
 
+// HasPanic returns true if at least one package had output that looked like a
+// panic.
+func (e *Execution) HasPanic() bool {
+	for _, pkg := range e.packages {
+		if pkg.panicked {
+			return true
+		}
+	}
+	return false
+}
+
 func (e *Execution) end() []TestEvent {
 	e.done = true
 	var result []TestEvent // nolint: prealloc