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.

Fixes golang#23565

Co-authored-by: James Roberts <[email protected]>

src/cmd/go/go_test.go

@@ -6,6 +6,19 @@ package main_test
 
 import (
 	"bytes"
+	cmdgo "cmd/go"
+	"cmd/go/internal/base"
+	"cmd/go/internal/cache"
+	"cmd/go/internal/cfg"
+	"cmd/go/internal/gover"
+	"cmd/go/internal/robustio"
+	"cmd/go/internal/search"
+	"cmd/go/internal/toolchain"
+	"cmd/go/internal/vcs"
+	"cmd/go/internal/vcweb/vcstest"
+	"cmd/go/internal/web"
+	"cmd/go/internal/work"
+	"cmd/internal/sys"
 	"debug/elf"
 	"debug/macho"
 	"debug/pe"
@@ -29,21 +42,6 @@ import (
 	"strings"
 	"testing"
 	"time"
-
-	"cmd/go/internal/base"
-	"cmd/go/internal/cache"
-	"cmd/go/internal/cfg"
-	"cmd/go/internal/gover"
-	"cmd/go/internal/robustio"
-	"cmd/go/internal/search"
-	"cmd/go/internal/toolchain"
-	"cmd/go/internal/vcs"
-	"cmd/go/internal/vcweb/vcstest"
-	"cmd/go/internal/web"
-	"cmd/go/internal/work"
-	"cmd/internal/sys"
-
-	cmdgo "cmd/go"
 )
 
 func init() {
@@ -2801,3 +2799,71 @@ func TestExecInDeletedDir(t *testing.T) {
 	// `go version` should not fail
 	tg.run("version")
 }
+
+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")
+}

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,19 @@ 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) error {
 	if coverMerge.f == nil {
-		return
+		return nil
 	}
 	coverMerge.Lock()
 	defer coverMerge.Unlock()
@@ -57,28 +59,41 @@ 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)
 	}