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

proposal: cmd/go: coverpkg should default to the package list #32939

Open
smoyer64 opened this issue Jul 4, 2019 · 3 comments
Open

proposal: cmd/go: coverpkg should default to the package list #32939

smoyer64 opened this issue Jul 4, 2019 · 3 comments
Milestone

Comments

@smoyer64
Copy link

@smoyer64 smoyer64 commented Jul 4, 2019

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

go version go1.12.6 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

GOARCH="amd64"
GOOS="linux"

What did you do?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/smoyer1/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you expect to see?

When performing "black-box" testing (where the test package is different from the code-under-test), running go test -coverprofile=coverage.out ./... resulted in the tests being run but 0% coverage.

What did you see instead?

The tests run to completion and proper coverage reported. Running the following command works as expected:

go test -coverprofile=coverage.out -coverpkg=./... ./...

This seems a bit redundant but is sufficiently generalized to run both white-box and black-box tests on our CI/CD system. I originally discovered this trying to run only the black-box tests which requires the following command:

go test -coverprofile=coverage.out -coverpkg ./pkg/... ./test/...

In this case, it make perfect sense that both the test package(s) and the code-under-test packages have to be specified.

@gopherbot gopherbot added this to the Proposal milestone Jul 4, 2019
@gopherbot gopherbot added the Proposal label Jul 4, 2019
@rsc rsc changed the title proposal: cmd/test: coverpkg should default to the package list proposal: cmd/go: coverpkg should default to the package list Jul 16, 2019
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jul 16, 2019

At this point I think that's pretty unlikely. Today it defaults to each package test running with coverage of the specific package under test. If you want to do a group with group coverage, that's fine, but it's not the default current users expect and it is likely not possible to specify any other way if we did change the default. It is also much less specific about helping evaluate whether a particular package's test covers that package well.

Why would group coverage be a better default?

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Sep 16, 2019

CC @jayconrod

Personally, I would expect go test -coverprofile=[…] to default to computing coverage for all of the packages matching the input patterns.

I agree that the current default is better for evaluating whether a given package's unit-tests cover that package, but defaulting to unit-test coverage without including integration-test coverage gets the developer incentives wrong.

If users are only going to write one kind of test, I would rather they write integration tests, because those not only better represent the actual use-cases that callers are likely to care about, but also do a better job of catching mismatched assumptions across package boundaries. But since integration tests are excluded from coverage results by default, we're nudging users to write isolated unit-tests instead.

@bcmills bcmills added the GoCommand label Sep 16, 2019
@smoyer64

This comment has been minimized.

Copy link
Author

@smoyer64 smoyer64 commented Sep 17, 2019

@bcmills I'm finding that I'm using a lot more blackbox testing even for so-called "unit tests". The tests that remain in the same package as the code-under-test
is pretty much relegated to functions that are not exported but are complex enough to need some guard against bugs by regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.