cmd/go: cache coverage profile with tests

src/cmd/go/internal/test/cover.go

@@ -14,6 +14,9 @@ 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
@@ -44,8 +47,8 @@ func initCoverProfile() {
 }
 
 // mergeCoverProfile merges file into the profile stored in testCoverProfile.
-// It prints any errors it encounters to ew.
-func mergeCoverProfile(ew io.Writer, file string) {
+// Errors encountered are logged and cause a non-zero exit status.
+func mergeCoverProfile(file string) {
 	if coverMerge.f == nil {
 		return
 	}
@@ -66,12 +69,13 @@ 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("test wrote malformed coverage profile %s: header %q, expected %q: %v", file, string(buf), expect, err)
 		return
 	}
 	_, err = io.Copy(coverMerge.f, r)
 	if err != nil {
-		fmt.Fprintf(ew, "error: saving coverage profile: %v\n", err)
+		base.Errorf("saving coverage profile: %v", err)
+		return
 	}
 }
 

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,7 +1639,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, coverProfTempFile(a))
 	} else {
 		if testFailFast {
 			testShouldFailFast.Store(true)
@@ -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: cached test result valid but cached coverage profile missing: %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,19 +2049,38 @@ 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 {
+					base.Errorf("closing temporary coverprofile: %v", err)
+				}
+			}()
+		} else if cache.DebugTest {
+			// Not indicative of a problem, as the test may not have generated a cover profile yet.
+			fmt.Fprintf(os.Stderr, "testcache: %s: opening temporary coverprofile: %s\n", 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 {
 			fmt.Fprintf(os.Stderr, "testcache: %s: save test ID %x => input ID %x => %x\n", a.Package.ImportPath, c.id2, testInputsID, testAndInputKey(c.id2, testInputsID))
 		}
 		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)
 	}
 }
 

src/cmd/go/testdata/script/test_cache_inputs.txt

@@ -128,6 +128,45 @@ go test testcache -run=TestOSArgs -fullpath
 go test testcache -run=TestOSArgs -fullpath
 stdout '\(cached\)'
 
+# Ensure that coverage profiles are being cached.
+go test testcache -run=TestCoverageCache -coverprofile=coverage.out
+go test testcache -run=TestCoverageCache -coverprofile=coverage.out
+stdout '\(cached\)'
+exists coverage.out
+grep -q 'mode: set' coverage.out
+grep -q 'testcache/hello.go:' coverage.out
+
+# 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=coverage_set.out -covermode=set
+stdout '\(cached\)'
+cmp coverage.out coverage_set.out
+
+# A new -covermode should not use the cached coverage profile.
+go test testcache -run=TestCoverageCache -coverprofile=coverage_atomic.out -covermode=atomic
+! stdout '\(cached\)'
+! cmp coverage.out coverage_atomic.out
+grep -q 'mode: atomic' coverage_atomic.out
+grep -q 'testcache/hello.go:' coverage_atomic.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 +198,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 +313,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