diff --git a/tools/gotestsummary/main.go b/tools/gotestsummary/main.go index 33c83f29b..21d44702e 100644 --- a/tools/gotestsummary/main.go +++ b/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 - tests []testResult - packages []packageResult + // failedBuilds holds the ImportPaths whose build failed and has not been + // reported through a package fail event yet. + 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, - output: make(map[testKey][]string), - dropped: make(map[testKey]int), - pkgOutput: make(map[string][]string), - stores: make(map[testKey]storeStats), - panics: make(map[string][]string), + out: out, + output: make(map[testKey][]string), + dropped: make(map[testKey]int), + pkgOutput: make(map[string][]string), + failedBuilds: make(map[string]bool), + 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": - // Test output arrives one line per event; build output may carry several. + case "output": + 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. -func (s *summarizer) printPackageOutput(pkg string) { +// or the compiler errors of a failed build, under the given header. +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 }) diff --git a/tools/gotestsummary/main_test.go b/tools/gotestsummary/main_test.go index e011f2016..6ba4a9fa8 100644 --- a/tools/gotestsummary/main_test.go +++ b/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", - "==== output of a outside tests ====", + "==== build output of a ====", " 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"} {