Merge pull request #156 from dnephin/fix-panic-in-fails-report

Fix panic caused by missing Action=run event

cmd/rerunfails_test.go

@@ -36,6 +36,28 @@ func TestWriteRerunFailsReport(t *testing.T) {
 	golden.Assert(t, string(raw), t.Name()+"-expected")
 }
 
+func TestWriteRerunFailsReport_HandlesMissingActionRunEvents(t *testing.T) {
+	reportFile := fs.NewFile(t, t.Name())
+	defer reportFile.Remove()
+
+	opts := &options{
+		rerunFailsReportFile:  reportFile.Path(),
+		rerunFailsMaxAttempts: 4,
+	}
+
+	exec, err := testjson.ScanTestOutput(testjson.ScanConfig{
+		Stdout: bytes.NewReader(golden.Get(t, "go-test-missing-run-events.out")),
+	})
+	assert.NilError(t, err)
+
+	err = writeRerunFailsReport(opts, exec)
+	assert.NilError(t, err)
+
+	raw, err := ioutil.ReadFile(reportFile.Path())
+	assert.NilError(t, err)
+	golden.Assert(t, string(raw), t.Name()+"-expected")
+}
+
 func TestGoTestRunFlagFromTestCases(t *testing.T) {
 	type testCase struct {
 		input    string

cmd/testdata/TestWriteRerunFailsReport_HandlesMissingActionRunEvents-expected

@@ -0,0 +1,3 @@
+github.com/hashicorp/consul/test/integration/connect/envoy.TestEnvoy: 5 runs, 5 failures
+github.com/hashicorp/consul/test/integration/connect/envoy.TestEnvoy/case-ent-cross-namespaces: 3 runs, 3 failures
+github.com/hashicorp/consul/test/integration/connect/envoy.TestEnvoy/case-ent-intra-namespace: 3 runs, 3 failures

go.mod

@@ -5,7 +5,7 @@ replace github.com/spf13/pflag => github.com/dnephin/pflag v0.0.0-20200521001137
 require (
 	github.com/fatih/color v1.9.0
 	github.com/fsnotify/fsnotify v1.4.9
-	github.com/google/go-cmp v0.4.0
+	github.com/google/go-cmp v0.5.2
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
 	github.com/jonboulle/clockwork v0.1.0
 	github.com/mattn/go-colorable v0.1.6 // indirect

go.sum

@@ -6,6 +6,8 @@ github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWo
 github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
 github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM=
+github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 h1:El6M4kTTCOh6aBiKaUGG7oYTSPP8MxqL4YI3kZKwcP4=
 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3pAXIBCelhxNneeOaAeabZDe5s4K6zSpQ=
 github.com/jonboulle/clockwork v0.1.0 h1:VKV+ZcuP6l3yW9doeqz6ziZGgcynBVQO+obU0+0hcPo=

testjson/execution.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/jonboulle/clockwork"
@@ -261,11 +262,12 @@ func newPackage() *Package {
 
 // Execution of one or more test packages
 type Execution struct {
-	started   time.Time
-	packages  map[string]*Package
-	errors    []string
-	done      bool
-	lastRunID int
+	started    time.Time
+	packages   map[string]*Package
+	errorsLock sync.RWMutex
+	errors     []string
+	done       bool
+	lastRunID  int
 }
 
 func (e *Execution) add(event TestEvent) {
@@ -296,27 +298,40 @@ func (e *Execution) addPackageEvent(pkg *Package, event TestEvent) {
 	}
 }
 
-func (p *Package) addTestEvent(event TestEvent) {
-	root, _ := TestName(event.Test).Split()
+func (p *Package) newTestCaseFromEvent(event TestEvent) TestCase {
+	// Incremental total before using it as the ID, because ID 0 is used for
+	// the package output
+	p.Total++
+	return TestCase{
+		Package: event.Package,
+		Test:    TestName(event.Test),
+		ID:      p.Total,
+		RunID:   event.RunID,
+	}
+}
 
-	switch event.Action {
-	case ActionRun:
-		// Incremental total before using it as the ID, because ID 0 is used for
-		// the package output
-		p.Total++
-		tc := TestCase{
-			Package: event.Package,
-			Test:    TestName(event.Test),
-			ID:      p.Total,
-			RunID:   event.RunID,
-		}
+func (p *Package) addTestEvent(event TestEvent) {
+	if event.Action == ActionRun {
+		tc := p.newTestCaseFromEvent(event)
 		p.running[event.Test] = tc
 
 		if tc.Test.IsSubTest() {
+			root, _ := TestName(event.Test).Split()
 			rootID := p.running[root].ID
 			p.subTests[rootID] = append(p.subTests[rootID], tc.ID)
 		}
 		return
+	}
+
+	tc := p.running[event.Test]
+	// This appears to be a bug in 'go test' or test2json. This test is missing
+	// an Action=run event. Create one on the first event received from the test.
+	if tc.ID == 0 {
+		tc = p.newTestCaseFromEvent(event)
+		p.running[event.Test] = tc
+	}
+
+	switch event.Action {
 	case ActionOutput, ActionBench:
 		tc := p.running[event.Test]
 		p.addOutput(tc.ID, event.Output)
@@ -326,7 +341,6 @@ func (p *Package) addTestEvent(event TestEvent) {
 	}
 
 	// the event.Action must be one of the three test end events
-	tc := p.running[event.Test]
 	delete(p.running, event.Test)
 	tc.Elapsed = elapsedDuration(event.Elapsed)
 
@@ -336,6 +350,7 @@ func (p *Package) addTestEvent(event TestEvent) {
 
 		// If this is a subtest, mark the root test as having a failed subtest
 		if tc.Test.IsSubTest() {
+			root, _ := TestName(event.Test).Split()
 			rootTestCase := p.running[root]
 			rootTestCase.hasSubTestFailed = true
 			p.running[root] = rootTestCase
@@ -462,11 +477,15 @@ func (e *Execution) addError(err string) {
 	if strings.HasPrefix(err, "# ") {
 		return
 	}
+	e.errorsLock.Lock()
 	e.errors = append(e.errors, err)
+	e.errorsLock.Unlock()
 }
 
 // Errors returns a list of all the errors.
 func (e *Execution) Errors() []string {
+	e.errorsLock.RLock()
+	defer e.errorsLock.RUnlock()
 	return e.errors
 }
 

testjson/format_test.go

@@ -118,6 +118,7 @@ var expectedExecution = &Execution{
 var cmpExecutionShallow = gocmp.Options{
 	gocmp.AllowUnexported(Execution{}, Package{}),
 	gocmp.FilterPath(stringPath("started"), opt.TimeWithThreshold(10*time.Second)),
+	cmpopts.IgnoreFields(Execution{}, "errorsLock"),
 	cmpopts.EquateEmpty(),
 	cmpPackageShallow,
 }

testjson/summary.go

@@ -150,7 +150,7 @@ type executionSummary interface {
 }
 
 type noOutputSummary struct {
-	Execution
+	*Execution
 }
 
 func (s *noOutputSummary) OutputLines(_ TestCase) []string {
@@ -161,7 +161,7 @@ func newExecSummary(execution *Execution, opts Summary) executionSummary {
 	if opts.Includes(SummarizeOutput) {
 		return execution
 	}
-	return &noOutputSummary{Execution: *execution}
+	return &noOutputSummary{Execution: execution}
 }
 
 func writeTestCaseSummary(out io.Writer, execution executionSummary, conf testCaseFormatConfig) {