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/cover: html renderer shows misleading coverage #25767

Closed
mhilton opened this issue Jun 7, 2018 · 6 comments
Closed

cmd/cover: html renderer shows misleading coverage #25767

mhilton opened this issue Jun 7, 2018 · 6 comments

Comments

@mhilton
Copy link

@mhilton mhilton commented Jun 7, 2018

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

go version devel +db49f76dc5 Tue Jun 5 20:31:21 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

no, 1.10.2 works fine

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

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"

What did you do?

With the following package:
==> local/x/x.go <==

package x

import "fmt"

func X() {
	if false {
		fmt.Printf("Hello")
	}
}

==> local/x/x_test.go <==

package x_test

import (
	"testing"
	"local/x"
)

func TestX(t *testing.T) {
	x.X()
}

Run the following commands:

$ go test -coverprofile=/tmp/cover.tmp local/x
$ go tool cover -html=/tmp/cover.tmp

What did you expect to see?

A code listing showing which statements have been executed.

What did you see instead?

A code listing showing that all statements have been executed.

The relevant snippet of HTML shows:

func X() <span class="cov8" title="1">{
        if false <span class="cov0" title="0"></span>{
                fmt.Printf("Hello")
        }</span>
}

Using 1.10.2 the equivalent snippet is:

func X() <span class="cov8" title="1">{
        if false </span><span class="cov0" title="0">{
                fmt.Printf("Hello")
        }</span>
}
@agnivade
Copy link
Contributor

@agnivade agnivade commented Jun 7, 2018

/cc @ianlancetaylor , @rsc

Marking as release blocker because this seems to be a regression.

@robpike
Copy link
Contributor

@robpike robpike commented Jun 7, 2018

@dsymonds Is this due to your recent change?

@dsymonds
Copy link
Member

@dsymonds dsymonds commented Jun 7, 2018

It certainly sounds like 4fe688c, which fixed a different instance of segment boundaries at the same place being in the wrong order.

@dsymonds
Copy link
Member

@dsymonds dsymonds commented Jun 7, 2018

I've got some of the state still in my head; let me see if I can fix this.

@dsymonds dsymonds self-assigned this Jun 7, 2018
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 7, 2018

Change https://golang.org/cl/116975 mentions this issue: cmd/cover: add test for HTML output

@gopherbot
Copy link

@gopherbot gopherbot commented Jun 7, 2018

Change https://golang.org/cl/116976 mentions this issue: cmd/cover: fix sorting of profile segment boundaries, again

@agnivade agnivade added NeedsFix and removed NeedsInvestigation labels Jun 7, 2018
bradfitz pushed a commit that referenced this issue Jun 7, 2018
This adds a case for what was fixed in 4fe688c to prevent regression;
a follow-on change will address #25767.

Change-Id: Iced8cc10e2993ef7caf7e9c59ffbc7147d78ddd7
Reviewed-on: https://go-review.googlesource.com/116975
Run-TryBot: David Symonds <dsymonds@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot gopherbot closed this in aadaec5 Jun 7, 2018
@golang golang locked and limited conversation to collaborators Jun 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.