Repository: gotestyourself/gotestsum
Pull request #129 (2020-06-11 03:28:41)

bug fixes for rerun fails
* add a `DONE` line after each attempt
* abort the rerun if any run has errors
* warn if `go test` exits with any code other than 0 or 1
* `./do update-golden` runs go1.13 in a container to update both sets of golden files
* some misc improvements to naming


Patch:
diff --git a/do b/do
index 03db7ba..503afd7 100755
--- 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
index 29afb5c..28c72ba 100644
--- a/flags.go
+++ b/flags.go
@@ -117,7 +117,7 @@ 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 {
@@ -125,7 +125,7 @@ func (s *stringSlice) String() string {
 }
 
 func (s *stringSlice) Set(raw string) error {
-	*s = append(*s, strings.Split(raw, " ")...)
+	*s = append(*s, strings.Fields(raw)...)
 	return nil
 }
 
diff --git a/main.go b/main.go
index d558693..1d4550a 100644
--- 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/rerunfails.go b/rerunfails.go
index 203af69..33b2833 100644
--- 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


Test patch:
diff --git a/flags_test.go b/flags_test.go
index 2a67c1f..817cb9d 100644
--- 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_e2e_test.go b/main_e2e_test.go
index 0a81b56..e9859af 100644
--- 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/testdata/e2e/expected/TestE2E_RerunFails/first_run_has_errors,_abort_rerun b/testdata/e2e/expected/TestE2E_RerunFails/first_run_has_errors,_abort_rerun
new file mode 100644
index 0000000..df56b5a
--- /dev/null
+++ 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
new file mode 100644
index 0000000..df56b5a
--- /dev/null
+++ 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
index c9ebffe..f5385e1 100644
--- 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
index 5a6de81..55b87c7 100644
--- 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
index cecc8fb..02f6ebe 100644
--- 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,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
@@ -36,12 +42,18 @@ SEED:  2
     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
index 337ba52..bb6144d 100644
--- 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,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
@@ -36,12 +42,18 @@ SEED:  2
 --- 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