cmd/cover: regression in 1.20, cover tool no longer includes packages without tests

[abraithwaite]

What version of Go are you using (go version)?

11:55:14 $ go version
go version go1.20.1 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

11:56:28 $ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/abraithwaite/Library/Caches/go-build"
GOENV="/Users/abraithwaite/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/abraithwaite/dev/golang/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/abraithwaite/dev/golang"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/opt/homebrew/Cellar/go/1.20.1/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.20.1/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20.1"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/abraithwaite/dev/github.com/runreveal/runreveal/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/x5/_x8ny1d57k717yy0m92c02zh0000gn/T/go-build1870615753=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I ran our unit tests under both go1.19 and go1.20 and observed different coverage results.

To calculate coverage, we have this makefile testing target:

test:
    $(GO) test -vet=off -tags='$(GOTAGS)' $(GOTESTFLAGS) -coverpkg="./..." -coverprofile=.coverprofile ./...
    grep -v 'cmd' < .coverprofile > .covprof && mv .covprof .coverprofile
    $(GO) tool cover -func=.coverprofile

What did you expect to see?

I expected the coverage reporting to stay the same between releases, assuming no code changes.

What did you see instead?

The reported coverage falsely increased with go1.20 because it's no longer tracking coverage for packages that are missing tests.

As a workaround, I've put TODO tests in every package missing tests, but can't count on that manual practice to catch packages not being covered that I want to include in the coverage report.

[thanm]

Thanks for the report; I will take a look.

[thanm]

I took a look at this issue.

What's happening here has to do with how the coverage machinery is handling "-coverpkg", specifically regarding imports.

You haven't supplied any specific code examples, but I will assume that this example will do for the discussion:

https://go.dev/play/p/253gWw8ItV5

Code from the link above:

-- go.mod --
module bug.example

go 1.20
-- a/a.go --
package a

func A() int {
	return 42
}
-- a/a_test.go --
package a

import "testing"

func TestPackageCoverageInA(t *testing.T) {
	A()
}
-- b/b.go --
package b

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

import (
	"bug.example/a"
	"bug.example/b"
)

func main() {
	println(a.A() + b.B())
}

We have three packages: "a", "b" and "main", where main imports "a" and "b" but neither "a" nor "b" imports anything. If you run this with Go 1.20 at the top level (where the "go.mod" is) you get:

$ go test -count=1 -coverpkg=./...  -cover ./...
ok  	bug.example/a	0.018s	coverage: 100.0% of statements in ./...
?   	bug.example/b	[no test files]
ok  	bug.example/main	0.033s	coverage: 100.0% of statements in ./...
$

whereas in Go 1.19 you would get

$ go test -count=1 -coverpkg=./...  -cover ./...
ok  	bug.example/a	0.017s	coverage: 33.3% of statements in ./...
?   	bug.example/b	[no test files]
ok  	bug.example/main	0.028s	coverage: 100.0% of statements in ./...
$

Some background on how "go test" works: when you kick off a run of "go test -cover" in package "a", the Go command builds the test binary by combining the code from the package of interest (package 'a' in this case) with a test harness (`_testmain.go') that includes the test code, the testing package, and a synthetically generated "main" routine.

In Go 1.19 and prior versions, when given a command like

go test -coverpkg=./... -cover ./...

When building the test binary for "a", the Go command would construct a _testmain.go file that did a force import of all the packages matching the "./..." pattern, e.g.

// Code generated by 'go test'. DO NOT EDIT.
package main

import (
	"os"
	"testing"
	"testing/internal/testdeps"
	_test "bug.example/a"
	_cover0 "bug.example/b"
	_cover1 "bug.example/main"
	_cover2 "bug.example/a"
)
...

and would register the coverage counter variables for all of these packages with the testing runtime.

Forcing imports of "b" and "main" into the package "a" test binary is strange, since package "a" has no imports, and in particular importing "main" into "a" is especially weird given that normally it is the other way around.

The forced imports does provide a mechanism for computing the correct result (here 33% of all statements in "./...") for package "a", but it also creates lots of confusion for users, who are surprise when various init functions in the "./..." packages that take actions that perturb the semantics of the run when testing "a".

Here are some of the bugs that users have filed regarding this quirk:

#21283
#23883
#23910
#27336
#46212

There are others as well (variants of this problem have been reported as bugs by Go users internal to Google). For this reason, in the coverage redesign rolled out in Go 1.20 we did away with the forced import mechanism.

Getting back to the bug at hand: you are quite correct however that this is a regression, we should be reporting the 33% number for package "a" and not 100%.

Rather than going back to forced imports, I believe that the best way to handle this is to have the Go command delay reporting of coverage numbers in this case (when "-coverpkg" is used). Internally the new implementation collects raw profiles for each tested packed ("a" and "main") into a separate directory, so if we process the directories in combinatoin at the end of all test runs it should be possible to get the right result.

In the mean time, as a workaround you can restore the 1.19 behavior by setting adding this to your environment:

export GOEXPERIMENT=nocoverageredesign

This will turn off the new implementation and revert back to the 1.19 scheme. Let me know if this works for you until the bug can be fixed.

[onsi]

thanks for the clear explanation @thanm - I was about to submit a related issue but came across this and figured I'd provide an additional data point. I've put together a reproducer here (which you can clone and run make on) for what I'm seeing but tl;dr I have the following setup:

---A/A.go---
package A
func A() bool { return true }

---B/B.go---
package B
func B() bool { return true }
   
---tests/tests.go---
package tests

import (
	"testing"
	"github.com/onsi/test-coverage/A"
)

func TestA(t *testing.T) {
	A.A()
}

when I run:

go test  -coverpkg=./... ./tests

I always get 100% coverage on both 1.19 and 1.20

When I run:

go test  -coverpkg=./... ./...

I get 50% coverage on 1.19 and 100% coverage on 1.20

That 100% coverage on 1.20 is this issue. But the fact that 1.19 behaves differently for ./tests vs ./... (which is kind of reported here but I don't see the root issue being addressed there) is confusing - particularly in light of your explanation. it seems that tests is built differently when invoked as single package than when invoked as ./...

I'm primarily sharing this here to call out that there is an opportunity in 1.20 not simply to fix the regression that's been identified but also to improve on the behavior in 1.19. I'm also trying to avoid opening a near-duplicate issue but would be happy to do so if y'all would prefer.

[thanm]

Thanks for that addition @onsi .

New issue: probably not needed, but up to you.

Regarding fixing things in 1.19, it would be nice, but in general the Golang policy on back-porting fixes is fairly conservative/restrictive. Unless a change is to fix a compiler error or runtime crash or some other serious problem (where there are no workarounds), the release team tends to be discourage this sort of back-port. We'll see.

[onsi]

oh for sure - i wouldn't backport it to 1.19 either. just noting that previous behavior wasn't quite working here either.

[gopherbot]

Change https://go.dev/cl/495452 mentions this issue: cmd/go: fix percent covered problems with -coverpkg

[gopherbot]

Change https://go.dev/cl/495446 mentions this issue: cmd/cover: add new "emit meta file" mode for packages without tests

[gopherbot]

Change https://go.dev/cl/495447 mentions this issue: cmd/go: improve handling of no-test packages for coverage

[thanm]

An update on this issue:

I have a CL stack (rooted at CL 495452) that should resolve the problem.

There is one area where the coverage percentage numbers are going to be slightly different for the "go test -coverpkg=./..." case, which has to do with how init functions are handled.

In the example below, we have a top level dir containing five package subdirectories, "a", "b", "main", "n", and "x". The first three packages have tests and have a user-written init function. If you count up the total number of statements, there are a total of 9 (2 in "a", 2 in "b", 1 in "x", and 4 in "main").

-- go.mod --
module M

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

func init() {
	println("package 'a' init: launch the missiles!")
}

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

import "testing"

func TestA(t *testing.T) {
	if AFunc() != 42 {
		t.Fatalf("bad!")
	}
}
-- b/b.go --
package b

func init() {
	println("package 'b' init: release the kraken")
}

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

import "testing"

func TestB(t *testing.T) {
	if BFunc() != -42 {
		t.Fatalf("bad!")
	}
}
-- main/main.go --
package main

import (
	"M/a"
	"M/b"
)

func MFunc() string {
	return "42"
}

func M2Func() int {
	return a.AFunc() + b.BFunc()
}

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

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

import "testing"

func TestMain(t *testing.T) {
	if MFunc() != "42" {
		t.Fatalf("bad!")
	}
	if M2Func() != 0 {
		t.Fatalf("also bad!")
	}
}
-- n/n.go --
package n

type N int
-- x/x.go --
package x

func XFunc() int {
	return 2 * 2
}

Here is the output from a test run with my fix:

$ go test -coverprofile=tip.cov  -count=1 -coverpkg=./... ./...  ; go tool cover -func=tip.cov
?   	M/n	[no test files]
ok  	M/a	0.028s	coverage: 22.2% of statements in ./...
ok  	M/b	0.015s	coverage: 22.2% of statements in ./...
ok  	M/main	0.040s	coverage: 77.8% of statements in ./...
	M/x		coverage: 0.0% of statements
M/a/a.go:3:		init		100.0%
M/a/a.go:7:		AFunc		100.0%
M/b/b.go:3:		init		100.0%
M/b/b.go:7:		BFunc		100.0%
M/main/main.go:8:	MFunc		100.0%
M/main/main.go:12:	M2Func		100.0%
M/main/main.go:16:	init		100.0%
M/main/main.go:20:	main		0.0%
M/x/x.go:3:		XFunc		0.0%
total:			(statements)	77.8%
$

Note the coverage percentage for package "a": 22%, or 2 out of 9 statements total. With Go 1.19, the coverage number is 44%, or 4 out of 9 (this is due to the artifact of force-importing all covered packages into testmain for each test).

Also worth noting is that for packages like "x", which have statements but no tests, we don't link or run a test binary for the package but we do report a coverage percentage for them (this is different from a previous attempt in CL 115095 (checked in and then rreverted).

[abraithwaite]

I think that's totally reasonable. Might be confusing for some, but the results I value most are the total coverages after all the tests have ran since I often have code exercising other packages in opaque and distant ways.

Looking great! Thanks so much for taking a look at this @thanm! Really appreciate your work. 💯

[Nov1kov]

A dirty workaround is to add an empty file with the name *_test.go to each package that is not included in coverage.