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.

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)
 	}