Skip to content

Commit 5e0f1be

Browse files
committed
cmd/go: only invalidate coverage report 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 didn't track changes to packages specified by -coverpkg. This change adds selective cache invalidation by moving covered package tracking from the main test cache to only the coverage report cache: - Add hashCoveredPackages function to hash source files of covered packages - Modify coverProfileAndInputKey to include covered packages hash - Add test case to verify coverage report cache invalidation works correctly When any file in a covered package changes, only the coverage report cache is invalidated while test results remain cached, improving performance while 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 5e0f1be

File tree

3 files changed

+278
-1
lines changed

3 files changed

+278
-1
lines changed

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1927,6 +1927,25 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
19271927
var errBadTestInputs = errors.New("error parsing test inputs")
19281928
var testlogMagic = []byte("# test log\n") // known to testing/internal/testdeps/deps.go
19291929

1930+
// hashCoveredPackages writes to h a hash of the source files of the covered packages.
1931+
func hashCoveredPackages(h io.Writer, pkgs []*load.Package) {
1932+
for _, pkg := range pkgs {
1933+
fmt.Fprintf(h, "coverpkg %s", pkg.ImportPath)
1934+
// Include source file hashes to detect changes
1935+
for _, file := range pkg.GoFiles {
1936+
if fh := hashStat(filepath.Join(pkg.Dir, file)); fh != (cache.ActionID{}) {
1937+
fmt.Fprintf(h, " %x", fh)
1938+
}
1939+
}
1940+
for _, file := range pkg.CgoFiles {
1941+
if fh := hashStat(filepath.Join(pkg.Dir, file)); fh != (cache.ActionID{}) {
1942+
fmt.Fprintf(h, " %x", fh)
1943+
}
1944+
}
1945+
fmt.Fprintf(h, "\n")
1946+
}
1947+
}
1948+
19301949
// computeTestInputsID computes the "test inputs ID"
19311950
// (see comment in tryCacheWithID above) for the
19321951
// test log.
@@ -2071,7 +2090,11 @@ func testAndInputKey(testID, testInputsID cache.ActionID) cache.ActionID {
20712090

20722091
// coverProfileAndInputKey returns the "coverprofile" cache key for the pair (testID, testInputsID).
20732092
func coverProfileAndInputKey(testID, testInputsID cache.ActionID) cache.ActionID {
2074-
return cache.Subkey(testAndInputKey(testID, testInputsID), "coverprofile")
2093+
// Include covered packages hash to invalidate coverage reports when covered packages change
2094+
h := cache.NewHash("coverprofile")
2095+
h.Write(testAndInputKey(testID, testInputsID)[:])
2096+
hashCoveredPackages(h, testCoverPkgs)
2097+
return h.Sum()
20752098
}
20762099

20772100
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+
}

test_coverage_cache.sh

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
#!/bin/bash
2+
3+
# Test script to verify that coverage cache invalidation works correctly
4+
# when coverpkg sources change but only invalidates coverage reports, not all cache
5+
6+
set -e
7+
8+
export PATH=/Users/ryancurrah/git/github.com/ryancurrah/go/bin:$PATH
9+
export GOCACHE=/tmp/test_cache_$$
10+
mkdir -p $GOCACHE
11+
12+
echo "Creating test project..."
13+
mkdir -p /tmp/test_proj_$$/{sub,sum}
14+
cd /tmp/test_proj_$$
15+
16+
# Create go.mod
17+
cat > go.mod << EOF
18+
module test_proj
19+
20+
go 1.21
21+
EOF
22+
23+
# Create main package
24+
cat > some_func.go << 'EOF'
25+
package main
26+
27+
import "test_proj/sum"
28+
29+
func SomeFunc(a, b int) int {
30+
if a == 0 && b == 0 {
31+
return 0
32+
}
33+
return sum.Sum(a, b)
34+
}
35+
36+
func main() {}
37+
EOF
38+
39+
cat > some_func_test.go << 'EOF'
40+
package main
41+
42+
import "testing"
43+
44+
func Test_SomeFunc(t *testing.T) {
45+
result := SomeFunc(1, 1)
46+
if result != 2 {
47+
t.Errorf("Expected 2, got %d", result)
48+
}
49+
}
50+
EOF
51+
52+
# Create sub package
53+
cat > sub/sub.go << 'EOF'
54+
package sub
55+
56+
func Sub(a, b int) int {
57+
if a == 0 && b == 0 {
58+
return 0
59+
}
60+
return a - b
61+
}
62+
EOF
63+
64+
cat > sub/sub_test.go << 'EOF'
65+
package sub
66+
67+
import "testing"
68+
69+
func Test_Sub(t *testing.T) {
70+
result := Sub(1, 1)
71+
if result != 0 {
72+
t.Errorf("Expected 0, got %d", result)
73+
}
74+
}
75+
EOF
76+
77+
# Create sum package
78+
cat > sum/sum.go << 'EOF'
79+
package sum
80+
81+
func Sum(a, b int) int {
82+
if a == 0 {
83+
return b
84+
}
85+
return a + b
86+
}
87+
EOF
88+
89+
echo "Running initial test with coverage..."
90+
go test -coverpkg=./... -coverprofile=cover1.out ./... -v
91+
92+
echo "Running test again to check caching..."
93+
go test -coverpkg=./... -coverprofile=cover2.out ./... -v
94+
95+
echo "Modifying sub/sub.go..."
96+
cat > sub/sub.go << 'EOF'
97+
package sub
98+
99+
func Sub(a, b int) int {
100+
if a == 0 && b == 0 || a == -100 {
101+
return 0
102+
}
103+
return a - b
104+
}
105+
EOF
106+
107+
echo "Running test after modification..."
108+
go test -coverpkg=./... -coverprofile=cover3.out ./... -v
109+
110+
echo "Test completed successfully!"
111+
112+
# Cleanup
113+
cd /
114+
rm -rf /tmp/test_proj_$$
115+
rm -rf $GOCACHE
116+
117+
echo "Coverage cache test passed!"

0 commit comments

Comments
 (0)