render-tests: Remove test output from flaky test reporter (#27919)

* render-tests: Remove test output from flaky test reporter

Remove the output from the summary output by render-tests in the
flakiness reporting mode. Sometimes this output is very large depending
on the test that failed, and we fail to post it to Slack due to the
length. This only changes what is written to the summary file. Stdout
still contains the failed test output.

When posted to Slack, the message will have a link to the run logs which
will contain the failed test output. This output will be sorted
alphabetically by package/test name and not by failure rate, as it was
before.

* render-tests: Print number of failure, not percentage

Print the number of failures from the total number of tests in the
flakiness report instead of percentages. Since we run less than 100 test
runs, the percentage is not that useful. It is more useful to see that a
test failed only once, for example, not 14.1% of the time.

* render-tests: Fix expected/actual ordering in tests

Fix the tests for render-tests where expected/actual params were
backwards.

* render-tests: Dont collect test output against package

Keep individual test output separate from package-level output.
Previously, all individual test output was recorded against a package
too and only the package output was printed if no tests in the package
failed. This meant we lost output for package-level failures such as
data races and crashes (segfaults) when there was an individual test
failure too.

There was no reason I could tell why it was done this way, so remove it.
This should now print all test failures.

* render-tests: Include packages in top flakiness summary

Include packages as well as individual tests in the flakiness summary,
as packages can have failures where no individual tests failed, such as
data races or crashes - both of these are recorded as a package-level
failure and not against individual tests. Without these in the summary,
we miss some failures.

* Change (M from N) to (M/N) in flaky summary

build.assets/tooling/cmd/render-tests/main.go

@@ -118,9 +118,9 @@ func main() {
 		rr.printFlakinessSummary(summaryOut)
 	} else {
 		rr.printSummary(summaryOut)
-		fmt.Fprintln(os.Stdout, separator)
-		rr.printFailedTestOutput(os.Stdout)
 	}
+	fmt.Fprintln(os.Stdout, separator)
+	rr.printFailedTestOutput(os.Stdout)
 
 	if rr.testCount.fail == 0 {
 		os.Exit(0)

build.assets/tooling/cmd/render-tests/result.go

@@ -170,7 +170,6 @@ func (rr *runResult) printSummary(out io.Writer) {
 }
 
 func (rr *runResult) printFlakinessSummary(out io.Writer) {
-	fmt.Fprintln(out, separator)
 	if rr.testCount.fail == 0 {
 		fmt.Fprintln(out, "No flaky tests!")
 		return
@@ -187,6 +186,15 @@ func (rr *runResult) printFlakinessSummary(out io.Writer) {
 				alltests = append(alltests, test)
 			}
 		}
+		// Create a pseudo-test result for the package level output
+		// as it can contain relevant output not included in individual
+		// tests such as crash or data race output.
+		tr := &testResult{
+			name:   pkg.name,
+			count:  pkg.count,
+			output: pkg.output,
+		}
+		alltests = append(alltests, tr)
 	}
 	// reverse sort by failure rate
 	sort.Slice(alltests, func(i, j int) bool {
@@ -196,18 +204,8 @@ func (rr *runResult) printFlakinessSummary(out io.Writer) {
 		if rr.top != 0 && i >= rr.top {
 			break
 		}
-		fmt.Fprintf(out, "FAIL(%3.1f%%): %s\n", test.count.failureRate()*100, test.name)
+		fmt.Fprintf(out, "FAIL(%d/%d): %s\n", test.count.fail, test.count.total, test.name)
 	}
-
-	fmt.Fprintln(out, separator)
-
-	for i, test := range alltests {
-		if rr.top != 0 && i >= rr.top {
-			break
-		}
-		printOutput(out, test.name, test.output)
-	}
-
 }
 
 // printFailedTests prints a summary list of the failed tests and packages in
@@ -240,19 +238,15 @@ func (rr *runResult) printFailedTestOutput(out io.Writer) {
 	sort.Slice(pkgs, func(i, j int) bool { return pkgs[i].name < pkgs[j].name })
 
 	for _, pkg := range pkgs {
-		testPrinted := false
 		if pkg.count.fail == 0 {
 			continue
 		}
+		printOutput(out, pkg.name, pkg.output)
 		for _, test := range pkg.tests {
 			if test.count.fail == 0 {
 				continue
 			}
 			printOutput(out, test.name, test.output)
-			testPrinted = true
-		}
-		if !testPrinted {
-			printOutput(out, pkg.name, pkg.output)
 		}
 	}
 }
@@ -267,17 +261,6 @@ func printOutput(out io.Writer, test string, output []string) {
 }
 
 func (pr *packageResult) processTestEvent(te TestEvent) {
-	if te.Action == actionOutput {
-		// Record the output of package AND test against the package
-		// TODO(camh): Why? not sure that makes sense
-
-		// Only append output if no failures. We only record the output
-		// of the first failure so we don't store too much redundant output.
-		if pr.count.fail == 0 {
-			pr.output = append(pr.output, te.Output)
-		}
-	}
-
 	if te.Test != "" {
 		tst := pr.getTest(pr.name + "." + te.Test)
 		tst.processTestEvent(te)
@@ -285,6 +268,11 @@ func (pr *packageResult) processTestEvent(te TestEvent) {
 	}
 
 	if te.Action == actionOutput {
+		// Only append output if no failures. We only record the output
+		// of the first failure so we don't store too much redundant output.
+		if pr.count.fail == 0 {
+			pr.output = append(pr.output, te.Output)
+		}
 		if matches := covPattern.FindStringSubmatch(te.Output); len(matches) > 0 {
 			value, err := strconv.ParseFloat(matches[1], 64)
 			if err != nil {

build.assets/tooling/cmd/render-tests/result_test.go

@@ -64,16 +64,16 @@ func TestStatus(t *testing.T) {
 	rr := newRunResult(byPackage, 0)
 	feedEvents(t, rr, passFailSkip)
 
-	require.Equal(t, rr.testCount.pass, 1)
-	require.Equal(t, rr.testCount.fail, 1)
-	require.Equal(t, rr.testCount.skip, 1)
-	require.Equal(t, rr.pkgCount.fail, 1)
+	require.Equal(t, 1, rr.testCount.pass)
+	require.Equal(t, 1, rr.testCount.fail)
+	require.Equal(t, 1, rr.testCount.skip)
+	require.Equal(t, 1, rr.pkgCount.fail)
 	pkgname := "example.com/package"
 	pkg := rr.packages[pkgname]
-	require.Equal(t, pkg.count.fail, 1)
-	require.Equal(t, pkg.tests[pkgname+".TestEmpty"].count.pass, 1)
-	require.Equal(t, pkg.tests[pkgname+".TestParse"].count.fail, 1)
-	require.Equal(t, pkg.tests[pkgname+".TestParseHostPort"].count.skip, 1)
+	require.Equal(t, 1, pkg.count.fail)
+	require.Equal(t, 1, pkg.tests[pkgname+".TestEmpty"].count.pass)
+	require.Equal(t, 1, pkg.tests[pkgname+".TestParse"].count.fail)
+	require.Equal(t, 1, pkg.tests[pkgname+".TestParseHostPort"].count.skip)
 }
 
 func TestSuccessOutput(t *testing.T) {
@@ -104,26 +104,12 @@ func TestFailureOutput(t *testing.T) {
 		"--- FAIL: TestParse (0.00s)\n",
 	}
 	expectedPkgOutput := []string{
-		"=== RUN   TestParseHostPort\n",
-		"=== PAUSE TestParseHostPort\n",
-		"=== RUN   TestEmpty\n",
-		"=== PAUSE TestEmpty\n",
-		"=== RUN   TestParse\n",
-		"=== PAUSE TestParse\n",
-		"=== CONT  TestParseHostPort\n",
-		"    addr_test.go:32: \n",
-		"=== CONT  TestParse\n",
-		"--- SKIP: TestParseHostPort (0.00s)\n",
-		"=== CONT  TestEmpty\n",
-		"    addr_test.go:71: failed\n",
-		"--- PASS: TestEmpty (0.00s)\n",
-		"--- FAIL: TestParse (0.00s)\n",
 		"FAIL\n",
 		"\texample.com/package\tcoverage: 2.4% of statements\n",
 		"FAIL\texample.com/package\t0.007s\n",
 	}
-	require.Equal(t, pkg.tests[pkgname+".TestParse"].output, expectedTestOutput)
-	require.Equal(t, pkg.output, expectedPkgOutput)
+	require.Equal(t, expectedTestOutput, pkg.tests[pkgname+".TestParse"].output)
+	require.Equal(t, expectedPkgOutput, pkg.output)
 }
 
 func TestPrintTestResultByPackage(t *testing.T) {
@@ -200,6 +186,12 @@ func TestPrintFailedTestOutput(t *testing.T) {
 	rr.printFailedTestOutput(output)
 
 	expected := `
+OUTPUT example.com/package
+===================================================
+FAIL
+	example.com/package	coverage: 2.4% of statements
+FAIL	example.com/package	0.007s
+===================================================
 OUTPUT example.com/package.TestParse
 ===================================================
 === RUN   TestParse
@@ -222,15 +214,12 @@ func TestPrintFlakinessSummaryNoFail(t *testing.T) {
 	output := &bytes.Buffer{}
 	rr.printFlakinessSummary(output)
 
-	expected := `
-===================================================
-No flaky tests!
-`[1:]
+	expected := "No flaky tests!\n"
 	require.Equal(t, expected, output.String())
 }
 
 func TestPrintFlakinessSummaryFail(t *testing.T) {
-	rr := newRunResult(byFlakiness, 2) // top 2 failures only
+	rr := newRunResult(byFlakiness, 3) // top 3 failures only (including packages)
 	feedEvents(t, rr, flakyPass)
 	feedEvents(t, rr, flakyFail1)
 	feedEvents(t, rr, flakyPass)
@@ -246,24 +235,9 @@ func TestPrintFlakinessSummaryFail(t *testing.T) {
 	rr.printFlakinessSummary(output)
 
 	expected := `
-===================================================
-FAIL(30.0%): example.com/package3.Test5
-FAIL(20.0%): example.com/package1.Test1
-===================================================
-OUTPUT example.com/package3.Test5
-===================================================
-=== RUN   Test5
-    baz_test.go:10: nevermind
---- FAIL: Test5 (0.00s)
-===================================================
-OUTPUT example.com/package1.Test1
-===================================================
-=== RUN   Test1
-    foo_test.go:6: doing stuff
-x =  1
-    foo_test.go:8: fail
---- FAIL: Test1 (0.00s)
-===================================================
+FAIL(4/10): example.com/package3
+FAIL(3/10): example.com/package3.Test5
+FAIL(2/10): example.com/package1.Test1
 `[1:]
 	require.Equal(t, expected, output.String())
 }