Skip to content

Commit

Permalink
Avoid rerun of fialed tests when previous run had a panic
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dnephin committed Apr 17, 2021
1 parent 5c6169b commit 80568f3
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 0 deletions.
32 changes: 32 additions & 0 deletions cmd/main_test.go
Expand Up @@ -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())
}
2 changes: 2 additions & 0 deletions cmd/rerunfails.go
Expand Up @@ -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
}
Expand Down
19 changes: 19 additions & 0 deletions testjson/execution.go
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 80568f3

Please sign in to comment.