Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Coverage display is misinterpreting the coverprofile file #1683

Closed
kentquirk opened this issue May 18, 2018 · 7 comments · Fixed by #1691
Closed

Coverage display is misinterpreting the coverprofile file #1683

kentquirk opened this issue May 18, 2018 · 7 comments · Fixed by #1691
Milestone

Comments

@kentquirk
Copy link
Contributor

I toggled the code coverage view for a package I was working on, and I quickly saw that some of my heavily-tested code was marked as untested.

I used go tool cover -html=/name/of/the/cover/profile to show the "official" html version of the same coverage file used by the vscode-go plugin and it showed the correct result.

Looking at the code here the test for whether code was covered is checking to see if fileRange[7] is exactly 1.

The comment in the code says:

// go test coverageprofile generates output:
//    filename:StartLine.StartColumn,EndLine.EndColumn Hits IsCovered

But in the comments in the Go profiler source, it says:

the fields are: name.go:line.column,line.column numberOfStatements count

In other words, the "count" value (the 8th value at index 7) is not a boolean, it is the number of times that code has been executed.

Consequently, code that has been executed many times is instead being marked as not covered.

I believe that the code should be changed from:

			// If is Covered
			if (parseInt(fileRange[7]) === 1) {
				coverage.coveredRange.push({ range });
			}
			// Not Covered
			else {
				coverage.uncoveredRange.push({ range });
			}

to:

			// If is Covered
			if (parseInt(fileRange[7]) > 0) {
				coverage.coveredRange.push({ range });
			}
			// Not Covered
			else {
				coverage.uncoveredRange.push({ range });
			}

I don't have time at the moment to figure out how to build and test a modification to turn this into a PR but hopefully this is enough that someone internal can easily take it on.

@kentquirk
Copy link
Contributor Author

FWIW, I was able to modify ~/.vscode/extensions/ms-vscode.go-0.6.80/out/src/goCover.js to make the corresponding change, and it works correctly now on my machine.

@ramya-rao-a
Copy link
Contributor

What version of Go are you using?
I wonder if this is a change in the way the coverage file is created in the latest version of Go.

I am using Go 1.9.2 on my Mac. I have tests for code that gets run multiple times too. But the the last number is always 1.

@kentquirk
Copy link
Contributor Author

kentquirk commented May 22, 2018

On my mac, I'm using go version go1.10.1 darwin/amd64. I thought I had linked to the golang source, but apparently I failed to paste that link. Here it is:

https://github.com/golang/go/blob/master/src/cmd/cover/profile.go

It doesn't appear to have changed lately. Note these lines:

https://github.com/golang/go/blob/master/src/cmd/cover/profile.go#L29
https://github.com/golang/go/blob/master/src/cmd/cover/profile.go#L56

Also, please note that my proposed change is equivalent to the current behavior if the value is only a boolean (either 1 or 0), but it continues to work if the value (as documented in the source) is actually a count.

It really has to be a count to explain the behavior of the go tool cover -html flag, which colors the HTML differently based on how many times the code has been run.

@ramya-rao-a
Copy link
Contributor

Thanks for the links! I'll make the necessary changes which should be out in the next update which should be in the next few days.

@ramya-rao-a
Copy link
Contributor

I don't have time at the moment to figure out how to build and test a modification to turn this into a PR but hopefully this is enough that someone internal can easily take it on.

Unfortunately the test coverage feature doesnt have much test coverage itself :(
So, you can just send a PR online on Github for the one line change if you are up for it.

I would prefer if you get the credit for this fix as you found the issue, root cause and the fix it self :)

@kentquirk
Copy link
Contributor Author

Thank you! I'll try to do that in the next couple of days.

Thank you so much for your work on this tool. It is really appreciated.

kentquirk added a commit to kentquirk/vscode-go that referenced this issue May 26, 2018
ramya-rao-a pushed a commit that referenced this issue Jun 1, 2018
* Fixes #1683; column 8 is the coverage count, not a boolean

* Adds color/pattern options for coverage decorators. Fixes #1302

* Add missing semicolon

* Fix typo in some svg files.

* trivial change to trigger CI again

* Remove unused isWholeLine

* Support the complex types in the schema
@ramya-rao-a
Copy link
Contributor

This bug fix is now out in the latest update to the Go extension (0.6.81)

@ramya-rao-a ramya-rao-a added this to the 0.6.81 milestone Jun 4, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants