From 7973b0e50861c49c1852a545b51b7ab977135d6d Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 8 Dec 2022 10:58:49 -0500 Subject: [PATCH] cmd/{go,cover,covdata}: fix 'package main' inconsistent handling Fix a buglet in cmd/cover in how we handle package name/path for the "go build -o foo.exe *.go" and "go run *.go" cases. The go command assigns a dummy import path of "command-line-arguments" to the main package built in these cases; rather than expose this dummy to the user in coverage reports, the cover tool had a special case hack intended to rewrite such package paths to "main". The hack was too general, however, and was rewriting the import path of all packages with (p.name == "main") to an import path of "main". The hack also produced unexpected results for cases such as go test -cover foo.go foo_test.go This patch removes the hack entirely, leaving the package path for such cases as "command-line-arguments". Fixes #57169. Change-Id: Ib6071db5e3485da3b8c26e16ef57f6fa1712402c Reviewed-on: https://go-review.googlesource.com/c/go/+/456237 TryBot-Result: Gopher Robot Run-TryBot: Than McIntosh Reviewed-by: Bryan Mills --- src/cmd/covdata/tool_test.go | 35 ++++++------ src/cmd/cover/cover.go | 3 -- .../script/cover_build_cmdline_pkgs.txt | 4 +- .../script/cover_main_import_path.txt | 54 +++++++++++++++++++ src/runtime/coverage/emitdata_test.go | 9 ++-- 5 files changed, 81 insertions(+), 24 deletions(-) create mode 100644 src/cmd/go/testdata/script/cover_main_import_path.txt diff --git a/src/cmd/covdata/tool_test.go b/src/cmd/covdata/tool_test.go index 9396266776..42334eae94 100644 --- a/src/cmd/covdata/tool_test.go +++ b/src/cmd/covdata/tool_test.go @@ -111,6 +111,8 @@ func emitFile(t *testing.T, dst, src string) { } } +const mainPkgPath = "prog" + func buildProg(t *testing.T, prog string, dir string, tag string, flags []string) (string, string) { // Create subdirs. subdir := filepath.Join(dir, prog+"dir"+tag) @@ -132,7 +134,7 @@ func buildProg(t *testing.T, prog string, dir string, tag string, flags []string // Emit go.mod. mod := filepath.Join(subdir, "go.mod") - modsrc := "\nmodule prog\n\ngo 1.19\n" + modsrc := "\nmodule " + mainPkgPath + "\n\ngo 1.19\n" if err := os.WriteFile(mod, []byte(modsrc), 0666); err != nil { t.Fatal(err) } @@ -305,7 +307,7 @@ func runToolOp(t *testing.T, s state, op string, args []string) []string { func testDump(t *testing.T, s state) { // Run the dumper on the two dirs we generated. - dargs := []string{"-pkg=main", "-live", "-i=" + s.outdirs[0] + "," + s.outdirs[1]} + dargs := []string{"-pkg=" + mainPkgPath, "-live", "-i=" + s.outdirs[0] + "," + s.outdirs[1]} lines := runToolOp(t, s, "debugdump", dargs) // Sift through the output to make sure it has some key elements. @@ -319,7 +321,7 @@ func testDump(t *testing.T, s state) { }, { "main package", - regexp.MustCompile(`^Package path: main\s*$`), + regexp.MustCompile(`^Package path: ` + mainPkgPath + `\s*$`), }, { "main function", @@ -337,7 +339,7 @@ func testDump(t *testing.T, s state) { } } if !found { - t.Errorf("dump output regexp match failed for %s", testpoint.tag) + t.Errorf("dump output regexp match failed for %q", testpoint.tag) bad = true } } @@ -348,7 +350,7 @@ func testDump(t *testing.T, s state) { func testPercent(t *testing.T, s state) { // Run the dumper on the two dirs we generated. - dargs := []string{"-pkg=main", "-i=" + s.outdirs[0] + "," + s.outdirs[1]} + dargs := []string{"-pkg=" + mainPkgPath, "-i=" + s.outdirs[0] + "," + s.outdirs[1]} lines := runToolOp(t, s, "percent", dargs) // Sift through the output to make sure it has the needful. @@ -380,11 +382,12 @@ func testPercent(t *testing.T, s state) { dumplines(lines) } } + func testPkgList(t *testing.T, s state) { dargs := []string{"-i=" + s.outdirs[0] + "," + s.outdirs[1]} lines := runToolOp(t, s, "pkglist", dargs) - want := []string{"main", "prog/dep"} + want := []string{mainPkgPath, mainPkgPath + "/dep"} bad := false if len(lines) != 2 { t.Errorf("expect pkglist to return two lines") @@ -405,7 +408,7 @@ func testPkgList(t *testing.T, s state) { func testTextfmt(t *testing.T, s state) { outf := s.dir + "/" + "t.txt" - dargs := []string{"-pkg=main", "-i=" + s.outdirs[0] + "," + s.outdirs[1], + dargs := []string{"-pkg=" + mainPkgPath, "-i=" + s.outdirs[0] + "," + s.outdirs[1], "-o", outf} lines := runToolOp(t, s, "textfmt", dargs) @@ -426,7 +429,7 @@ func testTextfmt(t *testing.T, s state) { dumplines(lines[0:10]) t.Errorf("textfmt: want %s got %s", want0, lines[0]) } - want1 := "prog/prog1.go:13.14,15.2 1 1" + want1 := mainPkgPath + "/prog1.go:13.14,15.2 1 1" if lines[1] != want1 { dumplines(lines[0:10]) t.Errorf("textfmt: want %s got %s", want1, lines[1]) @@ -571,7 +574,7 @@ func testMergeSimple(t *testing.T, s state, indir1, indir2, tag string) { nonzero: true, }, } - flags := []string{"-live", "-pkg=main"} + flags := []string{"-live", "-pkg=" + mainPkgPath} runDumpChecks(t, s, outdir, flags, testpoints) } @@ -585,7 +588,7 @@ func testMergeSelect(t *testing.T, s state, indir1, indir2 string, tag string) { // based on package. ins := fmt.Sprintf("-i=%s,%s", indir1, indir2) out := fmt.Sprintf("-o=%s", outdir) - margs := []string{"-pkg=prog/dep", ins, out} + margs := []string{"-pkg=" + mainPkgPath + "/dep", ins, out} lines := runToolOp(t, s, "merge", margs) if len(lines) != 0 { t.Errorf("merge run produced %d lines of unexpected output", len(lines)) @@ -600,9 +603,9 @@ func testMergeSelect(t *testing.T, s state, indir1, indir2 string, tag string) { t.Fatalf("dump run produced no output") } want := map[string]int{ - "Package path: prog/dep": 0, - "Func: Dep1": 0, - "Func: PDep": 0, + "Package path: " + mainPkgPath + "/dep": 0, + "Func: Dep1": 0, + "Func: PDep": 0, } bad := false for _, line := range lines { @@ -696,7 +699,7 @@ func testMergeCombinePrograms(t *testing.T, s state) { }, } - flags := []string{"-live", "-pkg=main"} + flags := []string{"-live", "-pkg=" + mainPkgPath} runDumpChecks(t, s, moutdir, flags, testpoints) } @@ -717,7 +720,7 @@ func testSubtract(t *testing.T, s state) { } // Dump the files in the subtract output dir and examine the result. - dargs := []string{"-pkg=main", "-live", "-i=" + soutdir} + dargs := []string{"-pkg=" + mainPkgPath, "-live", "-i=" + soutdir} lines = runToolOp(t, s, "debugdump", dargs) if len(lines) == 0 { t.Errorf("dump run produced no output") @@ -774,7 +777,7 @@ func testIntersect(t *testing.T, s state, indir1, indir2, tag string) { } // Dump the files in the subtract output dir and examine the result. - dargs := []string{"-pkg=main", "-live", "-i=" + ioutdir} + dargs := []string{"-pkg=" + mainPkgPath, "-live", "-i=" + ioutdir} lines = runToolOp(t, s, "debugdump", dargs) if len(lines) == 0 { t.Errorf("dump run produced no output") diff --git a/src/cmd/cover/cover.go b/src/cmd/cover/cover.go index 989c109a79..f4f225ef20 100644 --- a/src/cmd/cover/cover.go +++ b/src/cmd/cover/cover.go @@ -542,9 +542,6 @@ func annotate(names []string) { if *pkgcfg != "" { pp := pkgconfig.PkgPath pn := pkgconfig.PkgName - if pn == "main" { - pp = "main" - } mp := pkgconfig.ModulePath mdb, err := encodemeta.NewCoverageMetaDataBuilder(pp, pn, mp) if err != nil { diff --git a/src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt b/src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt index 4748a85f5e..ba382639e9 100644 --- a/src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt +++ b/src/cmd/go/testdata/script/cover_build_cmdline_pkgs.txt @@ -26,7 +26,7 @@ env GOCOVERDIR=$SAVEGOCOVERDIR # Check to make sure we instrumented just the main package, not # any dependencies. go tool covdata pkglist -i=$WORK/covdata -stdout main +stdout cmd/nm ! stdout cmd/internal/goobj pkglist.txt # ... now collect a coverage profile from a Go file @@ -41,7 +41,7 @@ env GOCOVERDIR=$SAVEGOCOVERDIR # Check to make sure we instrumented just the main package. go tool covdata pkglist -i=$WORK/covdata2 -stdout main +stdout command-line-arguments ! stdout fmt -- go.mod -- diff --git a/src/cmd/go/testdata/script/cover_main_import_path.txt b/src/cmd/go/testdata/script/cover_main_import_path.txt new file mode 100644 index 0000000000..3a2f3c3ee2 --- /dev/null +++ b/src/cmd/go/testdata/script/cover_main_import_path.txt @@ -0,0 +1,54 @@ + +# This test is intended to verify that coverage reporting is consistent +# between "go test -cover" and "go build -cover" with respect to how +# the "main" package is handled. See issue 57169 for details. + +[short] skip + +# Build this program with -cover and run to collect a profile. + +go build -cover -o $WORK/prog.exe . + +# Save off old GOCOVERDIR setting +env SAVEGOCOVERDIR=$GOCOVERDIR + +mkdir $WORK/covdata +env GOCOVERDIR=$WORK/covdata +exec $WORK/prog.exe + +# Restore previous GOCOVERDIR setting +env GOCOVERDIR=$SAVEGOCOVERDIR + +# Report percent lines covered. +go tool covdata percent -i=$WORK/covdata +stdout '\s*mainwithtest\s+coverage:' +! stdout 'main\s+coverage:' + +# Go test -cover should behave the same way. +go test -cover . +stdout 'ok\s+mainwithtest\s+\S+\s+coverage:' +! stdout 'ok\s+main\s+.*' + + +-- go.mod -- +module mainwithtest + +go 1.20 +-- mymain.go -- +package main + +func main() { + println("hi mom") +} + +func Mainer() int { + return 42 +} +-- main_test.go -- +package main + +import "testing" + +func TestCoverage(t *testing.T) { + println(Mainer()) +} diff --git a/src/runtime/coverage/emitdata_test.go b/src/runtime/coverage/emitdata_test.go index 0ccb2d27b0..3839e4437f 100644 --- a/src/runtime/coverage/emitdata_test.go +++ b/src/runtime/coverage/emitdata_test.go @@ -157,7 +157,7 @@ func runHarness(t *testing.T, harnessPath string, tp string, setGoCoverDir bool, func testForSpecificFunctions(t *testing.T, dir string, want []string, avoid []string) string { args := []string{"tool", "covdata", "debugdump", - "-live", "-pkg=main", "-i=" + dir} + "-live", "-pkg=command-line-arguments", "-i=" + dir} t.Logf("running: go %v\n", args) cmd := exec.Command(testenv.GoToolPath(t), args...) b, err := cmd.CombinedOutput() @@ -167,18 +167,21 @@ func testForSpecificFunctions(t *testing.T, dir string, want []string, avoid []s output := string(b) rval := "" for _, f := range want { - wf := "Func: " + f + wf := "Func: " + f + "\n" if strings.Contains(output, wf) { continue } rval += fmt.Sprintf("error: output should contain %q but does not\n", wf) } for _, f := range avoid { - wf := "Func: " + f + wf := "Func: " + f + "\n" if strings.Contains(output, wf) { rval += fmt.Sprintf("error: output should not contain %q but does\n", wf) } } + if rval != "" { + t.Logf("=-= begin output:\n" + output + "\n=-= end output\n") + } return rval } -- 2.50.0