diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 45de1cc869f1bc..26eecd6616c877 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -1800,8 +1800,9 @@ // // The rule for a match in the cache is that the run involves the same // test binary and the flags on the command line come entirely from a -// restricted set of 'cacheable' test flags, defined as -benchtime, -cpu, -// -list, -parallel, -run, -short, -timeout, -failfast, and -v. +// restricted set of 'cacheable' test flags, defined as -benchtime, -cover, +// -covermode, -coverprofile, -cpu, -list, -parallel, -run, -short, -timeout, +// -failfast, -v, -vet and -outputdir. // If a run of go test has any test or non-test flags outside this set, // the result is not cached. To disable test caching, use any test flag // or argument other than the cacheable flags. The idiomatic way to disable diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 54249f6f7a47f5..fa3d475d594597 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -2447,6 +2447,68 @@ func TestCacheCoverage(t *testing.T) { tg.run("test", "-cover", "-short", "math", "strings") } +func TestCacheCoverageProfile(t *testing.T) { + tooSlow(t, "links and runs a test binary multiple times with coverage enabled") + + if gocacheverify.Value() == "1" { + t.Skip("GODEBUG gocacheverify") + } + + tg := testgo(t) + defer tg.cleanup() + tg.parallel() + tg.setenv("GOPATH", filepath.Join(tg.pwd(), "testdata")) + tg.makeTempdir() + tg.setenv("GOCACHE", tg.path("c1")) + + // checkProfile asserts that the given profile contains the given mode + // and coverage lines for all given files. + checkProfile := func(t *testing.T, profile, mode string, files ...string) { + t.Helper() + if out, err := os.ReadFile(profile); err != nil { + t.Fatalf("failed to open coverprofile: %v", err) + } else { + if n := bytes.Count(out, []byte("mode: "+mode)); n != 1 { + if n == 0 { + t.Fatalf("missing mode: %s", mode) + } else { + t.Fatalf("too many mode: %s", mode) + } + } + for _, fname := range files { + if !bytes.Contains(out, []byte(fname)) { + t.Fatalf("missing file in coverprofile: %s", fname) + } + } + } + } + + tg.run("test", "-coverprofile="+tg.path("cover.out"), "-x", "-v", "-short", "strings") + tg.grepStdout(`ok \t`, "expected strings test to succeed") + checkProfile(t, tg.path("cover.out"), "set", "strings/strings.go") + // Repeat commands should use the cache. + tg.run("test", "-coverprofile="+tg.path("cover.out"), "-x", "-v", "-short", "strings") + tg.grepStdout(`ok \tstrings\t\(cached\)`, "expected strings test results to be cached") + checkProfile(t, tg.path("cover.out"), "set", "strings/strings.go") + // Cover profiles should be cached independently. Since strings is already cached, + // only math should need to run. + tg.run("test", "-coverprofile="+tg.path("cover.out"), "-x", "-v", "-short", "strings", "math") + tg.grepStdout(`ok \tstrings\t\(cached\)`, "expected strings test results to be cached") + checkProfile(t, tg.path("cover.out"), "set", "strings/strings.go", "math/mod.go") + // A new -coverprofile file should use the cached coverage profile contents. + tg.run("test", "-coverprofile="+tg.path("cover1.out"), "-x", "-v", "-short", "strings") + tg.grepStdout(`ok \tstrings\t\(cached\)`, "expected cached strings test results to be used regardless of -coverprofile") + checkProfile(t, tg.path("cover1.out"), "set", "strings/strings.go") + // A new -covermode should not use the cached coverage profile, since the covermode changes + // the profile output. + tg.run("test", "-covermode=count", "-coverprofile="+tg.path("cover.out"), "-x", "-v", "-short", "strings") + tg.grepStdoutNot(`ok \tstrings\t\(cached\)`, "cached strings test results should not be used with different -covermode") + // A new -coverpkg should not use the cached coverage profile, since the coverpkg changes + // the profile output. + tg.run("test", "-coverpkg=math", "-coverprofile="+tg.path("cover.out"), "-x", "-v", "-short", "strings") + tg.grepStdoutNot(`ok \tstrings\t\(cached\)`, "cached strings test results should not be used with different -coverpkg") +} + func TestIssue22588(t *testing.T) { // Don't get confused by stderr coming from tools. tg := testgo(t) diff --git a/src/cmd/go/internal/test/cover.go b/src/cmd/go/internal/test/cover.go index f614458dc46a15..add65056c5fa72 100644 --- a/src/cmd/go/internal/test/cover.go +++ b/src/cmd/go/internal/test/cover.go @@ -16,7 +16,8 @@ import ( var coverMerge struct { f *os.File - sync.Mutex // for f.Write + fsize int64 // size of valid data written to f + sync.Mutex // for f.Write } // initCoverProfile initializes the test coverage profile. @@ -36,18 +37,18 @@ func initCoverProfile() { if err != nil { base.Fatalf("%v", err) } - _, err = fmt.Fprintf(f, "mode: %s\n", cfg.BuildCoverMode) + n, err := fmt.Fprintf(f, "mode: %s\n", cfg.BuildCoverMode) if err != nil { base.Fatalf("%v", err) } coverMerge.f = f + coverMerge.fsize += int64(n) } // mergeCoverProfile merges file into the profile stored in testCoverProfile. -// It prints any errors it encounters to ew. -func mergeCoverProfile(ew io.Writer, file string) { +func mergeCoverProfile(file string) error { if coverMerge.f == nil { - return + return nil } coverMerge.Lock() defer coverMerge.Unlock() @@ -57,28 +58,37 @@ func mergeCoverProfile(ew io.Writer, file string) { r, err := os.Open(file) if err != nil { // Test did not create profile, which is OK. - return + return nil } defer r.Close() n, err := io.ReadFull(r, buf) if n == 0 { - return + return nil } if err != nil || string(buf) != expect { - fmt.Fprintf(ew, "error: test wrote malformed coverage profile %s.\n", file) - return + return fmt.Errorf("test wrote malformed coverage profile %s", file) } - _, err = io.Copy(coverMerge.f, r) + m, err := io.Copy(coverMerge.f, r) if err != nil { - fmt.Fprintf(ew, "error: saving coverage profile: %v\n", err) + if m > 0 { + // Attempt to rollback partial write. + coverMerge.f.Seek(coverMerge.fsize, 0) + } + return fmt.Errorf("saving coverage profile: %w", err) } + coverMerge.fsize += m + return nil } func closeCoverProfile() { if coverMerge.f == nil { return } + // Discard any partially written data from a failed merge. + if err := coverMerge.f.Truncate(coverMerge.fsize); err != nil { + base.Errorf("closing coverage profile: %v", err) + } if err := coverMerge.f.Close(); err != nil { base.Errorf("closing coverage profile: %v", err) } diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go index 7df6f421d6df46..6090f2f0891a23 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -124,8 +124,9 @@ elapsed time in the summary line. The rule for a match in the cache is that the run involves the same test binary and the flags on the command line come entirely from a -restricted set of 'cacheable' test flags, defined as -benchtime, -cpu, --list, -parallel, -run, -short, -timeout, -failfast, and -v. +restricted set of 'cacheable' test flags, defined as -benchtime, -cover, +-covermode, -coverprofile, -cpu, -list, -parallel, -run, -short, -timeout, +-failfast, -v, -vet and -outputdir. If a run of go test has any test or non-test flags outside this set, the result is not cached. To disable test caching, use any test flag or argument other than the cacheable flags. The idiomatic way to disable @@ -1342,11 +1343,13 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action) } args := str.StringList(execCmd, a.Deps[0].BuiltTarget(), testlogArg, panicArg, fuzzArg, coverdirArg, testArgs) + var tempCoverProfile string if testCoverProfile != "" { // Write coverage to temporary profile, for merging later. + tempCoverProfile = a.Objdir + "_cover_.out" for i, arg := range args { if strings.HasPrefix(arg, "-test.coverprofile=") { - args[i] = "-test.coverprofile=" + a.Objdir + "_cover_.out" + args[i] = "-test.coverprofile=" + tempCoverProfile } } } @@ -1426,7 +1429,10 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action) a.TestOutput = &buf t := fmt.Sprintf("%.3fs", time.Since(t0).Seconds()) - mergeCoverProfile(cmd.Stdout, a.Objdir+"_cover_.out") + if coverErr := mergeCoverProfile(tempCoverProfile); coverErr != nil { + // TODO: consider whether an error to merge cover profiles should fail the test. + fmt.Fprintf(cmd.Stdout, "error: %v\n", coverErr) + } if err == nil { norun := "" @@ -1448,7 +1454,7 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action) cmd.Stdout.Write([]byte("\n")) } fmt.Fprintf(cmd.Stdout, "ok \t%s\t%s%s%s\n", a.Package.ImportPath, t, coveragePercentage(out), norun) - r.c.saveOutput(a) + r.c.saveOutput(a, tempCoverProfile) } else { base.SetExitStatus(1) if cancelSignaled { @@ -1541,7 +1547,14 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo // Note that this list is documented above, // so if you add to this list, update the docs too. cacheArgs = append(cacheArgs, arg) - + case "-test.coverprofile", + "-test.outputdir": + // The -coverprofile and -outputdir arguments are cacheable but + // only change where profiles are written. They don't change the + // profile contents, so they aren't added to the cacheArgs. This + // allows cached coverage profiles to be written to different files. + // Note that this list is documented above, + // so if you add to this list, update the docs too. default: // nothing else is cacheable if cache.DebugTest { @@ -1653,6 +1666,28 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo j++ } c.buf.Write(data[j:]) + + // Write coverage data to profile. + if cfg.BuildCover { + // The cached coverprofile has the same expiration time as the + // test result it corresponds to. That time is already checked + // above, so we can ignore the entry returned by GetFile here. + f, _, err := cache.GetFile(cache.Default(), testCoverProfileKey(testID, testInputsID)) + if err != nil { + if cache.DebugTest { + fmt.Fprintf(os.Stderr, "testcache: %s: test coverage profile not found: %v\n", a.Package.ImportPath, err) + } + return false + } + if err := mergeCoverProfile(f); err != nil { + // TODO: consider whether an error to merge cover profiles should fail the test. + if cache.DebugTest { + fmt.Fprintf(os.Stderr, "testcache: %s: test coverage profile not merged: %v\n", a.Package.ImportPath, err) + } + return false + } + } + return true } @@ -1799,7 +1834,12 @@ func testAndInputKey(testID, testInputsID cache.ActionID) cache.ActionID { return cache.Subkey(testID, fmt.Sprintf("inputs:%x", testInputsID)) } -func (c *runCache) saveOutput(a *work.Action) { +// testCoverProfileKey returns the "coverprofile" cache key for the pair (testID, testInputsID). +func testCoverProfileKey(testID, testInputsID cache.ActionID) cache.ActionID { + return cache.Subkey(testAndInputKey(testID, testInputsID), "coverprofile") +} + +func (c *runCache) saveOutput(a *work.Action, coverprofileFile string) { if c.id1 == (cache.ActionID{}) && c.id2 == (cache.ActionID{}) { return } @@ -1820,12 +1860,29 @@ func (c *runCache) saveOutput(a *work.Action) { if err != nil { return } + saveCoverProfile := func(testID cache.ActionID) {} + if coverprofileFile != "" { + coverprof, err := os.Open(coverprofileFile) + if err == nil { + saveCoverProfile = func(testID cache.ActionID) { + cache.Default().Put(testCoverProfileKey(testID, testInputsID), coverprof) + } + defer func() { + if err := coverprof.Close(); err != nil && cache.DebugTest { + fmt.Fprintf(os.Stderr, "testcache: %s: closing temporary coverprofile: %v", a.Package.ImportPath, err) + } + }() + } else if cache.DebugTest { + fmt.Fprintf(os.Stderr, "testcache: %s: failed to open temporary coverprofile: %s", a.Package.ImportPath, err) + } + } if c.id1 != (cache.ActionID{}) { if cache.DebugTest { fmt.Fprintf(os.Stderr, "testcache: %s: save test ID %x => input ID %x => %x\n", a.Package.ImportPath, c.id1, testInputsID, testAndInputKey(c.id1, testInputsID)) } cache.PutNoVerify(cache.Default(), c.id1, bytes.NewReader(testlog)) cache.PutNoVerify(cache.Default(), testAndInputKey(c.id1, testInputsID), bytes.NewReader(a.TestOutput.Bytes())) + saveCoverProfile(c.id1) } if c.id2 != (cache.ActionID{}) { if cache.DebugTest { @@ -1833,6 +1890,7 @@ func (c *runCache) saveOutput(a *work.Action) { } cache.PutNoVerify(cache.Default(), c.id2, bytes.NewReader(testlog)) cache.PutNoVerify(cache.Default(), testAndInputKey(c.id2, testInputsID), bytes.NewReader(a.TestOutput.Bytes())) + saveCoverProfile(c.id2) } }