Skip to content

Commit

Permalink
[fix] Flakey test runner fixes (#10614)
Browse files Browse the repository at this point in the history
Changing the format of the test output broke the flakey test runner. In
a previous PR I changed the runner to parse JSON output.

This addresses some bugs that PR introduced, namely:
- stopping on the first test run failure, rather than continuing
- stripping out package (not test) failures
  • Loading branch information
cedric-cordenier authored Sep 13, 2023
1 parent 2f042f2 commit 72139e1
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 2 deletions.
14 changes: 12 additions & 2 deletions tools/flakeytests/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,23 @@ func parseOutput(readers ...io.Reader) (map[string]map[string]int, error) {
continue
}

// Skip the line if doesn't start with a "{" --
// this mean it isn't JSON output.
if !strings.HasPrefix(string(t), "{") {
continue
}

e, err := newEvent(t)
if err != nil {

return nil, err
}

// We're only interested in test failures, for which
// both Package and Test would be present.
if e.Package == "" || e.Test == "" {
continue
}

switch e.Action {
case "fail":
if tests[e.Package] == nil {
Expand Down Expand Up @@ -133,9 +140,12 @@ func (r *Runner) runTests(failedTests map[string]map[string]int) (io.Reader, err
err := r.runTestFn(pkg, ts, r.numReruns, &out)
if err != nil {
log.Printf("Test command errored: %s\n", err)
// There was an error because the command failed with a non-zero
// exit code. This could just mean that the test failed again, so let's
// keep going.
var exErr exitCoder
if errors.As(err, &exErr) && exErr.ExitCode() > 0 {
return &out, nil
continue
}
return &out, err
}
Expand Down
68 changes: 68 additions & 0 deletions tools/flakeytests/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,31 @@ func TestRunner_WithFlake(t *testing.T) {
assert.Equal(t, m.entries["core/assets"], []string{"TestLink"})
}

func TestRunner_WithFailedPackage(t *testing.T) {
output := `
{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/assets","Test":"TestLink","Elapsed":0}
{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/assets","Elapsed":0}
`
m := newMockReporter()
r := &Runner{
numReruns: 2,
readers: []io.Reader{strings.NewReader(output)},
runTestFn: func(pkg string, testNames []string, numReruns int, w io.Writer) error {
_, err := w.Write([]byte(output))
return err
},
parse: parseOutput,
reporter: m,
}

// This will report a flake since we've mocked the rerun
// to only report one failure (not two as expected).
err := r.Run()
require.NoError(t, err)
assert.Len(t, m.entries, 1)
assert.Equal(t, m.entries["core/assets"], []string{"TestLink"})
}

func TestRunner_AllFailures(t *testing.T) {
output := `{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/assets","Test":"TestLink","Elapsed":0}`

Expand Down Expand Up @@ -185,3 +210,46 @@ func TestRunner_RerunFailsWithNonzeroExitCode(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, m.entries["core/assets"], []string{"TestLink"})
}

func TestRunner_RerunWithNonZeroExitCodeDoesntStopCommand(t *testing.T) {
outputs := []io.Reader{
strings.NewReader(`
{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/assets","Test":"TestLink","Elapsed":0}
`),
strings.NewReader(`
{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/services/vrf/v2","Test":"TestMaybeReservedLinkV2","Elapsed":0}
`),
}

rerunOutputs := []string{
`
{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/assets","Test":"TestLink","Elapsed":0}
{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"pass","Package":"github.com/smartcontractkit/chainlink/v2/core/assets","Test":"TestLink","Elapsed":0}
`,
`
{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/services/vrf/v2","Test":"TestMaybeReservedLinkV2","Elapsed":0}
{"Time":"2023-09-07T15:39:46.378315+01:00","Action":"fail","Package":"github.com/smartcontractkit/chainlink/v2/core/services/vrf/v2","Test":"TestMaybeReservedLinkV2","Elapsed":0}
`,
}

index := 0
m := newMockReporter()
r := &Runner{
numReruns: 2,
readers: outputs,
runTestFn: func(pkg string, testNames []string, numReruns int, w io.Writer) error {

_, _ = w.Write([]byte(rerunOutputs[index]))
index++
return &exitError{}
},
parse: parseOutput,
reporter: m,
}

err := r.Run()
require.NoError(t, err)
calls := index
assert.Equal(t, 2, calls)
assert.Equal(t, m.entries["core/assets"], []string{"TestLink"})
}

0 comments on commit 72139e1

Please sign in to comment.