Merge pull request #129 from gotestyourself/rerun-fails

bug fixes for rerun fails
gotestyourself · Jun 12, 2020 · e55d7aa · e55d7aa
2 parents dcc31b5 + f93909c
commit e55d7aa
Show file tree

Hide file tree

Showing 12 changed files with 103 additions and 12 deletions.
diff --git a/do b/do
@@ -8,6 +8,11 @@ binary() {
 }
 
 update-golden() {
+    _update-golden
+    GOLANG_VERSION=1.13-alpine ./do shell bash -c 'go build; PATH=$PATH:. ./do _update-golden'
+}
+
+_update-golden() {
     gotestsum -- . ./testjson ./internal/junitxml ./cmd/tool/slowest -test.update-golden
 }
 

diff --git a/flags.go b/flags.go
@@ -117,15 +117,15 @@ func (c *commandValue) Value() []string {
 var _ pflag.Value = (*stringSlice)(nil)
 
 // stringSlice is a flag.Value which populates the string slice by splitting
-// the raw flag value on spaces.
+// the raw flag value on whitespace.
 type stringSlice []string
 
 func (s *stringSlice) String() string {
 	return strings.Join(*s, " ")
 }
 
 func (s *stringSlice) Set(raw string) error {
-	*s = append(*s, strings.Split(raw, " ")...)
+	*s = append(*s, strings.Fields(raw)...)
 	return nil
 }
 

diff --git a/flags_test.go b/flags_test.go
@@ -25,3 +25,11 @@ func TestNoSummaryValue_SetAndString(t *testing.T) {
 		assert.ErrorContains(t, value.Set("bogus"), "must be one or more of")
 	})
 }
+
+func TestStringSlice(t *testing.T) {
+	value := "one \ntwo  three\n\tfour\t five   \n"
+	var v []string
+	ss := (*stringSlice)(&v)
+	assert.NilError(t, ss.Set(value))
+	assert.DeepEqual(t, v, []string{"one", "two", "three", "four", "five"})
+}
diff --git a/main.go b/main.go
@@ -210,7 +210,7 @@ func run(opts *options) error {
 		return err
 	}
 	goTestExitErr := goTestProc.cmd.Wait()
-	if opts.rerunFailsMaxAttempts > 0 {
+	if goTestExitErr != nil && opts.rerunFailsMaxAttempts > 0 {
 		cfg := testjson.ScanConfig{Execution: exec, Handler: handler}
 		goTestExitErr = rerunFailed(ctx, opts, cfg)
 	}

diff --git a/main_e2e_test.go b/main_e2e_test.go
@@ -3,6 +3,7 @@ package main
 import (
 	"bytes"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"testing"
@@ -18,7 +19,7 @@ func TestE2E_RerunFails(t *testing.T) {
 	type testCase struct {
 		name        string
 		args        []string
-		expectedErr bool
+		expectedErr string
 	}
 	fn := func(t *testing.T, tc testCase) {
 		tmpFile := fs.NewFile(t, t.Name()+"-seedfile", fs.WithContent("0"))
@@ -38,16 +39,17 @@ func TestE2E_RerunFails(t *testing.T) {
 		opts.stderr = bufStderr
 
 		err := run(opts)
-		if tc.expectedErr {
-			assert.Error(t, err, "exit status 1")
+		if tc.expectedErr != "" {
+			assert.Error(t, err, tc.expectedErr)
 		} else {
 			assert.NilError(t, err)
 		}
 		out := text.ProcessLines(t, bufStdout,
 			text.OpRemoveSummaryLineElapsedTime,
-			text.OpRemoveTestElapsedTime)
+			text.OpRemoveTestElapsedTime,
+			filepath.ToSlash, // for windows
+		)
 		golden.Assert(t, out, expectedFilename(t.Name()))
-		assert.Equal(t, bufStderr.String(), "")
 	}
 	var testCases = []testCase{
 		{
@@ -67,7 +69,17 @@ func TestE2E_RerunFails(t *testing.T) {
 				"--packages=./testdata/e2e/flaky/",
 				"--", "-count=1", "-tags=testdata",
 			},
-			expectedErr: true,
+			expectedErr: "exit status 1",
+		},
+		{
+			name: "first run has errors, abort rerun",
+			args: []string{
+				"-f=testname",
+				"--rerun-fails=2",
+				"--packages=./testjson/internal/broken",
+				"--", "-count=1", "-tags=stubpkg",
+			},
+			expectedErr: "rerun aborted because previous run had errors",
 		},
 	}
 	for _, tc := range testCases {

diff --git a/rerunfails.go b/rerunfails.go
@@ -6,6 +6,7 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+	"gotest.tools/gotestsum/log"
 	"gotest.tools/gotestsum/testjson"
 )
 
@@ -32,13 +33,17 @@ func rerunFailed(ctx context.Context, opts *options, scanConfig testjson.ScanCon
 			"number of test failures (%d) exceeds maximum (%d) set by --rerun-fails-max-failures",
 			failed, opts.rerunFailsMaxInitialFailures)
 	}
+	if err := hasErrors(scanConfig.Execution); err != nil {
+		return err
+	}
 
 	rec := newFailureRecorderFromExecution(scanConfig.Execution)
-
 	var lastErr error
-	for count := 0; rec.count() > 0 && count < opts.rerunFailsMaxAttempts; count++ {
-		nextRec := newFailureRecorder(scanConfig.Handler)
+	for attempts := 0; rec.count() > 0 && attempts < opts.rerunFailsMaxAttempts; attempts++ {
+		testjson.PrintSummary(opts.stdout, scanConfig.Execution, testjson.SummarizeNone)
+		opts.stdout.Write([]byte("\n")) // nolint: errcheck
 
+		nextRec := newFailureRecorder(scanConfig.Handler)
 		for pkg, testCases := range rec.pkgFailures {
 			rerun := rerunOpts{
 				runFlag: goTestRunFlagFromTestCases(testCases),
@@ -59,12 +64,27 @@ func rerunFailed(ctx context.Context, opts *options, scanConfig testjson.ScanCon
 				return err
 			}
 			lastErr = goTestProc.cmd.Wait()
+			// 0 and 1 are expected.
+			if ExitCodeWithDefault(lastErr) > 1 {
+				log.Warnf("unexpected go test exit code: %v", lastErr)
+				// TODO: will 'go test' exit with 2 if it panics? maybe return err here.
+			}
+			if err := hasErrors(scanConfig.Execution); err != nil {
+				return err
+			}
 			rec = nextRec
 		}
 	}
 	return lastErr
 }
 
+func hasErrors(exec *testjson.Execution) error {
+	if len(exec.Errors()) > 0 {
+		return fmt.Errorf("rerun aborted because previous run had errors")
+	}
+	return nil
+}
+
 type failureRecorder struct {
 	testjson.EventHandler
 	pkgFailures map[string][]string

diff --git a/testdata/e2e/expected/TestE2E_RerunFails/first_run_has_errors,_abort_rerun b/testdata/e2e/expected/TestE2E_RerunFails/first_run_has_errors,_abort_rerun
@@ -0,0 +1,5 @@
+
+=== Errors
+testjson/internal/broken/broken.go:5:21: undefined: somepackage
+
+DONE 0 tests, 1 error
diff --git a/testdata/e2e/expected/TestE2E_RerunFails/first_run_has_errors,_abort_rerun-go1.14 b/testdata/e2e/expected/TestE2E_RerunFails/first_run_has_errors,_abort_rerun-go1.14
@@ -0,0 +1,5 @@
+
+=== Errors
+testjson/internal/broken/broken.go:5:21: undefined: somepackage
+
+DONE 0 tests, 1 error
diff --git a/testdata/e2e/expected/TestE2E_RerunFails/reruns_continues_to_fail b/testdata/e2e/expected/TestE2E_RerunFails/reruns_continues_to_fail
@@ -17,6 +17,9 @@ FAIL testdata/e2e/flaky.TestFailsOften
 PASS testdata/e2e/flaky.TestFailsOftenDoesNotPrefixMatch
 PASS testdata/e2e/flaky.TestFailsSometimesDoesNotPrefixMatch
 FAIL testdata/e2e/flaky
+
+DONE 6 tests, 3 failures
+
 PASS testdata/e2e/flaky.TestFailsRarely
 === RUN   TestFailsSometimes
 SEED:  1
@@ -29,6 +32,9 @@ SEED:  1
     flaky_test.go:65: not this time
 FAIL testdata/e2e/flaky.TestFailsOften
 FAIL testdata/e2e/flaky
+
+DONE 9 tests, 5 failures
+
 PASS testdata/e2e/flaky.TestFailsSometimes
 === RUN   TestFailsOften
 SEED:  2

diff --git a/testdata/e2e/expected/TestE2E_RerunFails/reruns_continues_to_fail-go1.14 b/testdata/e2e/expected/TestE2E_RerunFails/reruns_continues_to_fail-go1.14
@@ -17,6 +17,9 @@ FAIL testdata/e2e/flaky.TestFailsOften
 PASS testdata/e2e/flaky.TestFailsOftenDoesNotPrefixMatch
 PASS testdata/e2e/flaky.TestFailsSometimesDoesNotPrefixMatch
 FAIL testdata/e2e/flaky
+
+DONE 6 tests, 3 failures
+
 PASS testdata/e2e/flaky.TestFailsRarely
 === RUN   TestFailsSometimes
 SEED:  1
@@ -29,6 +32,9 @@ SEED:  1
 --- FAIL: TestFailsOften
 FAIL testdata/e2e/flaky.TestFailsOften
 FAIL testdata/e2e/flaky
+
+DONE 9 tests, 5 failures
+
 PASS testdata/e2e/flaky.TestFailsSometimes
 === RUN   TestFailsOften
 SEED:  2

diff --git a/testdata/e2e/expected/TestE2E_RerunFails/reruns_until_success b/testdata/e2e/expected/TestE2E_RerunFails/reruns_until_success
@@ -17,6 +17,9 @@ FAIL testdata/e2e/flaky.TestFailsOften
 PASS testdata/e2e/flaky.TestFailsOftenDoesNotPrefixMatch
 PASS testdata/e2e/flaky.TestFailsSometimesDoesNotPrefixMatch
 FAIL testdata/e2e/flaky
+
+DONE 6 tests, 3 failures
+
 PASS testdata/e2e/flaky.TestFailsRarely
 === RUN   TestFailsSometimes
 SEED:  1
@@ -29,19 +32,28 @@ SEED:  1
     flaky_test.go:65: not this time
 FAIL testdata/e2e/flaky.TestFailsOften
 FAIL testdata/e2e/flaky
+
+DONE 9 tests, 5 failures
+
 PASS testdata/e2e/flaky.TestFailsSometimes
 === RUN   TestFailsOften
 SEED:  2
 --- FAIL: TestFailsOften
     flaky_test.go:65: not this time
 FAIL testdata/e2e/flaky.TestFailsOften
 FAIL testdata/e2e/flaky
+
+DONE 11 tests, 6 failures
+
 === RUN   TestFailsOften
 SEED:  3
 --- FAIL: TestFailsOften
     flaky_test.go:65: not this time
 FAIL testdata/e2e/flaky.TestFailsOften
 FAIL testdata/e2e/flaky
+
+DONE 12 tests, 7 failures
+
 PASS testdata/e2e/flaky.TestFailsOften
 PASS testdata/e2e/flaky
 

diff --git a/testdata/e2e/expected/TestE2E_RerunFails/reruns_until_success-go1.14 b/testdata/e2e/expected/TestE2E_RerunFails/reruns_until_success-go1.14
@@ -17,6 +17,9 @@ FAIL testdata/e2e/flaky.TestFailsOften
 PASS testdata/e2e/flaky.TestFailsOftenDoesNotPrefixMatch
 PASS testdata/e2e/flaky.TestFailsSometimesDoesNotPrefixMatch
 FAIL testdata/e2e/flaky
+
+DONE 6 tests, 3 failures
+
 PASS testdata/e2e/flaky.TestFailsRarely
 === RUN   TestFailsSometimes
 SEED:  1
@@ -29,19 +32,28 @@ SEED:  1
 --- FAIL: TestFailsOften
 FAIL testdata/e2e/flaky.TestFailsOften
 FAIL testdata/e2e/flaky
+
+DONE 9 tests, 5 failures
+
 PASS testdata/e2e/flaky.TestFailsSometimes
 === RUN   TestFailsOften
 SEED:  2
     TestFailsOften: flaky_test.go:65: not this time
 --- FAIL: TestFailsOften
 FAIL testdata/e2e/flaky.TestFailsOften
 FAIL testdata/e2e/flaky
+
+DONE 11 tests, 6 failures
+
 === RUN   TestFailsOften
 SEED:  3
     TestFailsOften: flaky_test.go:65: not this time
 --- FAIL: TestFailsOften
 FAIL testdata/e2e/flaky.TestFailsOften
 FAIL testdata/e2e/flaky
+
+DONE 12 tests, 7 failures
+
 PASS testdata/e2e/flaky.TestFailsOften
 PASS testdata/e2e/flaky