Skip to content

Commit 956f8a6

Browse files
committed
internal/coverage/cfile: harden the coverage snapshot test
The existing testpoint TestCoverageSnapshot will fail if we happen to be selecting a set of packages for inclusion in the profile that don't include internal/coverage/cfile. Example: $ cd `go env GOROOT` $ cd src/internal/coverage $ go test -coverpkg=internal/coverage/decodecounter ./... ... --- FAIL: TestCoverageSnapshot (0.00s) ts_test.go:102: 0.276074 0.276074 ts_test.go:104: erroneous snapshots, C1 >= C2 = true C1=0.276074 C2=0.276074 To ensure that this doesn't happen, extract the test in question out into a separate file with a special build tag, and then have the original testpoint do a "go test -cover -tags ... " run to make sure that for that specific test run the cfile package is instrumented. Fixes #67951. Change-Id: I8ac6e07e1a6d93275b8c6acabfce85e04c70a102 Reviewed-on: https://go-review.googlesource.com/c/go/+/592200 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: David Chase <[email protected]>
1 parent 7bfc824 commit 956f8a6

File tree

2 files changed

+68
-36
lines changed

2 files changed

+68
-36
lines changed
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright 2022 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
//go:build SELECT_USING_THIS_TAG
6+
7+
package cfile
8+
9+
import "testing"
10+
11+
var funcInvoked bool
12+
13+
//go:noinline
14+
func thisFunctionOnlyCalledFromSnapshotTest(n int) int {
15+
if funcInvoked {
16+
panic("bad")
17+
}
18+
funcInvoked = true
19+
20+
// Contents here not especially important, just so long as we
21+
// have some statements.
22+
t := 0
23+
for i := 0; i < n; i++ {
24+
for j := 0; j < i; j++ {
25+
t += i ^ j
26+
}
27+
}
28+
return t
29+
}
30+
31+
// Tests runtime/coverage.snapshot() directly. Note that if
32+
// coverage is not enabled, the hook is designed to just return
33+
// zero.
34+
func TestCoverageSnapshotImpl(t *testing.T) {
35+
C1 := Snapshot()
36+
thisFunctionOnlyCalledFromSnapshotTest(15)
37+
C2 := Snapshot()
38+
cond := "C1 > C2"
39+
val := C1 > C2
40+
if testing.CoverMode() != "" {
41+
cond = "C1 >= C2"
42+
val = C1 >= C2
43+
}
44+
t.Logf("%f %f\n", C1, C2)
45+
if val {
46+
t.Errorf("erroneous snapshots, %s = true C1=%f C2=%f",
47+
cond, C1, C2)
48+
}
49+
}

src/internal/coverage/cfile/ts_test.go

Lines changed: 19 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -66,43 +66,26 @@ func TestTestSupport(t *testing.T) {
6666
}
6767
}
6868

69-
var funcInvoked bool
70-
71-
//go:noinline
72-
func thisFunctionOnlyCalledFromSnapshotTest(n int) int {
73-
if funcInvoked {
74-
panic("bad")
75-
}
76-
funcInvoked = true
77-
78-
// Contents here not especially important, just so long as we
79-
// have some statements.
80-
t := 0
81-
for i := 0; i < n; i++ {
82-
for j := 0; j < i; j++ {
83-
t += i ^ j
84-
}
85-
}
86-
return t
87-
}
88-
89-
// Tests runtime/coverage.snapshot() directly. Note that if
90-
// coverage is not enabled, the hook is designed to just return
91-
// zero.
69+
// Kicks off a sub-test to verify that Snapshot() works properly.
70+
// We do this as a separate shell-out, so as to avoid potential
71+
// interactions with -coverpkg. For example, if you do
72+
//
73+
// $ cd `go env GOROOT`
74+
// $ cd src/internal/coverage
75+
// $ go test -coverpkg=internal/coverage/decodecounter ./...
76+
// ...
77+
// $
78+
//
79+
// The previous version of this test could fail due to the fact
80+
// that "cfile" itself was not being instrumented, as in the
81+
// scenario above.
9282
func TestCoverageSnapshot(t *testing.T) {
93-
C1 := Snapshot()
94-
thisFunctionOnlyCalledFromSnapshotTest(15)
95-
C2 := Snapshot()
96-
cond := "C1 > C2"
97-
val := C1 > C2
98-
if testing.CoverMode() != "" {
99-
cond = "C1 >= C2"
100-
val = C1 >= C2
101-
}
102-
t.Logf("%f %f\n", C1, C2)
103-
if val {
104-
t.Errorf("erroneous snapshots, %s = true C1=%f C2=%f",
105-
cond, C1, C2)
83+
testenv.MustHaveGoRun(t)
84+
args := []string{"test", "-tags", "SELECT_USING_THIS_TAG",
85+
"-cover", "-run=TestCoverageSnapshotImpl", "internal/coverage/cfile"}
86+
cmd := exec.Command(testenv.GoToolPath(t), args...)
87+
if b, err := cmd.CombinedOutput(); err != nil {
88+
t.Fatalf("go test failed (%v): %s", err, b)
10689
}
10790
}
10891

0 commit comments

Comments
 (0)