Return artificial events to Handler for missing events

This fixes a bug, and prevents one in the new rerun-fails feature.

Incomplete tests were printed in the summary, but never passed to the EventHandler. Now when the execution ends, fake TestEVent will be created and sent to the Handler, so that it can print the output of the failed test.

This allows the rerun-fails loop to include these failed tests in the count, and avoid a situation where a test was panicing, but not getting rerun. If the panic test did not get rerun then 'go test' may exit 0, making it look like everything had passed, when in fact one test was still panicing.

This type of bug sounds pretty severe.

do

@@ -8,7 +8,7 @@ binary() {
 }
 
 update-golden() {
-    gotestsum -- ./testjson ./internal/junitxml -test.update-golden
+    gotestsum -- . ./testjson ./internal/junitxml -test.update-golden
 }
 
 lint() {

handler.go

@@ -25,7 +25,8 @@ func (h *eventHandler) Err(text string) error {
 }
 
 func (h *eventHandler) Event(event testjson.TestEvent, execution *testjson.Execution) error {
-	if h.jsonFile != nil {
+	// ignore artificial events with no raw Bytes()
+	if h.jsonFile != nil && len(event.Bytes()) > 0 {
 		_, err := h.jsonFile.Write(append(event.Bytes(), '\n'))
 		if err != nil {
 			return errors.Wrap(err, "failed to write JSON file")

handler_test.go

@@ -46,3 +46,28 @@ func newExecFromTestData(t *testing.T) *testjson.Execution {
 	assert.NilError(t, err)
 	return exec
 }
+
+type bufferCloser struct {
+	bytes.Buffer
+}
+
+func (bufferCloser) Close() error { return nil }
+
+func TestEventHandler_Event_WithMissingActionFail(t *testing.T) {
+	buf := new(bufferCloser)
+	errBuf := new(bytes.Buffer)
+	format := testjson.NewEventFormatter(errBuf, "testname")
+
+	source := golden.Get(t, "../testjson/testdata/go-test-json-missing-test-fail.out")
+	cfg := testjson.ScanConfig{
+		Stdout:  bytes.NewReader(source),
+		Handler: &eventHandler{jsonFile: buf, formatter: format},
+	}
+	_, err := testjson.ScanTestOutput(cfg)
+	assert.NilError(t, err)
+
+	assert.Equal(t, buf.String(), string(source))
+	// confirm the artificial event was sent to the handler by checking the output
+	// of the formatter.
+	golden.Assert(t, errBuf.String(), "event-handler-missing-test-fail-expected")
+}

testdata/event-handler-missing-test-fail-expected

@@ -0,0 +1,20 @@
+FAIL gotest.tools/v3/poll
+=== RUN   TestWaitOn_WithCompare
+panic: runtime error: index out of range [1] with length 1
+
+goroutine 7 [running]:
+gotest.tools/v3/internal/assert.ArgsFromComparisonCall(0xc0000552a0, 0x1, 0x1, 0x1, 0x0, 0x0)
+	/home/daniel/pers/code/gotest.tools/internal/assert/result.go:102 +0x9f
+gotest.tools/v3/internal/assert.runComparison(0x6bcb80, 0xc00000e180, 0x67dee8, 0xc00007a9f0, 0x0, 0x0, 0x0, 0x7f7f4fb6d108)
+	/home/daniel/pers/code/gotest.tools/internal/assert/result.go:34 +0x2b1
+gotest.tools/v3/internal/assert.Eval(0x6bcb80, 0xc00000e180, 0x67dee8, 0x627660, 0xc00007a9f0, 0x0, 0x0, 0x0, 0x642c60)
+	/home/daniel/pers/code/gotest.tools/internal/assert/assert.go:56 +0x2e4
+gotest.tools/v3/poll.Compare(0xc00007a9f0, 0x6b74a0, 0x618a60)
+	/home/daniel/pers/code/gotest.tools/poll/poll.go:151 +0x81
+gotest.tools/v3/poll.TestWaitOn_WithCompare.func1(0x6be4c0, 0xc00016c240, 0xc00016c240, 0x6be4c0)
+	/home/daniel/pers/code/gotest.tools/poll/poll_test.go:81 +0x58
+gotest.tools/v3/poll.WaitOn.func1(0xc00001e3c0, 0x67df50, 0x6c1960, 0xc00016c240)
+	/home/daniel/pers/code/gotest.tools/poll/poll.go:125 +0x62
+created by gotest.tools/v3/poll.WaitOn
+	/home/daniel/pers/code/gotest.tools/poll/poll.go:124 +0x16f
+FAIL gotest.tools/v3/poll.TestWaitOn_WithCompare (-1.00s)

testjson/execution.go

@@ -200,12 +200,25 @@ func (p *Package) TestMainFailed() bool {
 
 const neverFinished time.Duration = -1
 
-func (p *Package) end() {
-	// Add tests that were missing an ActionFail event to Failed
+// end adds any tests that were missing an ActionFail TestEvent to the list of
+// Failed, and returns a slice of artificial TestEvent for the missing ones.
+//
+// This is done to work around 'go test' not sending the ActionFail TestEvents
+// in some cases, when a test panics.
+func (p *Package) end() []TestEvent {
+	result := make([]TestEvent, 0, len(p.running))
 	for _, tc := range p.running {
 		tc.Elapsed = neverFinished
 		p.Failed = append(p.Failed, tc)
+
+		result = append(result, TestEvent{
+			Action:  ActionFail,
+			Package: tc.Package,
+			Test:    tc.Test,
+			Elapsed: float64(neverFinished),
+		})
 	}
+	return result
 }
 
 // TestCase stores the name and elapsed time for a test case.
@@ -427,11 +440,13 @@ func (e *Execution) Errors() []string {
 	return e.errors
 }
 
-func (e *Execution) end() {
+func (e *Execution) end() []TestEvent {
 	e.done = true
+	var result []TestEvent // nolint: prealloc
 	for _, pkg := range e.packages {
-		pkg.end()
+		result = append(result, pkg.end()...)
 	}
+	return result
 }
 
 // newExecution returns a new Execution and records the current time as the
@@ -471,8 +486,6 @@ type EventHandler interface {
 // Execution, calls the Handler for each event, and returns the Execution.
 //
 // If config.Handler is nil, a default no-op handler will be used.
-//
-// TODO: should the Execution return value be removed
 func ScanTestOutput(config ScanConfig) (*Execution, error) {
 	if config.Stdout == nil {
 		return nil, fmt.Errorf("stdout reader must be non-nil")
@@ -494,9 +507,16 @@ func ScanTestOutput(config ScanConfig) (*Execution, error) {
 	group.Go(func() error {
 		return readStderr(config, execution)
 	})
-	err := group.Wait()
-	execution.end()
-	return execution, err
+	if err := group.Wait(); err != nil {
+		return execution, err
+	}
+	for _, event := range execution.end() {
+		if err := config.Handler.Event(event, execution); err != nil {
+			return execution, err
+		}
+	}
+
+	return execution, nil
 }
 
 func readStdout(config ScanConfig, execution *Execution) error {