WIP: Flaky failures should be reported separate from tests that fail on retry

cmd/goversion_go122.go

@@ -9,5 +9,5 @@ import (
 )
 
 func isGoVersionAtLeast(v string) bool {
-	return goversion.Compare(runtime.Version(), v) < 0
+	return goversion.Compare(v, runtime.Version()) < 0
 }

cmd/rerunfails.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"os"
 	"regexp"
-	"sort"
 	"strings"
 
 	"gotest.tools/gotestsum/testjson"
@@ -159,38 +158,6 @@ func writeRerunFailsReport(opts *options, exec *testjson.Execution) error {
 		return nil
 	}
 
-	type testCaseCounts struct {
-		total  int
-		failed int
-	}
-
-	names := []string{}
-	results := map[string]testCaseCounts{}
-	for _, failure := range exec.Failed() {
-		name := failure.Package + "." + failure.Test.Name()
-		if _, ok := results[name]; ok {
-			continue
-		}
-		names = append(names, name)
-
-		pkg := exec.Package(failure.Package)
-		counts := testCaseCounts{}
-
-		for _, tc := range pkg.Failed {
-			if tc.Test == failure.Test {
-				counts.total++
-				counts.failed++
-			}
-		}
-		for _, tc := range pkg.Passed {
-			if tc.Test == failure.Test {
-				counts.total++
-			}
-		}
-		// Skipped tests are not counted, but presumably skipped tests can not fail
-		results[name] = counts
-	}
-
 	fh, err := os.Create(opts.rerunFailsReportFile)
 	if err != nil {
 		return err
@@ -200,10 +167,15 @@ func writeRerunFailsReport(opts *options, exec *testjson.Execution) error {
 		_ = fh.Close()
 	}()
 
-	sort.Strings(names)
-	for _, name := range names {
-		counts := results[name]
-		fmt.Fprintf(fh, "%s: %d runs, %d failures\n", name, counts.total, counts.failed)
-	}
+	exec.Results().Each(func(key interface{}, value interface{}) {
+		name := key.(string)
+		tr := value.(testjson.TestResult)
+		if tr.IsFlaky() {
+			fmt.Fprintf(fh, "%s: FLAKY, failed in %d out of %d runs\n", name, len(tr.Failed), tr.Total())
+		} else if len(tr.Failed) > 0 {
+			fmt.Fprintf(fh, "%s: FAILED in all %d runs\n", name, len(tr.Failed))
+		}
+	})
+
 	return nil
 }

cmd/testdata/TestWriteRerunFailsReport-expected

@@ -1,3 +1,3 @@
-gotest.tools/gotestsum/testdata/e2e/flaky.TestFailsOften: 4 runs, 3 failures
-gotest.tools/gotestsum/testdata/e2e/flaky.TestFailsRarely: 2 runs, 1 failures
-gotest.tools/gotestsum/testdata/e2e/flaky.TestFailsSometimes: 3 runs, 2 failures
+gotest.tools/gotestsum/testdata/e2e/flaky.TestFailsOften: FLAKY, failed in 3 out of 4 runs
+gotest.tools/gotestsum/testdata/e2e/flaky.TestFailsRarely: FLAKY, failed in 1 out of 2 runs
+gotest.tools/gotestsum/testdata/e2e/flaky.TestFailsSometimes: FLAKY, failed in 2 out of 3 runs

go.mod

@@ -3,6 +3,7 @@ module gotest.tools/gotestsum
 require (
 	github.com/bitfield/gotestdox v0.2.2
 	github.com/dnephin/pflag v1.0.7
+	github.com/emirpasic/gods v1.18.1
 	github.com/fatih/color v1.17.0
 	github.com/fsnotify/fsnotify v1.7.0
 	github.com/google/go-cmp v0.6.0

go.sum

@@ -2,6 +2,8 @@ github.com/bitfield/gotestdox v0.2.2 h1:x6RcPAbBbErKLnapz1QeAlf3ospg8efBsedU93CD
 github.com/bitfield/gotestdox v0.2.2/go.mod h1:D+gwtS0urjBrzguAkTM2wodsTQYFHdpx8eqRJ3N+9pY=
 github.com/dnephin/pflag v1.0.7 h1:oxONGlWxhmUct0YzKTgrpQv9AUA1wtPBn7zuSjJqptk=
 github.com/dnephin/pflag v1.0.7/go.mod h1:uxE91IoWURlOiTUIA8Mq5ZZkAv3dPUfZNaT80Zm7OQE=
+github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc=
+github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ=
 github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw=
 github.com/fatih/color v1.17.0 h1:GlRw1BRJxkpqUCBKzKOw098ed57fEsKeNjpTe3cSjK4=
 github.com/fatih/color v1.17.0/go.mod h1:YZ7TlrGPkiz6ku9fK3TLD/pl3CpsiFyu8N92HLgmosI=

testjson/execution.go

@@ -12,6 +12,8 @@ import (
 	"sync"
 	"time"
 
+	"github.com/emirpasic/gods/maps/treemap"
+	"github.com/emirpasic/gods/utils"
 	"golang.org/x/sync/errgroup"
 	"gotest.tools/gotestsum/internal/log"
 )
@@ -316,6 +318,20 @@ func rootTestPassed(p *Package, subtest TestCase) bool {
 	return false
 }
 
+type TestResult struct {
+	Failed []TestCase
+	Passed []TestCase
+}
+
+// isFlaky checks if any test case is present in both Failed and Passed slices.
+func (tr *TestResult) IsFlaky() bool {
+	return len(tr.Passed) > 0 && len(tr.Failed) > 0
+}
+
+func (tr *TestResult) Total() int {
+	return len(tr.Passed) + len(tr.Failed)
+}
+
 // TestCase stores the name and elapsed time for a test case.
 type TestCase struct {
 	// ID is unique ID for each test case. A test run may include multiple instances
@@ -335,6 +351,11 @@ type TestCase struct {
 	Time time.Time
 }
 
+// FQN returns the fully qualified name of the test case.
+func (tc *TestCase) FullyQualifiedName() string {
+	return tc.Package + "." + tc.Test.Name()
+}
+
 func newPackage() *Package {
 	return &Package{
 		output:   make(map[int][]string),
@@ -545,6 +566,54 @@ func (e *Execution) Failed() []TestCase {
 	return failed
 }
 
+// Collapse all test cases by by unique {package.test_name}
+func (e *Execution) Results() *treemap.Map {
+
+	// Skipped tests are not included in results
+	results := treemap.NewWith(utils.StringComparator)
+
+	for _, pkgName := range sortedKeys(e.packages) {
+		pkg := e.packages[pkgName]
+
+		if pkg.TestMainFailed() {
+			result := TestResult{}
+			result.Failed = append(result.Failed, TestCase{Package: pkgName})
+			results.Put(pkgName, result)
+			continue
+		}
+
+		for _, tc := range pkg.Failed {
+			name := tc.FullyQualifiedName()
+
+			// Check if the name exists in the results map
+			if _, ok := results.Get(name); !ok {
+				// If it doesn't exist, create a new TestResult and add it to the map
+				results.Put(name, TestResult{})
+			}
+			value, _ := results.Get(name)
+			tr := value.(TestResult)
+			tr.Failed = append(tr.Failed, tc)
+			results.Put(name, tr)
+		}
+
+		for _, tc := range pkg.Passed {
+			name := tc.FullyQualifiedName()
+
+			// Check if the name exists in the results map
+			if _, ok := results.Get(name); !ok {
+				// If it doesn't exist, create a new TestResult and add it to the map
+				results.Put(name, TestResult{})
+			}
+			value, _ := results.Get(name)
+			tr := value.(TestResult)
+			tr.Passed = append(tr.Passed, tc)
+			results.Put(name, tr)
+		}
+	}
+
+	return results
+}
+
 // FilterFailedUnique filters a slice of failed TestCases to remove any parent
 // tests that have failed subtests. The parent test will always be run when
 // running any of its subtests.

testjson/summary.go

@@ -81,11 +81,24 @@ func PrintSummary(out io.Writer, execution *Execution, opts Summary) {
 		writeErrorSummary(out, errors)
 	}
 
-	fmt.Fprintf(out, "\n%s %d tests%s%s%s in %s\n",
+	var total, failed, flaky int
+	execution.Results().Each(func(key interface{}, value interface{}) {
+		tr := value.(TestResult)
+		if tr.IsFlaky() {
+			flaky++
+		} else if len(tr.Failed) > 0 {
+			failed++
+		}
+		total++
+	})
+	total = total + len(execution.Skipped())
+
+	fmt.Fprintf(out, "\n%s %d tests%s%s%s%s in %s\n",
 		formatExecStatus(execution),
-		execution.Total(),
+		total,
 		formatTestCount(len(execution.Skipped()), "skipped", ""),
-		formatTestCount(len(execution.Failed()), "failure", "s"),
+		formatTestCount(flaky, "flaky", ""),
+		formatTestCount(failed, "failed", ""),
 		formatTestCount(countErrors(errors), "error", "s"),
 		FormatDurationAsSeconds(execution.Elapsed(), 3))
 }

testjson/summary_test.go

@@ -3,6 +3,7 @@ package testjson
 import (
 	"bytes"
 	"io"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -51,13 +52,18 @@ func TestPrintSummary_NoFailures(t *testing.T) {
 	out := new(bytes.Buffer)
 	start := time.Now()
 	exec := &Execution{
-		started: start,
-		done:    true,
-		packages: map[string]*Package{
-			"foo":   {Total: 12},
-			"other": {Total: 1},
-		},
+		started:  start,
+		done:     true,
+		packages: map[string]*Package{},
 	}
+	foo := Package{Total: 12}
+	foo.Passed = createTestCases("foo", &foo, 12)
+	exec.packages["foo"] = &foo
+
+	other := Package{Total: 1}
+	other.Passed = createTestCases("other", &other, 1)
+	exec.packages["other"] = &other
+
 	timeNow = func() time.Time {
 		return start.Add(34123111 * time.Microsecond)
 	}
@@ -67,6 +73,18 @@ func TestPrintSummary_NoFailures(t *testing.T) {
 	assert.Equal(t, out.String(), expected)
 }
 
+func createTestCases(pkgName string, pkg *Package, count int) []TestCase {
+	var tests []TestCase
+	for i := 0; i < count; i++ {
+		tests = append(tests, TestCase{
+			Package: pkgName,
+			Test:    TestName("Test" + strconv.Itoa(i)),
+			ID:      1,
+		})
+	}
+	return tests
+}
+
 func TestPrintSummary_WithFailures(t *testing.T) {
 	patchPkgPathPrefix(t, "example.com")
 	patchTimeNow(t)
@@ -174,7 +192,7 @@ Some stdout/stderr here
 === Errors
 pkg/file.go:99:12: missing ',' before newline
 
-DONE 13 tests, 1 skipped, 4 failures, 1 error in 34.123s
+DONE 5 tests, 1 skipped, 4 failed, 1 error in 34.123s
 `
 		assert.Equal(t, out.String(), expected)
 	})
@@ -196,7 +214,7 @@ DONE 13 tests, 1 skipped, 4 failures, 1 error in 34.123s
 === Errors
 pkg/file.go:99:12: missing ',' before newline
 
-DONE 13 tests, 1 skipped, 4 failures, 1 error in 34.123s
+DONE 5 tests, 1 skipped, 4 failed, 1 error in 34.123s
 `
 		assert.Equal(t, out.String(), expected)
 	})
@@ -209,7 +227,7 @@ DONE 13 tests, 1 skipped, 4 failures, 1 error in 34.123s
 === Errors
 pkg/file.go:99:12: missing ',' before newline
 
-DONE 13 tests, 1 skipped, 4 failures, 1 error in 34.123s
+DONE 5 tests, 1 skipped, 4 failed, 1 error in 34.123s
 `
 		assert.Equal(t, out.String(), expected)
 	})