Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/go: go test -cover & go test -coverprofile should always output a coverage #24570

Open
gustafj opened this issue Mar 27, 2018 · 14 comments

Comments

Projects
None yet
10 participants
@gustafj
Copy link

commented Mar 27, 2018

Description

Now in 1.10 when go test -cover supports multiple packages, I would expect it to print out a percentage for all packages (including those missing tests).
And for go test -coverprofile, I would expect all packages to be included in the calculated total.

Currently only packages that have at least one test (can be a *_test.go with only the package declaration) is included, see pkg2 below.

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

go version go1.10 linux/amd64

Does this issue reproduce with the latest release?

yes

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

Linux, amd64

What did you do?

go test ./... -cover
go test ./... -coverprofile cover.out; go tool cover -func cover.out

What did you expect to see?

?      path/to/pkg1   0.001s  coverage:   0.0% of statements [no test files]
ok     path/to/pkg2   0.019s  coverage:   0.0% of statements [no tests to run]
ok     path/to/pkg3   0.371s  coverage: 100.0% of statements
path/to/pkg1/pkg1.go:5: String 0.0%
path/to/pkg2/pkg2.go:5: String 0.0%
path/to/pkg3/pkg3.go:5: String 100.0%
total: (statements) 33.3%

What did you see instead?

?      path/to/pkg1   [no test files]
ok     path/to/pkg2   0.019s  coverage:   0.0% of statements [no tests to run]
ok     path/to/pkg3   0.371s  coverage: 100.0% of statements
path/to/pkg2/pkg2.go:5: String 0.0%
path/to/pkg3/pkg3.go:5: String 100.0%
total: (statements) 50.0%

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 27, 2018

@gopherbot

This comment has been minimized.

Copy link

commented May 29, 2018

Change https://golang.org/cl/115095 mentions this issue: cmd/go/internal/test: always output a coverage

@mvdan

This comment has been minimized.

Copy link
Member

commented May 30, 2018

@kyroy noticed that this issue is very similar to #25492. It seems to me like both should be fixed at once, as they suggest changes in opposite directions. The other issue wants to change a 0.0% with a [no statements], and this one wants to change [no test files] with 0.0%.

/cc @bcmills @egonk

@bcmills

This comment has been minimized.

Copy link
Member

commented May 30, 2018

they suggest changes in opposite directions.

#25492 is about the case where there are no statements to cover (and thus the percentage is undefined). That's materially different from the case here, where pkg1 does have a statement and it is not covered: 0.0% coverage is well-defined and correct for a package with one statement and no tests.

@mvdan

This comment has been minimized.

Copy link
Member

commented May 30, 2018

My reading of this issue is that go test -cover should always output a percentage if it succeeded. The other issue is precisely about removing a 0.0% output in favor of something else.

@bcmills

This comment has been minimized.

Copy link
Member

commented May 30, 2018

My reading of this issue is that go test -cover should always output a percentage if it succeeded.

That's the current issue title, but that behavior seems clearly wrong if there are no statements to cover: 0/0 is not a well-defined percentage.

(I guess we could output NaN%, but that seems strictly less helpful than [no statements].)

@kyroy

This comment has been minimized.

Copy link
Contributor

commented May 30, 2018

I like the idea of a consistent output that would be created by just implementing this issue.

Another approach is, as in #25492, for all statements it is true that they are covered by the tests. So one could argue for 100% which is obviously mathematically incorrect.

I am also the creator of the CL. Happy to implement a solution that results from the discussion :)

@gopherbot gopherbot closed this in ebb8a1f Jun 6, 2018

sdboyer added a commit to sdboyer/dep that referenced this issue Jul 2, 2018

ci: Ignore main file in licenseok
This should fix test failures that were being caused by golang/go#24570,
which caused test binaries to be built even when there are no test
files. The poor interaction there arose from the setting of flags in our
init() function, it seems.

sdboyer added a commit to sdboyer/dep that referenced this issue Jul 3, 2018

ci: Ignore main file in licenseok
This should fix test failures that were being caused by golang/go#24570,
which caused test binaries to be built even when there are no test
files. The poor interaction there arose from the setting of flags in our
init() function, it seems.
@gopherbot

This comment has been minimized.

Copy link

commented Jul 6, 2018

Change https://golang.org/cl/122518 mentions this issue: cmd/go: revert "output coverage report even if there are no test files"

gopherbot pushed a commit that referenced this issue Jul 9, 2018

cmd/go: revert "output coverage report even if there are no test files"
Original CL description:

    When using test -cover or -coverprofile the output for "no test files"
    is the same format as for "no tests to run".

Reverting because this CL changed cmd/go to build test binaries for
packages that have no tests, leading to extra work and confusion.

Updates #24570
Fixes #25789
Fixes #26157
Fixes #26242

Change-Id: Ibab1307d39dfaec0de9359d6d96706e3910c8efd
Reviewed-on: https://go-review.googlesource.com/122518
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2018

CL 115095 was reverted, so reopening this issue.

@agnivade

This comment has been minimized.

Copy link
Member

commented Dec 23, 2018

FYI, f7248f0 brought back the pkg{1,2,3} folders inside cmd/go/testdata/testcover that was originally reverted by 7254cfc. Those files are unused as of now.

@Nerzal

This comment has been minimized.

Copy link

commented Feb 10, 2019

Any news here?

@rabadin

This comment has been minimized.

Copy link

commented Apr 1, 2019

One can use -coverpkg=./... to get accurate results:

$ tree .
.
├── coverage.out
├── package0.go
├── package0_test.go
├── package1
│   ├── package1.go
│   └── package1_test.go
└── package2
    └── package2.go

2 directories, 6 files

Without -coverpkg=./...:

$ go test -v -coverprofile=coverage.out ./...
=== RUN   TestMin
--- PASS: TestMin (0.00s)
PASS
coverage: 100.0% of statements
ok  	github.com/rabadin/brokencoverage	0.007s	coverage: 100.0% of statements
=== RUN   TestAdd
--- PASS: TestAdd (0.00s)
PASS
coverage: 100.0% of statements
ok  	github.com/rabadin/brokencoverage/package1	0.006s	coverage: 100.0% of statements
?   	github.com/rabadin/brokencoverage/package2	[no test files]

$ go tool cover -func=coverage.out
github.com/rabadin/brokencoverage/package0.go:3:		min		100.0%
github.com/rabadin/brokencoverage/package1/package1.go:3:	add		100.0%
total:								(statements)	100.0%

With -coverpkg=./...:

$ rm coverage.out

$ go test -v -coverpkg=./... -coverprofile=coverage.out ./...
=== RUN   TestMin
--- PASS: TestMin (0.00s)
PASS
coverage: 33.3% of statements in ./...
ok  	github.com/rabadin/brokencoverage	0.006s	coverage: 33.3% of statements in ./...
=== RUN   TestAdd
--- PASS: TestAdd (0.00s)
PASS
coverage: 33.3% of statements in ./...
ok  	github.com/rabadin/brokencoverage/package1	0.006s	coverage: 33.3% of statements in ./...
?   	github.com/rabadin/brokencoverage/package2	[no test files]

$ go tool cover -func=coverage.out
github.com/rabadin/brokencoverage/package0.go:3:		min		100.0%
github.com/rabadin/brokencoverage/package1/package1.go:3:	add		100.0%
github.com/rabadin/brokencoverage/package2/package2.go:3:	mult		0.0%
total:								(statements)	66.7%
@puellanivis

This comment has been minimized.

Copy link

commented Jun 7, 2019

Using -coverpkg does not actually help if the package is never called in any test at all. -coverpkg is a cover-up that allows packages from a different package to provide coverage for this package.

So, say package a has no tests, and thus no coverage. But package b has tests, and those tests call into a. As a result, if you start using a -coverpkg that covers both packages, the tests from package b will report its coverage of package a as coverage for a.

However, if we have a package c that is never called by either a or b then that package will still be left out in the dry, even if it would match the -coverpkg given.

project$ find . -name "*_test.go"
project$ go test -coverpkg=./... -coverprofile=coverage.out ./...
?   	project	[no test files]
?   	project/sub/dir1	[no test files]
?   	project/sub/dir2	[no test files]
project$ cat coverage.out
mode: set
project$

Additionally, it is misleading to say that because b has called into a that that coverage of a counts as tests on a, because they are not unit tests of a’s functionality, and b is fundamentally not responsible for testing of a.

@rabadin

This comment has been minimized.

Copy link

commented Jun 7, 2019

@puellanivis:

Using -coverpkg does not actually help if the package is never called in any test at all.

I'm not sure this is true. Here is the code from the example above:

find . -name "*.go" -exec echo '******** {}' \; -exec cat {} \;
******** ./package2/package2.go
package package2

func mult(a int, b int) int {
	return a * b
}
******** ./package0_test.go
package brokencoverage

import "testing"

func TestMin(t *testing.T) {
	if min(3,5) != 3-5 {
		t.Errorf("min is broken")
	}
}
******** ./package1/package1_test.go
package package1

import "testing"

func TestAdd(t *testing.T) {
	if add(3,5) != 3+5 {
		t.Errorf("add is broken")
	}
}
******** ./package1/package1.go

package package1

func add(a int, b int) int {
	return a + b
}
******** ./package0.go
package brokencoverage

func min(a int, b int) int {
	return a - b
}

as you can see nothing is calling package2/package2.go:mult.

@puellanivis

This comment has been minimized.

Copy link

commented Jun 11, 2019

I don’t know exactly what is different, but even with -coverpkg=./... my project with no tests still reports no packages:

project-with-no-tests$ go test -v -coverpkg=./... -coverprofile=coverage.out ./...
?   	project-with-no-tests	[no test files]
?   	project-with-no-tests/subpackage	[no test files]
project-with-no-tests$ go tool cover -func=coverage.out
total:	(statements)	0.0%

While adding just a single _test.go with just the package name yields coverage details for a package.

Meanwhile, if I add a reference to package2.Mult in package0_test.go suddenly, with -coverpkg=./... we report that there is coverage of package2.Mult, even though it is not actually covered by any unit tests. (It would make sense to include such coverage in integration tests though, as there you are testing the system as a whole, but unit tests should never count coverage provided by any other package.)

broken-coverage$ cat package0_test.go 
package brokencoverage

import (
	"testing"
	"github.com/puellanivis/broken-coverage/package2"
)

func TestMin(t *testing.T) {
	if min(3,5) != package2.Mult(-1, 2) {
		t.Errorf("min is broken")
	}
}
broken-coverage$ go test -v -coverpkg=./... -coverprofile=coverage.out ./...
=== RUN   TestMin
--- PASS: TestMin (0.00s)
PASS
coverage: 66.7% of statements in ./...
ok  	github.com/puellanivis/broken-coverage	0.001s	coverage: 66.7% of statements in ./...
=== RUN   TestAdd
--- PASS: TestAdd (0.00s)
PASS
coverage: 33.3% of statements in ./...
ok  	github.com/puellanivis/broken-coverage/package1	0.004s	coverage: 33.3% of statements in ./...
?   	github.com/puellanivis/broken-coverage/package2	[no test files]
broken-coverage$ go tool cover -func=coverage.out
github.com/puellanivis/broken-coverage/package0.go:3:		min		100.0%
github.com/puellanivis/broken-coverage/package1/package1.go:3:	add		100.0%
github.com/puellanivis/broken-coverage/package2/package2.go:3:	Mult		100.0%
total:								(statements)	100.0%
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.