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 <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>

diff --git a/src/cmd/covdata/tool_test.go b/src/cmd/covdata/tool_test.go
index 9396266776b57ab4899816d8b3f3cdcd553461dd..42334eae9499be7b4924d7b72304ee198894e6b8 100644 (file)
--- 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 989c109a79ab6d9734480d13b66ea65229bde6f9..f4f225ef20859cfb02fa774f303d2d1dd54266f8 100644 (file)
--- 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 4748a85f5ef32124a8d779aad0a330d1f2970aba..ba382639e9cdf2e82d5f500afa541e2b456fc1bf 100644 (file)
--- 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 (file)
index 0000000..3a2f3c3
--- /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 0ccb2d27b0c831bd97427443666485afcb329334..3839e4437f8d7aa56500e0dd004fec60056d27b6 100644 (file)
--- 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
 }