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
Open

cmd/go: test coverage output twice #32717

dnephin opened this issue Jun 20, 2019 · 4 comments
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dnephin
Copy link
Contributor

dnephin 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
Copy link
Contributor

@bcmills

@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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 25, 2019
@andybons andybons added this to the Unplanned milestone Jun 25, 2019
@joshuastrauss
Copy link

joshuastrauss 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
Copy link

joshuastrauss 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
Copy link

joshuastrauss 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
Labels
help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants