Skip to content

Commit d8307ea

Browse files
committed
cmd/go: invalidate coverage report cache when coverpkg sources change
When using -coverpkg with coverage profiling, the coverage cache keys did not include the source file hashes of the covered packages. This could result in stale coverage reports that reference outdated line numbers when covered package sources were modified between test runs. This change adds a cached hash of the covered packages' source files that gets included in the coverage cache key computation. The hash is computed once per test execution using sync.Once to avoid redundant file system operations across multiple test packages. The fix ensures that coverage profiles are properly invalidated when any source file in the covered packages changes, preventing cache hits with stale line number references. Fixes golang#74873
1 parent 6fbad4b commit d8307ea

File tree

2 files changed

+180
-1
lines changed

2 files changed

+180
-1
lines changed

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

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,10 @@ var (
554554
testTimeout time.Duration // -timeout flag
555555
testV testVFlag // -v flag
556556
testVet = vetFlag{flags: defaultVetFlags} // -vet flag
557+
558+
// Cache for covered packages hash to avoid recomputing for each test package
559+
testCoverPkgsHash cache.ActionID
560+
testCoverPkgsHashOnce sync.Once
557561
)
558562

559563
type testVFlag struct {
@@ -1927,6 +1931,37 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
19271931
var errBadTestInputs = errors.New("error parsing test inputs")
19281932
var testlogMagic = []byte("# test log\n") // known to testing/internal/testdeps/deps.go
19291933

1934+
// hashCoveredPackages writes to h a hash of the source files of the covered packages.
1935+
func hashCoveredPackages(h io.Writer, pkgs []*load.Package) {
1936+
for _, pkg := range pkgs {
1937+
fmt.Fprintf(h, "coverpkg %s", pkg.ImportPath)
1938+
// Include source file hashes to detect changes
1939+
for _, file := range pkg.GoFiles {
1940+
if fh := hashStat(filepath.Join(pkg.Dir, file)); fh != (cache.ActionID{}) {
1941+
fmt.Fprintf(h, " %x", fh)
1942+
}
1943+
}
1944+
for _, file := range pkg.CgoFiles {
1945+
if fh := hashStat(filepath.Join(pkg.Dir, file)); fh != (cache.ActionID{}) {
1946+
fmt.Fprintf(h, " %x", fh)
1947+
}
1948+
}
1949+
fmt.Fprintf(h, "\n")
1950+
}
1951+
}
1952+
1953+
// getCoveredPackagesHash returns the cached hash of covered packages, computing it only once.
1954+
func getCoveredPackagesHash() cache.ActionID {
1955+
testCoverPkgsHashOnce.Do(func() {
1956+
if len(testCoverPkgs) > 0 {
1957+
h := cache.NewHash("coverpkgs")
1958+
hashCoveredPackages(h, testCoverPkgs)
1959+
testCoverPkgsHash = h.Sum()
1960+
}
1961+
})
1962+
return testCoverPkgsHash
1963+
}
1964+
19301965
// computeTestInputsID computes the "test inputs ID"
19311966
// (see comment in tryCacheWithID above) for the
19321967
// test log.
@@ -2071,7 +2106,14 @@ func testAndInputKey(testID, testInputsID cache.ActionID) cache.ActionID {
20712106

20722107
// coverProfileAndInputKey returns the "coverprofile" cache key for the pair (testID, testInputsID).
20732108
func coverProfileAndInputKey(testID, testInputsID cache.ActionID) cache.ActionID {
2074-
return cache.Subkey(testAndInputKey(testID, testInputsID), "coverprofile")
2109+
// Include covered packages hash to invalidate coverage reports when covered packages change
2110+
h := cache.NewHash("coverprofile")
2111+
testAndInputs := testAndInputKey(testID, testInputsID)
2112+
h.Write(testAndInputs[:])
2113+
// Use cached hash instead of recomputing for each test package
2114+
coveredPkgsHash := getCoveredPackagesHash()
2115+
h.Write(coveredPkgsHash[:])
2116+
return h.Sum()
20752117
}
20762118

20772119
func (c *runCache) saveOutput(a *work.Action) {
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
env GO111MODULE=on
2+
3+
# Test for bug where cached coverage profiles with -coverpkg can contain
4+
# outdated line references when source files are modified.
5+
# This reproduces the issue where coverage data from cache may reference
6+
# lines that no longer exist in the updated source files.
7+
8+
[short] skip
9+
[GODEBUG:gocacheverify=1] skip
10+
11+
# We're testing cache behavior, so start with a clean GOCACHE.
12+
env GOCACHE=$WORK/cache
13+
14+
# Create a project structure with multiple packages
15+
# proj/
16+
# some_func.go
17+
# some_func_test.go
18+
# sub/
19+
# sub.go
20+
# sub_test.go
21+
# sum/
22+
# sum.go
23+
24+
# Switch to the proj directory
25+
cd proj
26+
27+
# Run tests with -coverpkg to collect coverage for all packages
28+
go test -coverpkg=proj/... -coverprofile=cover1.out ./...
29+
stdout 'coverage:'
30+
31+
# Verify the first coverage profile exists and has expected content
32+
exists cover1.out
33+
grep -q 'proj/sub/sub.go:' cover1.out
34+
35+
# Run again to ensure caching works
36+
go test -coverpkg=proj/... -coverprofile=cover1_cached.out ./...
37+
stdout '\(cached\)'
38+
stdout 'coverage:'
39+
40+
# Note: Due to the bug, cached coverage profiles may have duplicate entries
41+
# This is the issue we're trying to fix
42+
43+
# Now modify sub.go to change line structure - this will invalidate
44+
# the cache for sub package but not for proj package
45+
cp ../sub_modified.go sub/sub.go
46+
47+
# The bug manifests as duplicate or invalid line references in the coverage profile
48+
# After modifying sub.go, we should not have both old and new line references
49+
go test -coverpkg=proj/... -coverprofile=cover2.out ./...
50+
stdout 'coverage:'
51+
52+
# With the bug present, we would see duplicate entries for the same lines
53+
# This demonstrates the bug - there should be duplicates in the coverage profile
54+
grep 'proj/sub/sub.go:' cover2.out
55+
# The fix should ensure that only the new line format exists, not the old one
56+
grep 'proj/sub/sub.go:3.24,4.35' cover2.out
57+
# This should fail if the stale coverage line exists (the bug is present)
58+
! grep 'proj/sub/sub.go:3.24,4.22' cover2.out
59+
60+
-- proj/go.mod --
61+
module proj
62+
63+
go 1.21
64+
65+
-- proj/some_func.go --
66+
package proj
67+
68+
import "proj/sum"
69+
70+
func SomeFunc(a, b int) int {
71+
if a == 0 && b == 0 {
72+
return 0
73+
}
74+
return sum.Sum(a, b)
75+
}
76+
77+
-- proj/some_func_test.go --
78+
package proj
79+
80+
import (
81+
"testing"
82+
)
83+
84+
func Test_SomeFunc(t *testing.T) {
85+
t.Run("test1", func(t *testing.T) {
86+
result := SomeFunc(1, 1)
87+
if result != 2 {
88+
t.Errorf("Expected 2, got %d", result)
89+
}
90+
})
91+
}
92+
93+
-- proj/sub/sub.go --
94+
package sub
95+
96+
func Sub(a, b int) int {
97+
if a == 0 && b == 0 {
98+
return 0
99+
}
100+
return a - b
101+
}
102+
103+
-- proj/sub/sub_test.go --
104+
package sub
105+
106+
import (
107+
"testing"
108+
)
109+
110+
func Test_Sub(t *testing.T) {
111+
t.Run("test_sub1", func(t *testing.T) {
112+
result := Sub(1, 1)
113+
if result != 0 {
114+
t.Errorf("Expected 0, got %d", result)
115+
}
116+
})
117+
}
118+
119+
-- proj/sum/sum.go --
120+
package sum
121+
122+
func Sum(a, b int) int {
123+
if a == 0 {
124+
return b
125+
}
126+
return a + b
127+
}
128+
129+
-- sub_modified.go --
130+
package sub
131+
132+
func Sub(a, b int) int {
133+
if a == 0 && b == 0 || a == -100 {
134+
return 0
135+
}
136+
return a - b
137+
}

0 commit comments

Comments
 (0)