Skip to content

Commit d5e4022

Browse files
committed
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
1 parent c8fb6ae commit d5e4022

File tree

4 files changed

+147
-22
lines changed

4 files changed

+147
-22
lines changed

src/cmd/go/alldocs.go

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,13 @@ import (
1414
"sync"
1515
)
1616

17+
// coverMerge manages the state for merging test coverage profiles.
18+
// It ensures thread-safe operations on a single coverage profile file
19+
// across multiple test runs and packages.
1720
var coverMerge struct {
1821
f *os.File
19-
sync.Mutex // for f.Write
22+
fsize int64 // Tracks the size of valid data written to f
23+
sync.Mutex // for f.Write
2024
}
2125

2226
// initCoverProfile initializes the test coverage profile.
@@ -36,16 +40,17 @@ func initCoverProfile() {
3640
if err != nil {
3741
base.Fatalf("%v", err)
3842
}
39-
_, err = fmt.Fprintf(f, "mode: %s\n", cfg.BuildCoverMode)
43+
s, err := fmt.Fprintf(f, "mode: %s\n", cfg.BuildCoverMode)
4044
if err != nil {
4145
base.Fatalf("%v", err)
4246
}
4347
coverMerge.f = f
48+
coverMerge.fsize = int64(s)
4449
}
4550

4651
// mergeCoverProfile merges file into the profile stored in testCoverProfile.
4752
// It prints any errors it encounters to ew.
48-
func mergeCoverProfile(ew io.Writer, file string) {
53+
func mergeCoverProfile(file string) {
4954
if coverMerge.f == nil {
5055
return
5156
}
@@ -66,19 +71,25 @@ func mergeCoverProfile(ew io.Writer, file string) {
6671
return
6772
}
6873
if err != nil || string(buf) != expect {
69-
fmt.Fprintf(ew, "error: test wrote malformed coverage profile %s.\n", file)
74+
base.Errorf("error: test wrote malformed coverage profile %s: header %q, expected %q: %v", file, string(buf), expect, err)
7075
return
7176
}
72-
_, err = io.Copy(coverMerge.f, r)
77+
s, err := io.Copy(coverMerge.f, r)
7378
if err != nil {
74-
fmt.Fprintf(ew, "error: saving coverage profile: %v\n", err)
79+
base.Errorf("error: saving coverage profile: %v", err)
80+
return
7581
}
82+
coverMerge.fsize += s
7683
}
7784

7885
func closeCoverProfile() {
7986
if coverMerge.f == nil {
8087
return
8188
}
89+
// Discard any partially written data from a failed merge.
90+
if err := coverMerge.f.Truncate(coverMerge.fsize); err != nil {
91+
base.Errorf("closing coverage profile: %v", err)
92+
}
8293
if err := coverMerge.f.Close(); err != nil {
8394
base.Errorf("closing coverage profile: %v", err)
8495
}

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

Lines changed: 64 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,9 @@ elapsed time in the summary line.
126126
127127
The rule for a match in the cache is that the run involves the same
128128
test binary and the flags on the command line come entirely from a
129-
restricted set of 'cacheable' test flags, defined as -benchtime, -cpu,
130-
-list, -parallel, -run, -short, -timeout, -failfast, -fullpath and -v.
129+
restricted set of 'cacheable' test flags, defined as -benchtime, -cover,
130+
-covermode, -coverprofile, -cpu, -list, -parallel, -run, -short, -timeout,
131+
-failfast, -fullpath and -v.
131132
If a run of go test has any test or non-test flags outside this set,
132133
the result is not cached. To disable test caching, use any test flag
133134
or argument other than the cacheable flags. The idiomatic way to disable
@@ -1353,6 +1354,13 @@ type runCache struct {
13531354
id2 cache.ActionID
13541355
}
13551356

1357+
func coverProfTempFile(a *work.Action) string {
1358+
if a.Objdir == "" {
1359+
panic("internal error: objdir not set in coverProfTempFile")
1360+
}
1361+
return a.Objdir + "_cover_.out"
1362+
}
1363+
13561364
// stdoutMu and lockedStdout provide a locked standard output
13571365
// that guarantees never to interlace writes from multiple
13581366
// goroutines, so that we can have multiple JSON streams writing
@@ -1450,13 +1458,6 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action)
14501458
return nil
14511459
}
14521460

1453-
coverProfTempFile := func(a *work.Action) string {
1454-
if a.Objdir == "" {
1455-
panic("internal error: objdir not set in coverProfTempFile")
1456-
}
1457-
return a.Objdir + "_cover_.out"
1458-
}
1459-
14601461
if p := a.Package; len(p.TestGoFiles)+len(p.XTestGoFiles) == 0 {
14611462
reportNoTestFiles := true
14621463
if cfg.BuildCover && cfg.Experiment.CoverageRedesign && p.Internal.Cover.GenMeta {
@@ -1480,7 +1481,7 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action)
14801481
if err := work.WriteCoverageProfile(b, a, mf, cp, stdout); err != nil {
14811482
return err
14821483
}
1483-
mergeCoverProfile(stdout, cp)
1484+
mergeCoverProfile(cp)
14841485
}
14851486
}
14861487
}
@@ -1643,7 +1644,7 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action)
16431644
a.TestOutput = &buf
16441645
t := fmt.Sprintf("%.3fs", time.Since(t0).Seconds())
16451646

1646-
mergeCoverProfile(cmd.Stdout, a.Objdir+"_cover_.out")
1647+
mergeCoverProfile(a.Objdir + "_cover_.out")
16471648

16481649
if err == nil {
16491650
norun := ""
@@ -1665,7 +1666,7 @@ func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action)
16651666
cmd.Stdout.Write([]byte("\n"))
16661667
}
16671668
fmt.Fprintf(cmd.Stdout, "ok \t%s\t%s%s%s\n", a.Package.ImportPath, t, coveragePercentage(out), norun)
1668-
r.c.saveOutput(a)
1669+
r.c.saveOutput(a, coverProfTempFile(a))
16691670
} else {
16701671
if testFailFast {
16711672
testShouldFailFast.Store(true)
@@ -1763,7 +1764,18 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
17631764
// Note that this list is documented above,
17641765
// so if you add to this list, update the docs too.
17651766
cacheArgs = append(cacheArgs, arg)
1766-
1767+
case "-test.coverprofile",
1768+
"-test.outputdir":
1769+
// The `-coverprofile` and `-outputdir` arguments, which
1770+
// only affect the location of profile output, are cacheable. This
1771+
// is based on the process where, upon a cache hit, stored profile
1772+
// data is copied to the specified output location. This mechanism
1773+
// ensures that output location preferences are honored without
1774+
// modifying the profile's content, thereby justifying their
1775+
// cacheability without impacting cache integrity. This allows
1776+
// cached coverage profiles to be written to different files.
1777+
// Note that this list is documented above,
1778+
// so if you add to this list, update the docs too.
17671779
default:
17681780
// nothing else is cacheable
17691781
if cache.DebugTest {
@@ -1875,6 +1887,21 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
18751887
j++
18761888
}
18771889
c.buf.Write(data[j:])
1890+
1891+
// Write coverage data to profile.
1892+
if cfg.BuildCover {
1893+
// The cached coverprofile has the same expiration time as the
1894+
// test result it corresponds to. That time is already checked
1895+
// above, so we can ignore the entry returned by GetFile here.
1896+
f, _, err := cache.GetFile(cache.Default(), coverProfileAndInputKey(testID, testInputsID))
1897+
if err != nil {
1898+
if cache.DebugTest {
1899+
fmt.Fprintf(os.Stderr, "testcache: %s: test coverage profile not found: %v\n", a.Package.ImportPath, err)
1900+
}
1901+
return false
1902+
}
1903+
mergeCoverProfile(f)
1904+
}
18781905
return true
18791906
}
18801907

@@ -2023,7 +2050,12 @@ func testAndInputKey(testID, testInputsID cache.ActionID) cache.ActionID {
20232050
return cache.Subkey(testID, fmt.Sprintf("inputs:%x", testInputsID))
20242051
}
20252052

2026-
func (c *runCache) saveOutput(a *work.Action) {
2053+
// coverProfileAndInputKey returns the "coverprofile" cache key for the pair (testID, testInputsID).
2054+
func coverProfileAndInputKey(testID, testInputsID cache.ActionID) cache.ActionID {
2055+
return cache.Subkey(testAndInputKey(testID, testInputsID), "coverprofile")
2056+
}
2057+
2058+
func (c *runCache) saveOutput(a *work.Action, coverProfileFile string) {
20272059
if c.id1 == (cache.ActionID{}) && c.id2 == (cache.ActionID{}) {
20282060
return
20292061
}
@@ -2044,19 +2076,37 @@ func (c *runCache) saveOutput(a *work.Action) {
20442076
if err != nil {
20452077
return
20462078
}
2079+
var saveCoverProfile = func(testID cache.ActionID) {}
2080+
if coverProfileFile != "" {
2081+
coverProfile, err := os.Open(coverProfileFile)
2082+
if err == nil {
2083+
saveCoverProfile = func(testID cache.ActionID) {
2084+
cache.Default().Put(coverProfileAndInputKey(testID, testInputsID), coverProfile)
2085+
}
2086+
defer func() {
2087+
if err := coverProfile.Close(); err != nil && cache.DebugTest {
2088+
fmt.Fprintf(os.Stderr, "testcache: %s: closing temporary coverprofile: %v", a.Package.ImportPath, err)
2089+
}
2090+
}()
2091+
} else if cache.DebugTest {
2092+
fmt.Fprintf(os.Stderr, "testcache: %s: failed to open temporary coverprofile: %s", a.Package.ImportPath, err)
2093+
}
2094+
}
20472095
if c.id1 != (cache.ActionID{}) {
20482096
if cache.DebugTest {
20492097
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))
20502098
}
20512099
cache.PutNoVerify(cache.Default(), c.id1, bytes.NewReader(testlog))
20522100
cache.PutNoVerify(cache.Default(), testAndInputKey(c.id1, testInputsID), bytes.NewReader(a.TestOutput.Bytes()))
2101+
saveCoverProfile(c.id1)
20532102
}
20542103
if c.id2 != (cache.ActionID{}) {
20552104
if cache.DebugTest {
20562105
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))
20572106
}
20582107
cache.PutNoVerify(cache.Default(), c.id2, bytes.NewReader(testlog))
20592108
cache.PutNoVerify(cache.Default(), testAndInputKey(c.id2, testInputsID), bytes.NewReader(a.TestOutput.Bytes()))
2109+
saveCoverProfile(c.id2)
20602110
}
20612111
}
20622112

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,45 @@ go test testcache -run=TestOSArgs -fullpath
128128
go test testcache -run=TestOSArgs -fullpath
129129
stdout '\(cached\)'
130130

131+
# Ensure that specifying a cover profile does not prevent test results from being cached.
132+
go test testcache -run=TestCoverageCache -coverprofile=coverage.out
133+
go test testcache -run=TestCoverageCache -coverprofile=coverage.out
134+
stdout '\(cached\)'
135+
exists coverage.out
136+
grep -q 'mode: set' coverage.out
137+
grep -q 'testcache/hello.go:' coverage.out
138+
139+
# A new -coverprofile file should use the cached coverage profile contents.
140+
go test testcache -run=TestCoverageCache -coverprofile=coverage2.out
141+
stdout '\(cached\)'
142+
cmp coverage.out coverage2.out
143+
144+
# Explicitly setting the default covermode should still use cache.
145+
go test testcache -run=TestCoverageCache -coverprofile=coverage_set.out -covermode=set
146+
stdout '\(cached\)'
147+
cmp coverage.out coverage_set.out
148+
149+
# A new -covermode should not use the cached coverage profile.
150+
go test testcache -run=TestCoverageCache -coverprofile=coverage_atomic.out -covermode=atomic
151+
! stdout '\(cached\)'
152+
! cmp coverage.out coverage_atomic.out
153+
grep -q 'mode: atomic' coverage_atomic.out
154+
grep -q 'testcache/hello.go:' coverage_atomic.out
155+
156+
# A new -coverpkg should not use the cached coverage profile.
157+
go test testcache -run=TestCoverageCache -coverprofile=coverage_pkg.out -coverpkg=all
158+
! stdout '\(cached\)'
159+
! cmp coverage.out coverage_pkg.out
160+
161+
# Test that -v doesn't prevent caching.
162+
go test testcache -v -run=TestCoverageCache -coverprofile=coverage_v.out
163+
go test testcache -v -run=TestCoverageCache -coverprofile=coverage_v2.out
164+
stdout '\(cached\)'
165+
cmp coverage_v.out coverage_v2.out
166+
167+
# Test that -count affects caching.
168+
go test testcache -run=TestCoverageCache -coverprofile=coverage_count.out -count=2
169+
! stdout '\(cached\)'
131170

132171
# Executables within GOROOT and GOPATH should affect caching,
133172
# even if the test does not stat them explicitly.
@@ -159,6 +198,18 @@ This file is outside of GOPATH.
159198
-- testcache/script.sh --
160199
#!/bin/sh
161200
exit 0
201+
-- testcache/hello.go --
202+
package testcache
203+
204+
import "fmt"
205+
206+
func HelloWorld(name string) string {
207+
if name == "" {
208+
return "Hello, World!"
209+
}
210+
return fmt.Sprintf("Hello, %s!", name)
211+
}
212+
162213
-- testcache/testcache_test.go --
163214
// Copyright 2017 The Go Authors. All rights reserved.
164215
// Use of this source code is governed by a BSD-style
@@ -262,6 +313,18 @@ func TestOSArgs(t *testing.T) {
262313
func TestBenchtime(t *testing.T) {
263314
}
264315

316+
func TestCoverageCache(t *testing.T) {
317+
result := HelloWorld("")
318+
if result != "Hello, World!" {
319+
t.Errorf("Expected 'Hello, World!', got '%s'", result)
320+
}
321+
322+
result = HelloWorld("Go")
323+
if result != "Hello, Go!" {
324+
t.Errorf("Expected 'Hello, Go!', got '%s'", result)
325+
}
326+
}
327+
265328
-- mkold.go --
266329
package main
267330

0 commit comments

Comments
 (0)