cmd/go: cache coverage profile with tests

This CL stores coverage profile data in the GOCACHE under the
'coverprofile' subkey alongside tests. This makes tests which use
coverage profiles cacheable. The values of the -coverprofile and
-outputdir flags are not included in the cache key to allow cached
profile data to be written to any output file.

Note: This is a rebase and squash from the original PRs below that
was created/closed/abandoned by @jproberts and @macnibblet that I
plan to maintain.

- golang#50483
- golang#65657

I made improvements to the change based on feedback from @bcmills in Gerrit
https://go-review.googlesource.com/c/go/+/563138.

From @macnibblet:

I don't know if anyone has considered the environmental impact
(Yes, of course, dev experience too), but on a team with 3 backend
developers, when I replaced our CI Golang version with this build,
it reduced the build time by 50%, which would have
equated to about 5000 hours of CI reduced in the past year.

Fixes golang#23565

Author: ryancurrah

src/cmd/go/alldocs.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)
 	}

src/cmd/go/internal/test/cover.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
@@ -1377,6 +1378,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
@@ -1478,13 +1486,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 {
@@ -1508,7 +1509,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)
 				}
 			}
 		}
@@ -1671,7 +1672,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 := ""
@@ -1693,7 +1694,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)
@@ -1791,7 +1792,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 {
@@ -1903,6 +1915,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
 }
 
@@ -2051,7 +2078,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
 	}
@@ -2072,19 +2104,37 @@ 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 {
 			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/internal/test/test.go

@@ -128,6 +128,45 @@ 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\)'
+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