diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go index 7993420a8fa238..2d68fc8933bec8 100644 --- a/src/cmd/go/alldocs.go +++ b/src/cmd/go/alldocs.go @@ -1885,8 +1885,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, -fullpath and -v. +// restricted set of 'cacheable' test flags, defined as -benchtime, -cover, +// -covermode, -coverprofile, -cpu, -list, -parallel, -run, -short, -timeout, +// -failfast, -fullpath and -v. // 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/internal/test/cover.go b/src/cmd/go/internal/test/cover.go index f614458dc46a15..c5444cc9b49147 100644 --- a/src/cmd/go/internal/test/cover.go +++ b/src/cmd/go/internal/test/cover.go @@ -14,9 +14,13 @@ import ( "sync" ) +// coverMerge manages the state for merging test coverage profiles. +// It ensures thread-safe operations on a single coverage profile file +// across multiple test runs and packages. var coverMerge struct { f *os.File - sync.Mutex // for f.Write + fsize int64 // Tracks the size of valid data written to f + sync.Mutex // for f.Write } // initCoverProfile initializes the test coverage profile. @@ -36,16 +40,17 @@ func initCoverProfile() { if err != nil { base.Fatalf("%v", err) } - _, err = fmt.Fprintf(f, "mode: %s\n", cfg.BuildCoverMode) + s, err := fmt.Fprintf(f, "mode: %s\n", cfg.BuildCoverMode) if err != nil { base.Fatalf("%v", err) } coverMerge.f = f + coverMerge.fsize = int64(s) } // 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) { if coverMerge.f == nil { return } @@ -66,19 +71,25 @@ func mergeCoverProfile(ew io.Writer, file string) { return } if err != nil || string(buf) != expect { - fmt.Fprintf(ew, "error: test wrote malformed coverage profile %s.\n", file) + base.Errorf("error: test wrote malformed coverage profile %s: header %q, expected %q: %v", file, string(buf), expect, err) return } - _, err = io.Copy(coverMerge.f, r) + s, err := io.Copy(coverMerge.f, r) if err != nil { - fmt.Fprintf(ew, "error: saving coverage profile: %v\n", err) + base.Errorf("error: saving coverage profile: %v", err) + return } + coverMerge.fsize += s } 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 7d20e28adef9b1..42f47fd4068174 100644 --- a/src/cmd/go/internal/test/test.go +++ b/src/cmd/go/internal/test/test.go @@ -126,8 +126,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, -fullpath and -v. +restricted set of 'cacheable' test flags, defined as -benchtime, -cover, +-covermode, -coverprofile, -cpu, -list, -parallel, -run, -short, -timeout, +-failfast, -fullpath and -v. 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 @@ -1338,6 +1339,13 @@ type runCache struct { id2 cache.ActionID } +func coverProfTempFile(a *work.Action) string { + if a.Objdir == "" { + panic("internal error: objdir not set in coverProfTempFile") + } + return a.Objdir + "_cover_.out" +} + // stdoutMu and lockedStdout provide a locked standard output // that guarantees never to interlace writes from multiple // goroutines, so that we can have multiple JSON streams writing @@ -1396,13 +1404,6 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action) return nil } - coverProfTempFile := func(a *work.Action) string { - if a.Objdir == "" { - panic("internal error: objdir not set in coverProfTempFile") - } - return a.Objdir + "_cover_.out" - } - if p := a.Package; len(p.TestGoFiles)+len(p.XTestGoFiles) == 0 { reportNoTestFiles := true if cfg.BuildCover && cfg.Experiment.CoverageRedesign && p.Internal.Cover.GenMeta { @@ -1426,7 +1427,7 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action) if err := work.WriteCoverageProfile(b, a, mf, cp, stdout); err != nil { return err } - mergeCoverProfile(stdout, cp) + mergeCoverProfile(cp) } } } @@ -1616,7 +1617,7 @@ 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") + mergeCoverProfile(a.Objdir + "_cover_.out") if err == nil { norun := "" @@ -1638,10 +1639,10 @@ 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, coverProfTempFile(a)) } else { if testFailFast { - testShouldFailFast.Store(true) + r.c.saveOutput(a, coverProfTempFile(a)) } base.SetExitStatus(1) @@ -1736,7 +1737,18 @@ 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, which + // only affect the location of profile output, are cacheable. This + // is based on the process where, upon a cache hit, stored profile + // data is copied to the specified output location. This mechanism + // ensures that output location preferences are honored without + // modifying the profile's content, thereby justifying their + // cacheability without impacting cache integrity. 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 { @@ -1848,6 +1860,21 @@ 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(), coverProfileAndInputKey(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 + } + mergeCoverProfile(f) + } return true } @@ -1996,7 +2023,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) { +// coverProfileAndInputKey returns the "coverprofile" cache key for the pair (testID, testInputsID). +func coverProfileAndInputKey(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 } @@ -2017,12 +2049,29 @@ func (c *runCache) saveOutput(a *work.Action) { if err != nil { return } + var saveCoverProfile = func(testID cache.ActionID) {} + if coverProfileFile != "" { + coverProfile, err := os.Open(coverProfileFile) + if err == nil { + saveCoverProfile = func(testID cache.ActionID) { + cache.Default().Put(coverProfileAndInputKey(testID, testInputsID), coverProfile) + } + defer func() { + if err := coverProfile.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 { @@ -2030,6 +2079,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) } } diff --git a/src/cmd/go/testdata/script/test_cache_inputs.txt b/src/cmd/go/testdata/script/test_cache_inputs.txt index 68a700b1160763..53053a39a3a62d 100644 --- a/src/cmd/go/testdata/script/test_cache_inputs.txt +++ b/src/cmd/go/testdata/script/test_cache_inputs.txt @@ -128,6 +128,40 @@ go test testcache -run=TestOSArgs -fullpath go test testcache -run=TestOSArgs -fullpath stdout '\(cached\)' +# Ensure that specifying a cover profile does not prevent test results from being cached. +go test testcache -run=TestCoverageCache -coverprofile=coverage.out +go test testcache -run=TestCoverageCache -coverprofile=coverage.out +stdout '\(cached\)' + +# A new -coverprofile file should use the cached coverage profile contents. +go test testcache -run=TestCoverageCache -coverprofile=coverage2.out +stdout '\(cached\)' +cmp coverage.out coverage2.out + +# Explicitly setting the default covermode should still use cache +go test testcache -run=TestCoverageCache -coverprofile=coverage3.out -covermode=set +stdout '\(cached\)' +cmp coverage.out coverage3.out + +# A new -covermode should not use the cached coverage profile. +go test testcache -run=TestCoverageCache -coverprofile=coverage_set.out -covermode=atomic +! stdout '\(cached\)' +! cmp coverage.out coverage_set.out + +# A new -coverpkg should not use the cached coverage profile. +go test testcache -run=TestCoverageCache -coverprofile=coverage_pkg.out -coverpkg=all +! stdout '\(cached\)' +! cmp coverage.out coverage_pkg.out + +# Test that -v doesn't prevent caching +go test testcache -v -run=TestCoverageCache -coverprofile=coverage_v.out +go test testcache -v -run=TestCoverageCache -coverprofile=coverage_v2.out +stdout '\(cached\)' +cmp coverage_v.out coverage_v2.out + +# Test that -count affects caching +go test testcache -run=TestCoverageCache -coverprofile=coverage_count.out -count=2 +! stdout '\(cached\)' # Executables within GOROOT and GOPATH should affect caching, # even if the test does not stat them explicitly. @@ -159,6 +193,18 @@ This file is outside of GOPATH. -- testcache/script.sh -- #!/bin/sh exit 0 +-- testcache/hello.go -- +package testcache + +import "fmt" + +func HelloWorld(name string) string { + if name == "" { + return "Hello, World!" + } + return fmt.Sprintf("Hello, %s!", name) +} + -- testcache/testcache_test.go -- // Copyright 2017 The Go Authors. All rights reserved. // Use of this source code is governed by a BSD-style @@ -262,6 +308,18 @@ func TestOSArgs(t *testing.T) { func TestBenchtime(t *testing.T) { } +func TestCoverageCache(t *testing.T) { + result := HelloWorld("") + if result != "Hello, World!" { + t.Errorf("Expected 'Hello, World!', got '%s'", result) + } + + result = HelloWorld("Go") + if result != "Hello, Go!" { + t.Errorf("Expected 'Hello, Go!', got '%s'", result) + } +} + -- mkold.go -- package main