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: test coverage output twice #32717

Open
dnephin opened this issue Jun 20, 2019 · 4 comments

Comments

Projects
None yet
5 participants
@dnephin
Copy link
Contributor

commented Jun 20, 2019

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

$ go version
go version go1.12.6 linux/amd64

What did you do?

$ go test -json -cover ./...

What did you expect to see?

Coverage statements output only once, or one of the events identified differently from the other.

{"Time":"2019-06-20T18:51:29.178821431-04:00","Action":"output","Package":"gotest.tools/assert/cmp","Output":"ok  \tgotest.tools/assert/cmp\t(cached)\tcoverage: 91.1% of statements\n"}
{"Time":"2019-06-20T18:51:29.183457861-04:00","Action":"output","Package":"gotest.tools/assert","Output":"ok  \tgotest.tools/assert\t(cached)\tcoverage: 85.2% of statements\n"}

What did you see instead?

Coverage statements output twice.

{"Time":"2019-06-20T18:51:29.178811583-04:00","Action":"output","Package":"gotest.tools/assert/cmp","Output":"coverage: 91.1% of statements\n"}
{"Time":"2019-06-20T18:51:29.178821431-04:00","Action":"output","Package":"gotest.tools/assert/cmp","Output":"ok  \tgotest.tools/assert/cmp\t(cached)\tcoverage: 91.1% of statements\n"}
{"Time":"2019-06-20T18:51:29.183452652-04:00","Action":"output","Package":"gotest.tools/assert","Output":"coverage: 85.2% of statements\n"}
{"Time":"2019-06-20T18:51:29.183457861-04:00","Action":"output","Package":"gotest.tools/assert","Output":"ok  \tgotest.tools/assert\t(cached)\tcoverage: 85.2% of statements\n"}

Related to #23036, if one of the events could be identified differently the duplicate output would be easy to filter out.

I noticed that go test -v has the same behaviour, but i'm not sure why.

@dnephin dnephin changed the title cmd/test2json: coverage output twice cmd/test: coverage output twice Jun 21, 2019

@agnivade agnivade changed the title cmd/test: coverage output twice cmd/go: test coverage output twice Jun 24, 2019

@agnivade

This comment has been minimized.

Copy link
Member

commented Jun 24, 2019

@dnephin dnephin changed the title cmd/go: test coverage output twice cmd/test: test coverage output twice Jun 24, 2019

@dnephin dnephin changed the title cmd/test: test coverage output twice cmd/go: test coverage output twice Jun 24, 2019

@andybons andybons added this to the Unplanned milestone Jun 25, 2019

@bcmills bcmills added the help wanted label Jun 26, 2019

@joshuastrauss

This comment has been minimized.

Copy link

commented Jul 4, 2019

go test fmt -cover -v -count=1

 ...
 --- PASS: ExampleStringer (0.00s)
 PASS
 coverage: 95.1% of statements
 ok  	fmt	0.065s	coverage: 95.1% of statements

go test fmt -cover -count=1

 ok  	fmt	0.061s	coverage: 95.1% of statements

go test -cover -count=1

 PASS
 coverage: 95.1% of statements
 ok  	fmt	0.062s

go test -cover -count=1 -v

 ...
 --- PASS: ExampleStringer (0.00s)
 PASS
 coverage: 95.1% of statements
 ok  	fmt	0.064s
@joshuastrauss

This comment has been minimized.

Copy link

commented Jul 6, 2019

It seems like the appropriate fix in the code base should be to account for the first condition listed in my previous comment. This is the only condition I can find that prints the coverage output line as well as including it in the summary.

cmd/go/internal/test/test.go

 func coveragePercentage(out []byte) string {
     // Prevent the coverage output from being displayed in the summary line when invoked
     // with verbose mode and package args != 0 because the output includes the report 
     if !testCover || (len(pkgArgs) != 0 && testV) {
	return ""
     }

I added the additional condition to this function and it appears to resolve the issue.

@joshuastrauss

This comment has been minimized.

Copy link

commented Jul 6, 2019

I still need to contribute a test and ensure that this change won't affect the code in the linked ticket for the gotestsum repo. I intend to do this within the next few days as I'm currently obligated to address a previous issue I worked on.

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.