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: inconsistent treatment of comments #22545

Open
alandonovan opened this issue Nov 2, 2017 · 2 comments
Open

cmd/cover: inconsistent treatment of comments #22545

alandonovan opened this issue Nov 2, 2017 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@alandonovan
Copy link
Contributor

alandonovan commented Nov 2, 2017

Run coverage on the following program using these commands:

$ cd $GOROOT/src/test
$ cat test.go
package test

func f() {
        if true {
                println("A")
        }
        // comment
        if true {
                // comment
                if false {
                        println("B")
                }
        } else {
                // comment
                println("D")
        }
}
$ cat test_test.go 
package test

import "testing"

func Test(t *testing.T) { f() }
$ go test -coverprofile=c.out test
ok      test    0.015s  coverage: 66.7% of statements
$ command go tool cover -html=c.out

This is the rendered HTML output:

coverage

Observe that the first comment is grey, not green, even though it was covered. Is this a simple bookkeeping error, or is there a design reason why we shouldn't consider the entire span from func f() { up to if false as covered, and render it green?

(This is Google internal issue 68650370.)

@ianlancetaylor ianlancetaylor added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 29, 2018
@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Mar 29, 2018
@dsymonds
Copy link
Contributor

I've been observing this and similar. Comments and function headers (the bit before the opening brace) should be grey, because they are not executable statements. (I shouldn't decrease my code's coverage simply by adding a bunch more comments, for instance.)

@gregoryv
Copy link

gregoryv commented Jan 6, 2020

I've been observing this and similar. Comments and function headers (the bit before the opening brace) should be grey, because they are not executable statements. (I shouldn't decrease my code's coverage simply by adding a bunch more comments, for instance.)

@dsymonds adding more comments to the uncovered section in your example does not decrease test coverage, the numbers are the same even if there are more non-green lines.

comments

The colours are however misleading as you say, some are green and others are not. I've been looking into this, not in the "cover" tool but in a variant(based on the original) which outputs the result to a terminal.

However I disagree that the function signature should be grey, it is called during execution isn't it?
Comments are also grey(sometimes) but in the view of a coverage analysis I think they are part of the path between two statements and execution does cover that path.

In my variation I actually changed this to only use green and red colours which resulted in

uncover-colors

Changes can be summarized as

  • signature is green if it has > 0% coverage
  • function open and close braces are green
  • all comments crossed by an executed path within a function are green

Benefit of this is the reader only has to scan for the red colour, grey and green basically mean the same within a function context.
To summarize; I would like to simplify colouring to only display red and green within a function context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. 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