From b9d18ab4ae30ec1d960a31bff101c55a30874624 Mon Sep 17 00:00:00 2001 From: roulle_a Date: Fri, 28 Aug 2026 15:53:40 +0200 Subject: [PATCH] Pass absolute source paths to `go tool cover` `go tool cover` copies each source path it is given verbatim into a `//line :1:1` directive at the top of the file it generates. Since go.dev/issue/70478, released in Go 1.27, the compiler resolves a relative filename in a //line directive against the directory of the file containing the directive rather than against the current directory. Absolute filenames are unaffected. please_go passes relative paths, so from Go 1.27 onwards the package directory ends up duplicated in every position recorded for an instrumented file. Code calling runtime.Caller under `plz cover` sees paths like lib/go/logger/lib/go/logger/logger_test.go:39 instead of lib/go/logger/logger_test.go:39 which breaks tests asserting on caller information, and feeds wrong paths into coverage metadata. `plz test` is unaffected because it does not instrument. Make the paths absolute before handing them over. While here, fix an adjacent bug: filepath.Join(dir, "_covervars.cover.go\n") ran the trailing newline through Clean and swallowed it, so the covervars entry and the first source shared a line in the output file list. --- ChangeLog | 5 ++++ VERSION | 2 +- tools/please_go/cover/BUILD | 15 ++++++++++- tools/please_go/cover/cover.go | 11 +++++++- tools/please_go/cover/cover_test.go | 42 +++++++++++++++++++++++++++++ 5 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 tools/please_go/cover/cover_test.go diff --git a/ChangeLog b/ChangeLog index b1c71eb0..e55046c6 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +Version 1.32.1 +-------------- + * Pass absolute source paths to `go tool cover` so that coverage builds + report correct file positions under Go 1.27 (#372) + Version 1.32.0 -------------- * Allow passing command-line arguments to go_test targets (#368) diff --git a/VERSION b/VERSION index 359c4108..96cd6ee1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.32.0 +1.32.1 diff --git a/tools/please_go/cover/BUILD b/tools/please_go/cover/BUILD index 5d4333d7..3bf47585 100644 --- a/tools/please_go/cover/BUILD +++ b/tools/please_go/cover/BUILD @@ -2,7 +2,10 @@ subinclude("//build_defs:go") filegroup( name = "srcs", - srcs = ["cover.go"], + srcs = glob( + include = ["*.go"], + exclude = ["*_test.go"], + ), visibility = ["//tools/please_go:bootstrap"], ) @@ -12,3 +15,13 @@ go_library( visibility = ["//tools/please_go/..."], deps = ["//tools/please_go/install/toolchain"], ) + +go_test( + name = "cover_test", + srcs = ["cover_test.go"], + deps = [ + ":cover", + "///third_party/go/github.com_stretchr_testify//assert", + "///third_party/go/github.com_stretchr_testify//require", + ], +) diff --git a/tools/please_go/cover/cover.go b/tools/please_go/cover/cover.go index 156ae3c2..1a4eeaba 100644 --- a/tools/please_go/cover/cover.go +++ b/tools/please_go/cover/cover.go @@ -21,6 +21,15 @@ func WriteCoverage(goTool, coverTool, covercfg, output, pkg string, srcs []strin if err != nil { return err } + // `go tool cover` copies these into `//line` directives, and since Go 1.27 + // (go.dev/issue/70478) a relative one resolves against the file's own directory. + for i, src := range srcs { + abs, err := filepath.Abs(src) + if err != nil { + return err + } + srcs[i] = abs + } const pkgConfigFile = "pkgcfg" b, _ := json.Marshal(coverConfig{ OutConfig: covercfg, @@ -34,7 +43,7 @@ func WriteCoverage(goTool, coverTool, covercfg, output, pkg string, srcs []strin var buf bytes.Buffer // 1.21 requires a cover vars file to be written into the output file list if coverTool != "" || needs121CoverVars(goTool) { - buf.WriteString(filepath.Join(filepath.Dir(srcs[0]), "_covervars.cover.go\n")) + buf.WriteString(filepath.Join(filepath.Dir(srcs[0]), "_covervars.cover.go") + "\n") } for _, src := range srcs { buf.WriteString(strings.TrimSuffix(src, ".go") + ".cover.go\n") diff --git a/tools/please_go/cover/cover_test.go b/tools/please_go/cover/cover_test.go new file mode 100644 index 00000000..343e28fa --- /dev/null +++ b/tools/please_go/cover/cover_test.go @@ -0,0 +1,42 @@ +package cover + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestWriteCoveragePassesAbsoluteSources(t *testing.T) { + tmpDir := t.TempDir() + + pkgDir := filepath.Join(tmpDir, "src", "mypkg") + require.NoError(t, os.MkdirAll(pkgDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(pkgDir, "mypkg.go"), []byte("package mypkg\n"), 0644)) + + // A fake cover tool that records the arguments it was given. + argsFile := filepath.Join(tmpDir, "args") + coverTool := filepath.Join(tmpDir, "cover.sh") + script := "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"" + argsFile + "\"\n" + require.NoError(t, os.WriteFile(coverTool, []byte(script), 0755)) + + t.Chdir(tmpDir) + src := filepath.Join("src", "mypkg", "mypkg.go") + outfilelist := filepath.Join(tmpDir, "outfilelist") + require.NoError(t, WriteCoverage("go", coverTool, filepath.Join(tmpDir, "covcfg"), outfilelist, "example.com/mypkg", []string{src})) + + args, err := os.ReadFile(argsFile) + require.NoError(t, err) + assert.Contains(t, strings.Fields(string(args)), filepath.Join(pkgDir, "mypkg.go")) + assert.NotContains(t, strings.Fields(string(args)), src) + + list, err := os.ReadFile(outfilelist) + require.NoError(t, err) + assert.Equal(t, []string{ + filepath.Join(pkgDir, "_covervars.cover.go"), + filepath.Join(pkgDir, "mypkg.cover.go"), + }, strings.Split(strings.TrimSpace(string(list)), "\n")) +}