Skip to content

Commit f7cd72d

Browse files
committed
cmd/go: invalidate test cache when -coverpkg sources change
When using -coverpkg, test caching could return stale coverage data if covered packages were modified but the test package itself was unchanged. This occurred because the test cache only considered the test package's dependencies, not the broader set of packages specified by -coverpkg. This change adds dependency tracking for covered packages and includes their source file hashes in cache validation. When any file in a covered package changes, the test cache is properly invalidated, ensuring fresh coverage data is generated. Fixes issue where coverage reports contained duplicate entries with different line ranges after source modifications in covered packages. Fixes #74873
1 parent 6fbad4b commit f7cd72d

File tree

2 files changed

+166
-2
lines changed

2 files changed

+166
-2
lines changed

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

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,8 +1290,15 @@ func builderTest(b *work.Builder, ctx context.Context, pkgOpts load.PackageOpts,
12901290
// Add a dependence edge from p to writeCoverMetaAct,
12911291
// which needs to know the name of that meta-data
12921292
// file.
1293-
compileAction := b.CompileAction(work.ModeBuild, work.ModeBuild, p)
1294-
writeCoverMetaAct.Deps = append(writeCoverMetaAct.Deps, compileAction)
1293+
writeCoverMetaAct.Deps = append(writeCoverMetaAct.Deps, b.CompileAction(work.ModeBuild, work.ModeBuild, p))
1294+
1295+
// When using -coverpkg, the test action should also depend on the
1296+
// compilation of the packages being covered. These compilation actions
1297+
// become dependencies of writeCoverMetaAct.
1298+
for _, testCoverPkg := range testCoverPkgs {
1299+
coverAction := b.CompileAction(work.ModeBuild, work.ModeBuild, testCoverPkg)
1300+
writeCoverMetaAct.Deps = append(writeCoverMetaAct.Deps, coverAction)
1301+
}
12951302
}
12961303
}
12971304
runAction.Objdir = testDir
@@ -1833,6 +1840,7 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
18331840

18341841
h := cache.NewHash("testResult")
18351842
fmt.Fprintf(h, "test binary %s args %q execcmd %q", id, cacheArgs, work.ExecCmd)
1843+
hashCoveredPackages(h, testCoverPkgs)
18361844
testID := h.Sum()
18371845
if c.id1 == (cache.ActionID{}) {
18381846
c.id1 = testID
@@ -1927,6 +1935,25 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
19271935
var errBadTestInputs = errors.New("error parsing test inputs")
19281936
var testlogMagic = []byte("# test log\n") // known to testing/internal/testdeps/deps.go
19291937

1938+
// hashCoveredPackages writes to h a hash of the source files of the covered packages.
1939+
func hashCoveredPackages(h io.Writer, pkgs []*load.Package) {
1940+
for _, pkg := range pkgs {
1941+
fmt.Fprintf(h, "coverpkg %s", pkg.ImportPath)
1942+
// Include source file hashes to detect changes
1943+
for _, file := range pkg.GoFiles {
1944+
if fh := hashStat(filepath.Join(pkg.Dir, file)); fh != (cache.ActionID{}) {
1945+
fmt.Fprintf(h, " %x", fh)
1946+
}
1947+
}
1948+
for _, file := range pkg.CgoFiles {
1949+
if fh := hashStat(filepath.Join(pkg.Dir, file)); fh != (cache.ActionID{}) {
1950+
fmt.Fprintf(h, " %x", fh)
1951+
}
1952+
}
1953+
fmt.Fprintf(h, "\n")
1954+
}
1955+
}
1956+
19301957
// computeTestInputsID computes the "test inputs ID"
19311958
// (see comment in tryCacheWithID above) for the
19321959
// test log.
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)