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 NumStmt when //line clauses are used with Go 1.13.4 #35781

Open
jszwedko opened this issue Nov 22, 2019 · 6 comments
Open
Labels
help wanted NeedsInvestigation
Milestone

Comments

@jszwedko
Copy link
Contributor

@jszwedko jszwedko commented Nov 22, 2019

This seems to be the same as #27350 but still happens even after running go fmt which was the recommended fix in that issue (and also in a release that includes d689946 which was intended to mitigate it).

Removing all //line comments from the file allows it to pass.

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

$ go version
go version go1.13.4 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/jesse/.cache/go-build"
GOENV="/home/jesse/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/jesse/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/jesse/go/src/github.com/timberio/go-datemath/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build279863626=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Using this repo, which uses goyacc to generate datemath.y.go, https://github.com/timberio/go-datemath

$ go test -coverprofile=c.out && go tool cover -html=c.out
PASS
coverage: 71.4% of statements
ok      github.com/timberio/go-datemath 0.003s
cover: inconsistent NumStmt: changed from 2 to 1

I see the same issue using the example in the goyacc directory:

https://github.com/golang/tools/tree/master/cmd/goyacc/testdata/expr

When adding a dummy test file:

$ cat expr_test.go
package main

import "testing"

func TestFoo(t *testing.T) {
}

and then running:

$ go generate && go fmt && go test -coverprofile=c.out && go tool cover -html=c.out
PASS
coverage: 0.0% of statements
ok      golang.org/x/tools/cmd/goyacc/testdata/expr     0.003s
cover: inconsistent NumStmt: changed from 1 to 2

What did you expect to see?

The coverage pulled up in the browser.

What did you see instead?

PASS
coverage: 71.4% of statements
ok      github.com/timberio/go-datemath 0.006s
cover: inconsistent NumStmt: changed from 2 to 1
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 22, 2019

The problem seems to be that the cover tool assumes that all lines in a single input file correspond to a single input file. In this case the input file datemath.y.go is composed of different input files. The coverage tool is assigning some coverage information to line 89 of datemath.y and some to line 89 of yaccpar. The first problem is that the coverage information for those two different lines is being mixed together. The second more immediate problem is that line 89 of yaccpar is a for statement with multiple statements on the line, and line 89 of datemath.y is not. That causes the cover tool to report an inconsistency.

I think the correct fix has to involve somehow making the cover tool aware of multiple source files within a single input file. Or, alternatively, entirely ignoring //line directives.

@ianlancetaylor ianlancetaylor added NeedsInvestigation help wanted labels Nov 22, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.15 milestone Nov 22, 2019
@jaqx0r
Copy link
Contributor

@jaqx0r jaqx0r commented Dec 12, 2019

This was working in Go 1.12, and now not in 1.13, so what changed? Is this a bug in how goyacc makes line number comments or in how go cover treats them?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 12, 2019

I'm not sure. Want to look into it?

@jaqx0r
Copy link
Contributor

@jaqx0r jaqx0r commented Dec 12, 2019

@odeke-em
Copy link
Member

@odeke-em odeke-em commented May 29, 2020

Moving this to Go1.16.

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Feb 4, 2021

Punting to Backlog

@odeke-em odeke-em removed this from the Go1.16 milestone Feb 4, 2021
@odeke-em odeke-em added this to the Backlog milestone Feb 4, 2021
@seankhliao seankhliao changed the title cmd/cover: inconsistent NumStmt when //line clauses are used with Go 1.13.4 cmd/cover: inconsistent NumStmt when //line clauses are used with Go 1.13.4 Jun 18, 2021
powersj added a commit to powersj/telegraf that referenced this issue Jun 17, 2022
On amd64, linux unit test runs, code coverage will be collected and
uploaded to the coveralls coverage service on runs with the master
branch. This does not include forks or PRs from community members.

Currently, the coveralls service requires a token to be used to upload
all coverage reports. However, there is no safe/secure way with circle
ci to hide this with our current processes that we have in place.

This also removes the coverage for the parsers/influx/machine.go file as
it contains many "//line" comments which currently break reports. See
golang/go#35781 for more details.

fixes: influxdata#10733
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

4 participants