Skip to content

Commit 2af3f27

Browse files
authored
Support integration test coverage system (coverageredesign) (#4397)
go1.25 removes the current runtime code coverage system adopted by rules_go (nocoverageredesign) https://go-review.googlesource.com/c/go/+/644997 which makes rules_go incompatible with the latest upcoming version of go. The change completely adopts the new integration test friendly coverageredesign system (https://go.dev/blog/integration-test-coverage) in rules_go. Below I describe how the current rules_go coverage system currently works, what changes in the new coverage redesign, and what needs to change as a result in rules_go. How nocoverageredesign currently works: - rules_go invokes `go tool cover` to generate instrumented source files that are then passed to the Go compiler (in compilepkg.go). Instrumented source files update generated package variable counters. - The rules_go generated test main (generate_test_main.go) calls testing.RegisterCover to set a global variable that tracks references to coverage generated package local variables that coverage instrumented code modify (ref: https://github.com/golang/go/blob/go1.24.5/src/testing/cover.go#L72) This replicates the behavior in go test's generated test main (cmd/go/internal/load/test.go) - rules_go specially adds a call to coverdata.RegisterFile call to all coverage instrumented files which connects each of the file coverage counter variables with a global object and enables rules_go to customize the file name that coverage variables are associated with. - It's important to note that because rules_go accesses to this global coverage struct, it's able to manipulate the file path name associated with coverage counter variables. This is important because the lcov format that bazel uses expect exec root relative files. - When a test exits, This global object is flushed in a call to `testing.coverReport()` to generate a coverage report. This reports gets written a file configured in test.coverprofile flag. - Via a special SetPanicOnExit0 hook, the coverage file set in test.coverprofile is coverted to a Bazel friendly Lcov format via logic in bzltestutil/lcov.go What changes in coverageredesign: - A new flexible system for configuring coverage behavior. Instead of a single esting.RegisterCover function, there is a new testdeps package that exposes hooks to configure behavior. - The global variable (https://github.com/golang/go/blob/go1.24.5/src/testing/cover.go#L72) that tracked all coverage data is completely removed. Instead, functions in https://pkg.go.dev/internal/coverage/cfile that are called when writing coverage reports on test exit are internally aware of and able to access coverage counter variable data. - Functionality to flush coverage counter variables write to an intermediate coverage directory in a binary format. Hooks configured in the testdeps package are responsible for translating this binary format into a human readable coverage output file. - 'go tool cover' takes a new package based configuration "pkgcfg" option that enables this new coverage instrumentation logic in coverage source files. - Coverage counters can be flushed on demand via the APIs of "runtime/coverage" This allows coverage to work outside of test situations where coverage used to rely on exit hooks generated into test mains to flush and write coverage data. What needs to change in rules_go as a result: - Use the new testdeps configuration system in the rules_go generated test main. We can following the example of the go test generated test main. ref: https://github.com/golang/go/blob/go1.24.5/src/cmd/go/internal/load/test.go#L986 - Invoke 'go tool cover' with the new -pkgcfg flag. We can follow the example of how the go toolchain invokes coverage redesign instrumentation in https://github.com/golang/go/blob/go1.24.5/src/cmd/go/internal/work/exec.go#L1932 - Not strictly necessary but removes technical debt, call bzltestutil.ConvertCoverToLcov inside of testdeps.CoverProcessTestDirFunc hook wrapper instead of the bzltestutil.LcovTestDeps SetPanicOnExit0 hook. - Adjust the output of lcov profile to make file path keys in coverage files execution root relative. Implementation Note: Unfortunately, we still need github.com/bazelbuild/rules_go/go/tools/coverdata dependencies in coverage source files because the lcov coverage format expects file paths associated with coverage data to be execution root relative. coverageredesign writes Go coverage file data in the format of `importpath/file_name` which is not execution root (e.g. `src/`) relative. During compilation is the only place we have the information to map `importpath/file_name` to an execution root relative path so we have to continue to use the trick of generating code that corrects the file path coverage mapping at runtime. This makes it hard for coverage to work outside of tests because depend on a special github.com/bazelbuild/rules_go/go/tools/coverdata dependency that can't be guaranteed because we don't control a generated test main in these situations. If Go coverage filepaths can be configured during invocations of the `go tool cover`, we can break this dependency. I've filed an upstream go ticket to descrbe this issue golang/go#74749. ref #3513
1 parent 55932be commit 2af3f27

File tree

14 files changed

+202
-135
lines changed

14 files changed

+202
-135
lines changed

go/private/actions/compilepkg.bzl

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,8 @@ def emit_compilepkg(
120120
)
121121

122122
if cover and go.coverdata:
123-
if go.mode.race:
124-
cover_mode = "atomic"
125-
else:
126-
cover_mode = "set"
123+
# Always use atomic mode as the "runtime/coverage" APIs require it.
124+
cover_mode = "atomic"
127125
shared_args.add("-cover_mode", cover_mode)
128126
compile_args.add("-cover_format", go.mode.cover_format)
129127
compile_args.add_all(cover, before_each = "-cover")

go/private/rules/test.bzl

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,9 @@ def _go_test_impl(ctx):
120120
arguments = go.builder_args(go, "gentestmain", use_path_mapping = True)
121121
arguments.add("-output", main_go)
122122
if go.coverage_enabled:
123-
if go.mode.race:
124-
arguments.add("-cover_mode", "atomic")
125-
else:
126-
arguments.add("-cover_mode", "set")
123+
# Always use atomic mode as the "runtime/coverage" APIs require it
124+
# and test behavior should follow non-test behavior.
125+
arguments.add("-cover_mode", "atomic")
127126
arguments.add("-cover_format", go.mode.cover_format)
128127
arguments.add(
129128
# the l is the alias for the package under test, the l_test must be the

go/private/sdk.bzl

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -433,14 +433,6 @@ def _sdk_build_file(ctx, platform, version, experiments):
433433
ctx.file("ROOT")
434434
goos, _, goarch = platform.partition("_")
435435

436-
pv = parse_version(version)
437-
if pv != None and pv[1] >= 20:
438-
# Turn off coverageredesign GOEXPERIMENT on 1.20+
439-
# until rules_go is updated to work with the
440-
# coverage redesign.
441-
if not "nocoverageredesign" in experiments and not "coverageredesign" in experiments:
442-
experiments = experiments + ["nocoverageredesign"]
443-
444436
ctx.template(
445437
"BUILD.bazel",
446438
ctx.path(ctx.attr._sdk_build_file),

go/tools/builders/compilepkg.go

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package main
1818

1919
import (
20+
"crypto/sha256"
2021
"errors"
2122
"flag"
2223
"fmt"
@@ -243,7 +244,8 @@ func compileArchive(
243244
goSrcsNogo := append([]string{}, goSrcs...)
244245
cgoSrcsNogo := append([]string{}, cgoSrcs...)
245246

246-
// Instrument source files for coverage.
247+
// Instrument source files in a package for coverage.
248+
var coverageCfg string
247249
if coverMode != "" {
248250
relCoverPath := make(map[string]string)
249251
for _, s := range coverSrcs {
@@ -254,6 +256,12 @@ func compileArchive(
254256
if cgoEnabled {
255257
combined = append(combined, cgoSrcs...)
256258
}
259+
260+
var (
261+
coverIn []string
262+
coverOut []string
263+
srcPathMapping = make(map[string]string)
264+
)
257265
for i, origSrc := range combined {
258266
if _, ok := relCoverPath[origSrc]; !ok {
259267
continue
@@ -269,7 +277,16 @@ func compileArchive(
269277
case "lcov":
270278
// Bazel merges lcov reports across languages and thus assumes
271279
// that the source file paths are relative to the exec root.
280+
//
281+
// In the go's coverageredesign, rules_go no longer is able to
282+
// set the filepath key generated to Go's coverage output files
283+
// so we keep a mapping from importpath/filename format emitted
284+
// by the go runtime in the coverageredesign to the exec root
285+
// relative source path required by lcov. We will use this mapping
286+
// to write the lcov file with the expected exec root relative source
287+
// path.
272288
srcName = relCoverPath[origSrc]
289+
srcPathMapping[srcName] = path.Join(importPath, filepath.Base(srcName))
273290
default:
274291
return fmt.Errorf("invalid value for -cover_format: %q", coverFormat)
275292
}
@@ -278,12 +295,10 @@ func compileArchive(
278295
if ext := filepath.Ext(stem); ext != "" {
279296
stem = stem[:len(stem)-len(ext)]
280297
}
281-
coverVar := fmt.Sprintf("Cover_%s_%d_%s", sanitizePathForIdentifier(importPath), i, sanitizePathForIdentifier(stem))
282-
coverVar = strings.ReplaceAll(coverVar, "_", "Z")
283298
coverSrc := filepath.Join(workDir, fmt.Sprintf("cover_%d.go", i))
284-
if err := instrumentForCoverage(goenv, origSrc, srcName, coverVar, coverMode, coverSrc); err != nil {
285-
return err
286-
}
299+
300+
coverIn = append(coverIn, origSrc)
301+
coverOut = append(coverOut, coverSrc)
287302

288303
if i < len(goSrcs) {
289304
goSrcs[i] = coverSrc
@@ -292,6 +307,17 @@ func compileArchive(
292307

293308
cgoSrcs[i-len(goSrcs)] = coverSrc
294309
}
310+
311+
// Modeled after go toolchain's coverage variables configuration.
312+
// https://github.com/golang/go/blob/go1.24.5/src/cmd/go/internal/work/exec.go#L1932
313+
sum := sha256.Sum256([]byte(importPath))
314+
coverVar := fmt.Sprintf("goCover_%x_", sum[:6])
315+
coverageCfg = workDir + "pkgcfg.txt"
316+
coverOut, err := instrumentForCoverage(goenv, importPath, packageName, coverIn, coverVar, coverMode, coverOut, workDir, relCoverPath, srcPathMapping)
317+
if err != nil {
318+
return err
319+
}
320+
goSrcs = append(goSrcs, coverOut[0])
295321
}
296322

297323
// If we have cgo, generate separate C and go files, and compile the
@@ -389,7 +415,7 @@ func compileArchive(
389415
}
390416

391417
// Compile the filtered .go files.
392-
if err := compileGo(goenv, goSrcs, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath, gcFlags, pgoprofile, outLinkObj, outInterfacePath); err != nil {
418+
if err := compileGo(goenv, goSrcs, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath, gcFlags, pgoprofile, outLinkObj, outInterfacePath, coverageCfg); err != nil {
393419
return err
394420
}
395421

@@ -465,6 +491,7 @@ func checkImportsAndBuildCfg(goenv *env, importPath string, srcs archiveSrcs, de
465491
return "", errors.New("coverage requested but coverdata dependency not provided")
466492
}
467493
imports[coverdataPath] = coverdata
494+
imports["runtime/coverage"] = nil
468495
}
469496

470497
// Build an importcfg file for the compiler.
@@ -475,7 +502,7 @@ func checkImportsAndBuildCfg(goenv *env, importPath string, srcs archiveSrcs, de
475502
return importcfgPath, nil
476503
}
477504

478-
func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath string, gcFlags []string, pgoprofile, outLinkobjPath, outInterfacePath string) error {
505+
func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath string, gcFlags []string, pgoprofile, outLinkobjPath, outInterfacePath, coverageCfg string) error {
479506
args := goenv.goTool("compile")
480507
args = append(args, "-p", packagePath, "-importcfg", importcfgPath, "-pack")
481508
if embedcfgPath != "" {
@@ -490,6 +517,9 @@ func compileGo(goenv *env, srcs []string, packagePath, importcfgPath, embedcfgPa
490517
if pgoprofile != "" {
491518
args = append(args, "-pgoprofile", pgoprofile)
492519
}
520+
if coverageCfg != "" {
521+
args = append(args, "-coveragecfg", coverageCfg)
522+
}
493523
args = append(args, gcFlags...)
494524
args = append(args, "-o", outInterfacePath)
495525
args = append(args, "-linkobj", outLinkobjPath)

go/tools/builders/cover.go

Lines changed: 102 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,31 +16,120 @@ package main
1616

1717
import (
1818
"bytes"
19+
"encoding/json"
1920
"fmt"
21+
"os"
22+
"path/filepath"
23+
"strings"
2024
"go/parser"
2125
"go/token"
22-
"io/ioutil"
23-
"os"
2426
"strconv"
2527
)
2628

29+
const writeFileMode = 0o666
30+
2731
// instrumentForCoverage runs "go tool cover" on a source file to produce
2832
// a coverage-instrumented version of the file. It also registers the file
2933
// with the coverdata package.
30-
func instrumentForCoverage(goenv *env, srcPath, srcName, coverVar, mode, outPath string) error {
31-
goargs := goenv.goTool("cover", "-var", coverVar, "-mode", mode, "-o", outPath, srcPath)
34+
func instrumentForCoverage(
35+
goenv *env,
36+
importPath string,
37+
pkgName string,
38+
infiles []string,
39+
coverVar string,
40+
coverMode string,
41+
outfiles []string,
42+
workDir string,
43+
relCoverPath map[string]string,
44+
srcPathMapping map[string]string,
45+
) ([]string, error) {
46+
// This implementation follows the go toolchain's setup of the pkgcfg file
47+
// https://github.com/golang/go/blob/go1.24.5/src/cmd/go/internal/work/exec.go#L1954
48+
pkgcfg := workDir + "pkgcfg.txt"
49+
covoutputs := workDir + "coveroutfiles.txt"
50+
odir := filepath.Dir(outfiles[0])
51+
cv := filepath.Join(odir, "covervars.go")
52+
outputFiles := append([]string{cv}, outfiles...)
53+
54+
pcfg := coverPkgConfig{
55+
PkgPath: importPath,
56+
PkgName: pkgName,
57+
Granularity: "perblock",
58+
OutConfig: pkgcfg,
59+
Local: false,
60+
}
61+
data, err := json.Marshal(pcfg)
62+
if err != nil {
63+
return nil, err
64+
}
65+
data = append(data, '\n')
66+
if err := os.WriteFile(pkgcfg, data, writeFileMode); err != nil {
67+
return nil, err
68+
}
69+
var sb strings.Builder
70+
for i := range outputFiles {
71+
fmt.Fprintf(&sb, "%s\n", outputFiles[i])
72+
}
73+
if err := os.WriteFile(covoutputs, []byte(sb.String()), writeFileMode); err != nil {
74+
return nil, err
75+
}
76+
77+
goargs := goenv.goTool("cover", "-pkgcfg", pkgcfg, "-var", coverVar, "-mode", coverMode, "-outfilelist", covoutputs)
78+
goargs = append(goargs, infiles...)
3279
if err := goenv.runCommand(goargs); err != nil {
33-
return err
80+
return nil, err
3481
}
3582

36-
return registerCoverage(outPath, coverVar, srcName)
83+
for i, outfile := range outfiles {
84+
srcName := relCoverPath[infiles[i]]
85+
importPathFile := srcPathMapping[srcName]
86+
// Augment coverage source files to store a mapping of <importpath>/<filename> -> <execroot_relative_path>
87+
// as this information is only known during compilation but is required when the rules_go generated
88+
// test main exits and go coverage files are converted to lcov format.
89+
if err := registerCoverage(outfile, importPathFile, srcName); err != nil {
90+
return nil, err
91+
}
92+
}
93+
return outputFiles, nil
94+
}
95+
96+
// coverPkgConfig matches https://cs.opensource.google/go/go/+/refs/tags/go1.24.4:src/cmd/internal/cov/covcmd/cmddefs.go;l=18
97+
type coverPkgConfig struct {
98+
// File into which cmd/cover should emit summary info
99+
// when instrumentation is complete.
100+
OutConfig string
101+
102+
// Import path for the package being instrumented.
103+
PkgPath string
104+
105+
// Package name.
106+
PkgName string
107+
108+
// Instrumentation granularity: one of "perfunc" or "perblock" (default)
109+
Granularity string
110+
111+
// Module path for this package (empty if no go.mod in use)
112+
ModulePath string
113+
114+
// Local mode indicates we're doing a coverage build or test of a
115+
// package selected via local import path, e.g. "./..." or
116+
// "./foo/bar" as opposed to a non-relative import path. See the
117+
// corresponding field in cmd/go's PackageInternal struct for more
118+
// info.
119+
Local bool
120+
121+
// EmitMetaFile if non-empty is the path to which the cover tool should
122+
// directly emit a coverage meta-data file for the package, if the
123+
// package has any functions in it. The go command will pass in a value
124+
// here if we've been asked to run "go test -cover" on a package that
125+
// doesn't have any *_test.go files.
126+
EmitMetaFile string
37127
}
38128

39129
// registerCoverage modifies coverSrcFilename, the output file from go tool cover.
40-
// It adds a call to coverdata.RegisterCoverage, which ensures the coverage
41-
// data from each file is reported. The name by which the file is registered
42-
// need not match its original name (it may use the importpath).
43-
func registerCoverage(coverSrcFilename, varName, srcName string) error {
130+
// It adds a call to coverdata.RegisterSrcPathMapping, which ensures that rules_go
131+
// can produce lcov files with exec root relative file paths.
132+
func registerCoverage(coverSrcFilename, importPathFile, srcName string) error {
44133
coverSrc, err := os.ReadFile(coverSrcFilename)
45134
if err != nil {
46135
return fmt.Errorf("instrumentForCoverage: reading instrumented source: %w", err)
@@ -97,13 +186,10 @@ func registerCoverage(coverSrcFilename, varName, srcName string) error {
97186
var buf = bytes.NewBuffer(editor.Bytes())
98187
fmt.Fprintf(buf, `
99188
func init() {
100-
%s.RegisterFile(%q,
101-
%[3]s.Count[:],
102-
%[3]s.Pos[:],
103-
%[3]s.NumStmt[:])
189+
%s.RegisterSrcPathMapping(%q, %q)
104190
}
105-
`, coverdataName, srcName, varName)
106-
if err := ioutil.WriteFile(coverSrcFilename, buf.Bytes(), 0666); err != nil {
191+
`, coverdataName, importPathFile, srcName)
192+
if err := os.WriteFile(coverSrcFilename, buf.Bytes(), writeFileMode); err != nil {
107193
return fmt.Errorf("registerCoverage: %v", err)
108194
}
109195
return nil

go/tools/builders/cover_test.go

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,7 @@ var tests = []test{
2121
out: `package main; import "github.com/bazelbuild/rules_go/go/tools/coverdata"
2222
2323
func init() {
24-
coverdata.RegisterFile("srcName",
25-
varName.Count[:],
26-
varName.Pos[:],
27-
varName.NumStmt[:])
24+
coverdata.RegisterSrcPathMapping("some.importh/path/file.go", "src/path/file.go")
2825
}
2926
`,
3027
},
@@ -43,10 +40,7 @@ import (
4340
)
4441
4542
func init() {
46-
coverdata.RegisterFile("srcName",
47-
varName.Count[:],
48-
varName.Pos[:],
49-
varName.NumStmt[:])
43+
coverdata.RegisterSrcPathMapping("some.importh/path/file.go", "src/path/file.go")
5044
}
5145
`,
5246
},
@@ -61,10 +55,7 @@ import "github.com/bazelbuild/rules_go/go/tools/coverdata"
6155
import "github.com/bazelbuild/rules_go/go/tools/coverdata"
6256
6357
func init() {
64-
coverdata.RegisterFile("srcName",
65-
varName.Count[:],
66-
varName.Pos[:],
67-
varName.NumStmt[:])
58+
coverdata.RegisterSrcPathMapping("some.importh/path/file.go", "src/path/file.go")
6859
}
6960
`,
7061
},
@@ -79,10 +70,7 @@ import _ "github.com/bazelbuild/rules_go/go/tools/coverdata"
7970
import coverdata "github.com/bazelbuild/rules_go/go/tools/coverdata"
8071
8172
func init() {
82-
coverdata.RegisterFile("srcName",
83-
varName.Count[:],
84-
varName.Pos[:],
85-
varName.NumStmt[:])
73+
coverdata.RegisterSrcPathMapping("some.importh/path/file.go", "src/path/file.go")
8674
}
8775
`,
8876
},
@@ -97,10 +85,7 @@ import cover0 "github.com/bazelbuild/rules_go/go/tools/coverdata"
9785
import cover0 "github.com/bazelbuild/rules_go/go/tools/coverdata"
9886
9987
func init() {
100-
cover0.RegisterFile("srcName",
101-
varName.Count[:],
102-
varName.Pos[:],
103-
varName.NumStmt[:])
88+
cover0.RegisterSrcPathMapping("some.importh/path/file.go", "src/path/file.go")
10489
}
10590
`,
10691
},
@@ -113,7 +98,7 @@ func TestRegisterCoverage(t *testing.T) {
11398
t.Errorf("writing input file: %v", err)
11499
return
115100
}
116-
err := registerCoverage(filename, "varName", "srcName")
101+
err := registerCoverage(filename, "some.importh/path/file.go", "src/path/file.go")
117102
if err != nil {
118103
t.Errorf("%q: %+v", test.name, err)
119104
continue

0 commit comments

Comments
 (0)