golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
121.15k stars 17.37k forks source link

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

Open smoyer64 opened 5 years ago

smoyer64 commented 5 years ago

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.

rsc commented 4 years ago

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 commented 4 years ago

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.

smoyer64 commented 4 years ago

@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.