-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/go: go 1.22.0 inconsistent and variable code coverage output for the same folder #65570
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
Comments
This is almost certainly an effect of recent changes in
See previously: (attn @thanm) |
Thanks for the report. I'll take a look. |
@dev-gto would you kindly post a pointer to the code that you are working with, so that I can reproduce the problem on my machine? Thanks. |
@thanm , actually my project is a private and large one. Despite some testing errors (the original project does not emit any errors), you can reproduce with go test -cover ./...
ok github.com/registrobr/zxcvbn-go (cached) coverage: 60.6% of statements
ok github.com/registrobr/zxcvbn-go/adjacency (cached) coverage: 40.8% of statements
github.com/registrobr/zxcvbn-go/data coverage: 0.0% of statements
github.com/registrobr/zxcvbn-go/data/ptbr coverage: 0.0% of statements
ok github.com/registrobr/zxcvbn-go/entropy (cached) coverage: 35.9% of statements
... go test -cover ./[ae]*
ok github.com/registrobr/zxcvbn-go/adjacency (cached) coverage: 83.3% of statements
ok github.com/registrobr/zxcvbn-go/entropy (cached) coverage: 50.0% of statements
|
#62212 might be related |
I've confirmed this with both @dev-gto's example above and a large private monorepo where I also encountered the issue. |
Indeed, |
hi! |
Code coverage has been broken for about a year now... Please can someone make this a priority to fix. |
I came here because I just ran into this as well on |
# Description This is to upgrade to 1.22.2. However we might need to wait until the fix for coverage drop issue is ready - golang/go#65570 ## Type of change - This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Radius (issue link optional). <!-- Please update the following to link the associated issue. This is required for some kinds of changes (see above). --> Fixes: #issue_number Signed-off-by: Young Bu Park <[email protected]>
Change https://go.dev/cl/592204 mentions this issue: |
Change https://go.dev/cl/592205 mentions this issue: |
Refactor cformat.EmitPercent to accept a package filter (list of packages to report). This is a no-op in terms of exposed coverage functionality, but we will need this feature in a subsequent patch. Updates #65570. Change-Id: I04ddc624a634837ea31c12ec395aa1295a0ea1f1 Reviewed-on: https://go-review.googlesource.com/c/go/+/592204 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: David Chase <[email protected]>
1.23.1 and 1.22.7 still have this issue, although it was closed. |
sorry about that, actually it is working |
is it? Can't see the difference |
Change https://go.dev/cl/627316 mentions this issue: |
https://go-review.googlesource.com/c/go/+/627316 Seems to include a fix for this, but it has now a merge conflict. |
This is expected behavior given the way Gerrit works. It's a 2-patch sequence, with 627316 dependent on 627315; since 627315 hasn't been submitted yet 627316 will always show "merge conflict". Yes, it's confusing; virtually every developer new to Gerrit/Go puzzles over this same thing. |
Was this fixed in Go 1.23? |
…le packages In ProcessCoverTestDir pass the selected set of packages to EmitTextual in addition to EmitPercent, so that when we have runs with multiple packages selected but without -coverpkg, text format output for package P was incorrectly including output for P's covered dependencies. This is in effect an extension of the fix for issue 65570. Includes a cmd/go script test to verify correct behavior; ideally it would be nice to locate this test in .../internal/coverage somewhere but at the moment script tests are only supported for cmd/{go,compile,link}. Updates #65570. Fixes #70244. Change-Id: Ia0bb10155353aa0f2ead46e81a2aaa71bde4ef82 Reviewed-on: https://go-review.googlesource.com/c/go/+/627316 Reviewed-by: David Chase <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Auto-Submit: Than McIntosh <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
Change https://go.dev/cl/646355 mentions this issue: |
The new test was committed after the removal was tested. For #51430 For #65570 For #70244 Change-Id: I5f94c36a68ea96ba76d018dc06a5a233ad684aa5 Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest Reviewed-on: https://go-review.googlesource.com/c/go/+/646355 Reviewed-by: Ian Lance Taylor <[email protected]> TryBot-Bypass: Ian Lance Taylor <[email protected]> Reviewed-by: Robert Griesemer <[email protected]> Auto-Submit: Ian Lance Taylor <[email protected]>
As far as I can tell, the fix is in this commit 5045538 (the previous commit in #65570 (comment) was prep work, not a fix) This commit is only present in the 1.24 series |
Go version
go version go1.22.0 linux/amd64
Output of
go env
in your module/workspace:What did you do?
Having a directory called, e.g.,
holiday
with some go files and their corresponding test files, I called thego test -cover
from the parent directory. Depending on directories arguments, I have different percentage values, which wasn't occurring on go < 1.22.0.What did you see happen?
What did you expect to see?
A consistent percentage value (in this case, 97.6%) for all commands:
The text was updated successfully, but these errors were encountered: