[misc] Report a failed build once through the package fail event

go test emits a build-fail event for the test binary and then a fail event
for the package with FailedBuild, so the summary printed two FAIL lines and
two output blocks per build failure. The package fail event now reports the
compiler output; a build-fail never followed by one is still printed at the
end. Build output also skips the panic detection, so a compiler crash is
shown as compiler output instead of a test panic.

tools/gotestsummary/main.go

@@ -52,6 +52,9 @@ type event struct {
 	// " [pkg.test]" suffix naming the test binary the package was compiled
 	// for, and the same package can be built for several binaries at once.
 	ImportPath string `json:"ImportPath"`
+	// FailedBuild names the ImportPath whose build failure made the package
+	// fail; go test reports the package fail event after the build-fail one.
+	FailedBuild string `json:"FailedBuild"`
 }
 
 type testKey struct {
@@ -80,9 +83,12 @@ type summarizer struct {
 	// pkgOutput keeps what a package printed outside any test, which is where
 	// compiler diagnostics of a failed build end up.
 	pkgOutput map[string][]string
-	stores    map[testKey]storeStats
+	// failedBuilds holds the ImportPaths whose build failed and has not been
-	tests     []testResult
+	// reported through a package fail event yet.
-	packages  []packageResult
+	failedBuilds map[string]bool
+	stores       map[testKey]storeStats
+	tests        []testResult
+	packages     []packageResult
 	// panics holds the head of a panic per package. Package streams interleave
 	// in a go test -json run, so one package's dump must not swallow another's
 	// output.
@@ -96,12 +102,13 @@ type storeStats struct {
 
 func newSummarizer(out io.Writer) *summarizer {
 	return &summarizer{
-		out:       out,
+		out:          out,
-		output:    make(map[testKey][]string),
+		output:       make(map[testKey][]string),
-		dropped:   make(map[testKey]int),
+		dropped:      make(map[testKey]int),
-		pkgOutput: make(map[string][]string),
+		pkgOutput:    make(map[string][]string),
-		stores:    make(map[testKey]storeStats),
+		failedBuilds: make(map[string]bool),
-		panics:    make(map[string][]string),
+		stores:       make(map[testKey]storeStats),
+		panics:       make(map[string][]string),
 	}
 }
 
@@ -166,13 +173,19 @@ func (s *summarizer) handle(ev event) {
 				s.output[key] = []string{}
 			}
 		}
-	case "output", "build-output":
+	case "output":
-		// Test output arrives one line per event; build output may carry several.
+		s.handleOutput(key, strings.TrimRight(ev.Output, "\n"))
+	case "build-output":
+		// Compiler output may carry several lines per event and is never test
+		// output, so it skips the panic and store-marker detection.
 		for _, line := range strings.Split(strings.TrimRight(ev.Output, "\n"), "\n") {
-			s.handleOutput(key, line)
+			s.pkgOutput[key.pkg] = appendBounded(s.pkgOutput[key.pkg], line)
 		}
 	case "build-fail":
-		s.handlePackageResult(ev)
+		// The package fail event that follows carries FailedBuild and reports
+		// the compiler output; this only remembers the build in case it never
+		// comes.
+		s.failedBuilds[key.pkg] = true
 	case "pass", "fail", "skip":
 		if ev.Test == "" {
 			s.handlePackageResult(ev)
@@ -278,7 +291,13 @@ func (s *summarizer) handlePackageResult(ev event) {
 	fmt.Fprintf(s.out, "%s %s %s\n", label, shortPkg(ev.Package), elapsed.Round(time.Millisecond))
 
 	if label == "FAIL" {
-		s.printPackageOutput(ev.Package)
+		if ev.FailedBuild != "" {
+			// Several test binaries can share one failed dependency, so its
+			// output stays available for the next package that names it.
+			s.printPackageOutput(ev.FailedBuild, "build output of %s")
+			delete(s.failedBuilds, ev.FailedBuild)
+		}
+		s.printPackageOutput(ev.Package, "output of %s outside tests")
 		s.printUnfinished(ev.Package)
 		s.printPanicHead(ev.Package)
 	}
@@ -286,9 +305,23 @@ func (s *summarizer) handlePackageResult(ev event) {
 	delete(s.panics, ev.Package)
 }
 
+// printUnclaimedBuildFailures reports the failed builds no package fail event
+// accounted for, so a compiler error never disappears from the log.
+func (s *summarizer) printUnclaimedBuildFailures() {
+	var builds []string
+	for b := range s.failedBuilds {
+		builds = append(builds, b)
+	}
+	sort.Strings(builds)
+	for _, b := range builds {
+		fmt.Fprintf(s.out, "FAIL %s [build failed]\n", shortPkg(b))
+		s.printPackageOutput(b, "build output of %s")
+	}
+}
+
 // printPackageOutput shows what a failed package printed outside its tests,
-// such as the compiler errors of a build failure.
+// or the compiler errors of a failed build, under the given header.
-func (s *summarizer) printPackageOutput(pkg string) {
+func (s *summarizer) printPackageOutput(pkg, header string) {
 	lines := s.pkgOutput[pkg]
 	if len(lines) == 0 {
 		return
@@ -296,7 +329,7 @@ func (s *summarizer) printPackageOutput(pkg string) {
 	if len(lines) > failedTestOutputLines {
 		lines = lines[len(lines)-failedTestOutputLines:]
 	}
-	fmt.Fprintf(s.out, "\n==== output of %s outside tests ====\n", shortPkg(pkg))
+	fmt.Fprintf(s.out, "\n==== "+header+" ====\n", shortPkg(pkg))
 	for _, l := range lines {
 		fmt.Fprintf(s.out, "    %s\n", l)
 	}
@@ -338,6 +371,8 @@ func (s *summarizer) printUnfinished(pkg string) {
 }
 
 func (s *summarizer) printSummary(slowest int) {
+	s.printUnclaimedBuildFailures()
+
 	fmt.Fprintln(s.out)
 	fmt.Fprintln(s.out, "==== package durations ====")
 	sort.Slice(s.packages, func(i, j int) bool { return s.packages[i].elapsed > s.packages[j].elapsed })

tools/gotestsummary/main_test.go

@@ -83,20 +83,81 @@ func TestStoreSetupTimeIsAttributedToTheTest(t *testing.T) {
 }
 
 func TestBuildFailureShowsCompilerOutput(t *testing.T) {
+	// The event sequence go test emits for a build failure: the build events
+	// name the test binary, then the package itself fails with FailedBuild.
 	events := `
 {"Action":"build-output","ImportPath":"a [a.test]","Output":"# a [a.test]\na_test.go:7:2: undefined: nope\na_test.go:9:2: undefined: nope2\n"}
 {"Action":"build-fail","ImportPath":"a [a.test]"}
+{"Action":"start","Package":"a"}
+{"Action":"output","Package":"a","Output":"FAIL\ta [build failed]\n"}
+{"Action":"fail","Package":"a","Elapsed":0,"FailedBuild":"a [a.test]"}
 `
 	got := feed(t, events)
 	for _, want := range []string{
-		"FAIL a 0s",
+		"==== build output of a ====",
-		"==== output of a outside tests ====",
 		"    a_test.go:7:2: undefined: nope\n    a_test.go:9:2: undefined: nope2",
+		"FAIL\ta [build failed]",
 	} {
 		if !strings.Contains(got, want) {
 			t.Errorf("output lacks %q:\n%s", want, got)
 		}
 	}
+	if n := strings.Count(got, "FAIL a 0s"); n != 1 {
+		t.Errorf("expected one FAIL line for the package, got %d:\n%s", n, got)
+	}
+	if n := strings.Count(got, "undefined: nope2"); n != 1 {
+		t.Errorf("expected the compiler output once, got %d:\n%s", n, got)
+	}
+}
+
+func TestFailedDependencyOutputIsShownForEveryImporter(t *testing.T) {
+	events := `
+{"Action":"build-output","ImportPath":"m/x","Output":"# m/x\nx.go:3:11: undefined: y\n"}
+{"Action":"build-fail","ImportPath":"m/x"}
+{"Action":"start","Package":"m/a"}
+{"Action":"output","Package":"m/a","Output":"FAIL\tm/a [build failed]\n"}
+{"Action":"fail","Package":"m/a","Elapsed":0,"FailedBuild":"m/x"}
+{"Action":"start","Package":"m/b"}
+{"Action":"output","Package":"m/b","Output":"FAIL\tm/b [build failed]\n"}
+{"Action":"fail","Package":"m/b","Elapsed":0,"FailedBuild":"m/x"}
+`
+	got := feed(t, events)
+	if n := strings.Count(got, "x.go:3:11: undefined: y"); n != 2 {
+		t.Errorf("expected the dependency's compiler output under both packages, got %d:\n%s", n, got)
+	}
+	if strings.Contains(got, "FAIL m/x") {
+		t.Errorf("the dependency must not be reported as a package of its own:\n%s", got)
+	}
+}
+
+func TestBuildFailureWithoutPackageEventIsStillReported(t *testing.T) {
+	events := `
+{"Action":"build-output","ImportPath":"a [a.test]","Output":"a_test.go:7:2: undefined: nope\n"}
+{"Action":"build-fail","ImportPath":"a [a.test]"}
+`
+	got := feed(t, events)
+	for _, want := range []string{"FAIL a [build failed]", "==== build output of a ====", "undefined: nope"} {
+		if !strings.Contains(got, want) {
+			t.Errorf("output lacks %q:\n%s", want, got)
+		}
+	}
+}
+
+func TestCompilerPanicIsBuildOutputNotTestPanic(t *testing.T) {
+	events := `
+{"Action":"build-output","ImportPath":"a [a.test]","Output":"# a [a.test]\npanic: internal compiler error\n\ngoroutine 1 [running]:\n"}
+{"Action":"build-fail","ImportPath":"a [a.test]"}
+{"Action":"start","Package":"a"}
+{"Action":"output","Package":"a","Output":"FAIL\ta [build failed]\n"}
+{"Action":"fail","Package":"a","Elapsed":0,"FailedBuild":"a [a.test]"}
+`
+	got := feed(t, events)
+	if !strings.Contains(got, "==== build output of a ====\n    # a [a.test]\n    panic: internal compiler error") {
+		t.Errorf("compiler diagnostic missing from the build output block:\n%s", got)
+	}
+	if strings.Contains(got, "==== panic in") {
+		t.Errorf("compiler output must not be reported as a test panic:\n%s", got)
+	}
 }
 
 func TestBuildVariantsOfOnePackageKeepSeparateOutput(t *testing.T) {
@@ -105,9 +166,13 @@ func TestBuildVariantsOfOnePackageKeepSeparateOutput(t *testing.T) {
 {"Action":"build-output","ImportPath":"a [b.test]","Output":"a.go:1:1: broken for b.test\n"}
 {"Action":"build-fail","ImportPath":"a [a.test]"}
 {"Action":"build-fail","ImportPath":"a [b.test]"}
+{"Action":"start","Package":"a"}
+{"Action":"fail","Package":"a","Elapsed":0,"FailedBuild":"a [a.test]"}
+{"Action":"start","Package":"b"}
+{"Action":"fail","Package":"b","Elapsed":0,"FailedBuild":"a [b.test]"}
 `
 	got := feed(t, events)
-	if strings.Count(got, "==== output of a outside tests ====") != 2 {
+	if strings.Count(got, "==== build output of a ====") != 2 {
 		t.Errorf("expected one output block per build variant:\n%s", got)
 	}
 	for _, want := range []string{"broken for a.test", "broken for b.test"} {