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

Jump to bottom

cmd/cover: inconsistent NumStmt with prometheus #45361

Open
catenacyber opened this issue Apr 2, 2021 · 7 comments
Open

cmd/cover: inconsistent NumStmt with prometheus #45361

catenacyber opened this issue Apr 2, 2021 · 7 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@catenacyber
Copy link
Contributor

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

$ go version
go version go1.16.2 linux/amd64

Does this issue reproduce with the latest release?

It is indeed the latest release

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/root/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/root/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/root/.go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/root/.go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.2"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build2943966716=/tmp/go-build -gno-record-gcc-switches"

What did you do?

git clone https://github.com/prometheus/prometheus
cd prometheus/promql/parser
go test -coverprofile=wrong.out
go tool cover -html wrong.out

What did you expect to see?

Get the HTML coverage report

What did you see instead?

Error message cover: inconsistent NumStmt: changed from 2 to 1

The wrong.out contains indeed (running grep generated_parser.y.go wrong.out | grep 178

github.com/prometheus/prometheus/promql/parser/generated_parser.y.go:178.0,180.0 2 1
github.com/prometheus/prometheus/promql/parser/generated_parser.y.go:179.0,178.0 2 1
github.com/prometheus/prometheus/promql/parser/generated_parser.y.go:176.0,178.0 1 1
github.com/prometheus/prometheus/promql/parser/generated_parser.y.go:178.0,180.0 1 1

I found this by modifying the covermessage with adding %#+v about other fields
@catenacyber catenacyber mentioned this issue Apr 2, 2021
Closed
@seankhliao seankhliao changed the title Coverage inconsistent NumStmt with prometheus cmd/cover: inconsistent NumStmt with prometheus Apr 3, 2021
@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 3, 2021
@seankhliao seankhliao mentioned this issue Apr 11, 2021
Closed
benthor added a commit to netsys-lab/scion-rains that referenced this issue Dec 3, 2021
@benthor
coverage tests currently broken, see golang/go#45361
f97fce2
@yufeiminds
Copy link

It still does not work on my laptop.

  • OS: macOS Catalina 10.15.5 19F101 x86_64 / Intel Chip
  • Go version: 1.18.3

Any update on this issue?

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
@wojciech-sneller
Copy link

I encountered the same problem with the latest Go 1.20. Unfortunately, it's in a proprietary software, but also applies to a generated file -- in our case by Ragel. I'll try to find reproducer for this.

@catenacyber
Copy link
Contributor Author

friendly ping @golang/runtime ?

@jmnote jmnote mentioned this issue Apr 26, 2023
Merged
@ywwg
Copy link

ywwg commented May 2, 2023

This is also still broken for our repo and hampers development

@catenacyber
Copy link
Contributor Author

@neild the line directive strikes again it seems (messing up line numbers between the generated file and the generating file)

@dhubler
Copy link

dhubler commented Jun 13, 2023

Is there a workaround? My project does have generated code from goyacc FWIW.

AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 11, 2023
@AlexanderYastrebov
build: simplify go commands
f979407 
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was nessesary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test intruduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@AlexanderYastrebov AlexanderYastrebov mentioned this issue Jul 11, 2023
Merged
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 11, 2023
@AlexanderYastrebov
build: simplify go commands
a3caf44 
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was nessesary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test intruduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 11, 2023
@AlexanderYastrebov
build: simplify go commands
7ba6007 
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test intruduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 11, 2023
@AlexanderYastrebov
build: simplify go commands
f2c7133 
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test introduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 11, 2023
@AlexanderYastrebov
build: simplify go commands
39ea712 
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test introduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 11, 2023
@AlexanderYastrebov
build: simplify go commands
0f0302d 
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* increases redis testcontainer ping and shutdown timeouts after observed CI failures
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test introduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 12, 2023
@AlexanderYastrebov
eskip: re-generate parser
318e9ef 
go tool cover fails for eskip package:
```
$ go test ./eskip/ -coverprofile=.coverprofile && go tool cover -func .coverprofile
ok      github.com/zalando/skipper/eskip        0.026s  coverage: 88.2% of statements
cover: inconsistent NumStmt: changed from 1 to 2
```

this apparently happens due to malformed .coverprofile containing non-go files
```sh
$ cut -d: -f1 .coverprofile | grep -v -F '.go' | sort -u
github.com/zalando/skipper/eskip/parser.y
github.com/zalando/skipper/eskip/yaccpar
mode
```
added by line directives in generated parser.go.

It looks similar to the issue golang/go#45361

To fix the problem updated go:generate directive omit line directives (-l) and
parsing tables (-v "") and re-generated parser:
```sh
$ rm $(which goyacc)
$ go install golang.org/x/tools/cmd/goyacc
$ go generate ./eskip
```

Benchmark results did not change after re-generate:
```
name               old time/op    new time/op    delta
ParsePredicates-8    4.58µs ± 7%    4.42µs ± 0%   ~     (p=0.064 n=10+8)
LBBackendString-8    2.40µs ±12%    2.53µs ±11%   ~     (p=0.060 n=10+10)

name               old alloc/op   new alloc/op   delta
ParsePredicates-8      744B ± 0%      744B ± 0%   ~     (all equal)
LBBackendString-8    4.65kB ± 0%    4.65kB ± 0%   ~     (all equal)

name               old allocs/op  new allocs/op  delta
ParsePredicates-8      35.0 ± 0%      35.0 ± 0%   ~     (all equal)
LBBackendString-8      12.0 ± 0%      12.0 ± 0%   ~     (all equal)
```

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 12, 2023
@AlexanderYastrebov
eskip: re-generate parser
2c9d91b 
go tool cover fails for eskip package:
```
$ go test ./eskip/ -coverprofile=.coverprofile && go tool cover -func .coverprofile
ok      github.com/zalando/skipper/eskip        0.026s  coverage: 88.2% of statements
cover: inconsistent NumStmt: changed from 1 to 2
```

this apparently happens due to malformed .coverprofile containing non-go files
```sh
$ cut -d: -f1 .coverprofile | grep -v -F '.go' | sort -u
github.com/zalando/skipper/eskip/parser.y
github.com/zalando/skipper/eskip/yaccpar
mode
```
added by line directives in generated parser.go.

The problem looks similar to the issue golang/go#45361.

To fix the problem updated go:generate directive to omit line directives (-l) and
parsing tables (-v "") and re-generated the parser:
```sh
$ rm $(which goyacc)
$ go install golang.org/x/tools/cmd/goyacc
$ go generate ./eskip
```

Benchmark results did not change after re-generate:
```
name               old time/op    new time/op    delta
ParsePredicates-8    4.58µs ± 7%    4.42µs ± 0%   ~     (p=0.064 n=10+8)
LBBackendString-8    2.40µs ±12%    2.53µs ±11%   ~     (p=0.060 n=10+10)

name               old alloc/op   new alloc/op   delta
ParsePredicates-8      744B ± 0%      744B ± 0%   ~     (all equal)
LBBackendString-8    4.65kB ± 0%    4.65kB ± 0%   ~     (all equal)

name               old allocs/op  new allocs/op  delta
ParsePredicates-8      35.0 ± 0%      35.0 ± 0%   ~     (all equal)
LBBackendString-8      12.0 ± 0%      12.0 ± 0%   ~     (all equal)
```

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
@AlexanderYastrebov AlexanderYastrebov mentioned this issue Jul 12, 2023
Merged
@AlexanderYastrebov
Copy link
Contributor

It seems that //line directives inserted into generated parser by goyacc result in invalid entries in coverprofile.
The workaround could be to disable line directives via -l flag, see zalando/skipper#2450.

AlexanderYastrebov added a commit to zalando/skipper that referenced this issue Jul 12, 2023
@AlexanderYastrebov
eskip: re-generate parser (#2450)
2a38f0e 
go tool cover fails for eskip package:
```
$ go test ./eskip/ -coverprofile=.coverprofile && go tool cover -func .coverprofile
ok      github.com/zalando/skipper/eskip        0.026s  coverage: 88.2% of statements
cover: inconsistent NumStmt: changed from 1 to 2
```

this apparently happens due to malformed .coverprofile containing non-go files
```sh
$ cut -d: -f1 .coverprofile | grep -v -F '.go' | sort -u
github.com/zalando/skipper/eskip/parser.y
github.com/zalando/skipper/eskip/yaccpar
mode
```
added by line directives in generated parser.go.

The problem looks similar to the issue golang/go#45361.

To fix the problem updated go:generate directive to omit line directives (-l) and
parsing tables (-v "") and re-generated the parser:
```sh
$ rm $(which goyacc)
$ go install golang.org/x/tools/cmd/goyacc
$ go generate ./eskip
```

Benchmark results did not change after re-generate:
```
name               old time/op    new time/op    delta
ParsePredicates-8    4.58µs ± 7%    4.42µs ± 0%   ~     (p=0.064 n=10+8)
LBBackendString-8    2.40µs ±12%    2.53µs ±11%   ~     (p=0.060 n=10+10)

name               old alloc/op   new alloc/op   delta
ParsePredicates-8      744B ± 0%      744B ± 0%   ~     (all equal)
LBBackendString-8    4.65kB ± 0%    4.65kB ± 0%   ~     (all equal)

name               old allocs/op  new allocs/op  delta
ParsePredicates-8      35.0 ± 0%      35.0 ± 0%   ~     (all equal)
LBBackendString-8      12.0 ± 0%      12.0 ± 0%   ~     (all equal)
```

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
MustafaSaber pushed a commit to zalando/skipper that referenced this issue Jul 12, 2023
@AlexanderYastrebov
build: simplify go commands (#2449)
4175663 
Use ./... instead of loop over packages.

Originally loop over packages was added by #359 that introduced glide package manager.
This manager fetches dependencies into vendor/ subdirectory (see https://glide.readthedocs.io/en/latest/getting-started/) and
apparently loop over packages was necessary back then.
glide usage was removed by #764

The change now also runs testable examples.

Also:
* increases redis testcontainer ping and shutdown timeouts after observed CI failures
* fixes cmd/eskip etcd shutdown similar to #2359
* fixes cmd/eskip test introduced by #2202
* fixes lua example
* removed bench target as it makes little sense
* .coverprofile-all can not be simplified yet due to golang/go#45361

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
Go Compiler / Runtime
Status: Triage Backlog
Milestone
Unplanned
Development

No branches or pull requests
9 participants
@dhubler @AlexanderYastrebov @ywwg @gopherbot @yufeiminds @seankhliao @catenacyber @wojciech-sneller and others