Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit cb64dbe

Browse files
fix(cmd/go): only invalidate coverage cache when coverpkg sources change
Instead of invalidating all test cache when covered packages change, only invalidate the coverage report cache. This improves performance by allowing test results to remain cached while ensuring coverage reports are updated when covered packages are modified. - Remove covered packages hash from main test ID calculation - Move coverage invalidation to coverProfileAndInputKey function - Remove unnecessary build dependencies for covered packages Fixes #74873 
1 parent 6fbad4b commit cb64dbe

File tree

2 files changed

+162
-3
lines changed

2 files changed

+162
-3
lines changed

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

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1290,8 +1290,7 @@ 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))
12951294
}
12961295
}
12971296
runAction.Objdir = testDir
@@ -1927,6 +1926,25 @@ func (c *runCache) tryCacheWithID(b *work.Builder, a *work.Action, id string) bo
19271926
var errBadTestInputs = errors.New("error parsing test inputs")
19281927
var testlogMagic = []byte("# test log\n") // known to testing/internal/testdeps/deps.go
19291928

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

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

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

AltStyle によって変換されたページ (->オリジナル) /