Skip to content

Commit

Permalink
cmd/go: fix problems with coverage percentage reporting w/ -coverpkg
Browse files Browse the repository at this point in the history
This patch resolves a set of problems with "percent covered" metrics
reported when the "-coverpkg" is in effect; these bugs were introduced
in Go 1.22 with the rollout of CL 495452 and related changes.
Specifically, for runs with multiple packages selected but without
-coverpkg, "percent covered" metrics were generated for package P not
based just on P's statements but on the entire corpus of statements.

Fixes #65570.

Change-Id: I38d61886cb46ebd38d8c4313c326d671197c3568
Reviewed-on: https://go-review.googlesource.com/c/go/+/592205
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
  • Loading branch information
thanm committed Jun 14, 2024
1 parent 7196db9 commit a85b881
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 6 deletions.
17 changes: 17 additions & 0 deletions src/cmd/go/internal/load/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,22 @@ func (t *testFuncs) Covered() string {
return " in " + strings.Join(t.Cover.Paths, ", ")
}

func (t *testFuncs) CoverSelectedPackages() string {
if t.Cover == nil || t.Cover.Paths == nil {
return `[]string{"` + t.Package.ImportPath + `"}`
}
var sb strings.Builder
fmt.Fprintf(&sb, "[]string{")
for k, p := range t.Cover.Pkgs {
if k != 0 {
sb.WriteString(", ")
}
fmt.Fprintf(&sb, `"%s"`, p.ImportPath)
}
sb.WriteString("}")
return sb.String()
}

// Tested returns the name of the package being tested.
func (t *testFuncs) Tested() string {
return t.Package.Name
Expand Down Expand Up @@ -950,6 +966,7 @@ func init() {
{{if .Cover}}
testdeps.CoverMode = {{printf "%q" .Cover.Mode}}
testdeps.Covered = {{printf "%q" .Covered}}
testdeps.CoverSelectedPackages = {{printf "%s" .CoverSelectedPackages}}
testdeps.CoverSnapshotFunc = cfile.Snapshot
testdeps.CoverProcessTestDirFunc = cfile.ProcessCoverTestDir
testdeps.CoverMarkProfileEmittedFunc = cfile.MarkProfileEmitted
Expand Down
63 changes: 63 additions & 0 deletions src/cmd/go/testdata/script/cover_single_vs_multiple.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# Without -coverpkg, we should get the same value for a given
# package regardless of how many other packages are selected
# (see issue 65570).

[short] skip

go test -count=1 -cover ./a ./b ./main
stdout '^ok\s+M/main\s+\S+\s+coverage: 75.0% of statements'
go test -count=1 -cover ./main
stdout '^ok\s+M/main\s+\S+\s+coverage: 75.0% of statements'

-- go.mod --
module M

go 1.21
-- a/a.go --
package a

func AFunc() int {
return 42
}
-- b/b.go --
package b

func BFunc() int {
return -42
}
-- main/main.go --
package main

import (
"M/a"
)

func MFunc() string {
return "42"
}

func M2Func() int {
return a.AFunc()
}

func init() {
println("package 'main' init")
}

func main() {
println(a.AFunc())
}
-- main/main_test.go --
package main

import "testing"

func TestMain(t *testing.T) {
if MFunc() != "42" {
t.Fatalf("bad!")
}
if M2Func() != 42 {
t.Fatalf("also bad!")
}
}

4 changes: 2 additions & 2 deletions src/internal/coverage/cfile/testsupport.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
// testmain code when "go test -cover" is in effect. It is not
// intended to be used other than internally by the Go command's
// generated code.
func ProcessCoverTestDir(dir string, cfile string, cm string, cpkg string, w io.Writer) error {
func ProcessCoverTestDir(dir string, cfile string, cm string, cpkg string, w io.Writer, selpkgs []string) error {
cmode := coverage.ParseCounterMode(cm)
if cmode == coverage.CtrModeInvalid {
return fmt.Errorf("invalid counter mode %q", cm)
Expand Down Expand Up @@ -103,7 +103,7 @@ func ProcessCoverTestDir(dir string, cfile string, cm string, cpkg string, w io.
}

// Emit percent.
if err := ts.cf.EmitPercent(w, nil, cpkg, true, true); err != nil {
if err := ts.cf.EmitPercent(w, selpkgs, cpkg, true, true); err != nil {
return err
}

Expand Down
4 changes: 2 additions & 2 deletions src/internal/coverage/cfile/ts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestTestSupport(t *testing.T) {
textfile := filepath.Join(t.TempDir(), "file.txt")
var sb strings.Builder
err := ProcessCoverTestDir(tgcd, textfile,
testing.CoverMode(), "", &sb)
testing.CoverMode(), "", &sb, nil)
if err != nil {
t.Fatalf("bad: %v", err)
}
Expand Down Expand Up @@ -168,7 +168,7 @@ func TestAuxMetaDataFiles(t *testing.T) {
var sb strings.Builder
textfile := filepath.Join(td, "file2.txt")
err = ProcessCoverTestDir(tgcd, textfile,
testing.CoverMode(), "", &sb)
testing.CoverMode(), "", &sb, nil)
if err != nil {
t.Fatalf("bad: %v", err)
}
Expand Down
5 changes: 3 additions & 2 deletions src/testing/internal/testdeps/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,15 @@ func (TestDeps) SnapshotCoverage() {

var CoverMode string
var Covered string
var CoverSelectedPackages []string

// These variables below are set at runtime (via code in testmain) to point
// to the equivalent functions in package internal/coverage/cfile; doing
// things this way allows us to have tests import internal/coverage/cfile
// only when -cover is in effect (as opposed to importing for all tests).
var (
CoverSnapshotFunc func() float64
CoverProcessTestDirFunc func(dir string, cfile string, cm string, cpkg string, w io.Writer) error
CoverProcessTestDirFunc func(dir string, cfile string, cm string, cpkg string, w io.Writer, selpkgs []string) error
CoverMarkProfileEmittedFunc func(val bool)
)

Expand All @@ -232,7 +233,7 @@ func coverTearDown(coverprofile string, gocoverdir string) (string, error) {
}
CoverMarkProfileEmittedFunc(true)
cmode := CoverMode
if err := CoverProcessTestDirFunc(gocoverdir, coverprofile, cmode, Covered, os.Stdout); err != nil {
if err := CoverProcessTestDirFunc(gocoverdir, coverprofile, cmode, Covered, os.Stdout, CoverSelectedPackages); err != nil {
return "error generating coverage report", err
}
return "", nil
Expand Down

0 comments on commit a85b881

Please sign in to comment.