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
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@mhilton
Copy link

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 agnivade added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Jun 7, 2018
@agnivade agnivade added this to the Go1.11 milestone Jun 7, 2018
@agnivade
Copy link
Contributor

agnivade commented Jun 7, 2018

/cc @ianlancetaylor , @rsc

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

@robpike
Copy link
Contributor

robpike commented Jun 7, 2018

@dsymonds Is this due to your recent change?

@dsymonds
Copy link
Contributor

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
Contributor

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
Contributor

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

@gopherbot
Copy link
Contributor

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

@agnivade agnivade added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. 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>
@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.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants