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/go: -cover stdout format is not consistent between failed & pass #39070

Open
christophermancini opened this issue May 14, 2020 · 6 comments
Open
Milestone

Comments

@christophermancini
Copy link

@christophermancini christophermancini commented May 14, 2020

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

$ go version
go version go1.14.2 windows/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
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\\AppData\Local\go-build
set GOENV=C:\Users\\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Chris\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\\AppData\Local\Temp\go-build342408862=/tmp/go-build -gno-record-gcc-switches

What did you do?

Run go test -cover on any set of packages including a failed tests.

What did you expect to see?

$ go test -count=1 -cover -tags="sample" github.com/digitalocean/gocop/sample/...
# cover github.com/digitalocean/gocop/sample/failbuild
2020/05/14 09:18:17 cover: C:\Users\Chris\dev\code\do\gocop\sample\failbuild\broken.go: C:\Users\Chris\dev\code\do\gocop\sample\failbuild\broken.go:5:1: expected declaration, found fun
--- FAIL: TestWillFail (0.00s)
    failing_test.go:14: number does equal eleven
--- FAIL: TestWillFailComplex (0.00s)
    --- FAIL: TestWillFailComplex/my_simple_test (0.00s)      
        failing_test.go:37: 423 (int) to equal 10 (int)       
            failing_test.go:37
    --- FAIL: TestWillFailComplex/my_simple_test2 (0.00s)     
        failing_test.go:37: 517 (int) to equal 12 (int)       
            failing_test.go:37
FAIL
FAIL    github.com/digitalocean/gocop/sample/fail       0.028s  coverage: 100.0% of statements
FAIL    github.com/digitalocean/gocop/sample/failbuild [build failed]
ok      github.com/digitalocean/gocop/sample/flaky      0.024s  coverage: [no statements]        
?       github.com/digitalocean/gocop/sample/numbers    [no test files]
ok      github.com/digitalocean/gocop/sample/pass       0.018s  coverage: 100.0% of statements   
ok      github.com/digitalocean/gocop/sample/pass/nostatements  0.025s  coverage: [no statements]
FAIL

What did you see instead?

$ go test -count=1 -cover -tags="sample" github.com/digitalocean/gocop/sample/...
# cover github.com/digitalocean/gocop/sample/failbuild
2020/05/14 09:18:17 cover: C:\Users\Chris\dev\code\do\gocop\sample\failbuild\broken.go: C:\Users\Chris\dev\code\do\gocop\sample\failbuild\broken.go:5:1: expected declaration, found fun
--- FAIL: TestWillFail (0.00s)
    failing_test.go:14: number does equal eleven
--- FAIL: TestWillFailComplex (0.00s)
    --- FAIL: TestWillFailComplex/my_simple_test (0.00s)      
        failing_test.go:37: 423 (int) to equal 10 (int)       
            failing_test.go:37
    --- FAIL: TestWillFailComplex/my_simple_test2 (0.00s)     
        failing_test.go:37: 517 (int) to equal 12 (int)       
            failing_test.go:37
FAIL
coverage: 100.0% of statements
FAIL    github.com/digitalocean/gocop/sample/fail       0.028s
FAIL    github.com/digitalocean/gocop/sample/failbuild [build failed]
ok      github.com/digitalocean/gocop/sample/flaky      0.024s  coverage: [no statements]        
?       github.com/digitalocean/gocop/sample/numbers    [no test files]
ok      github.com/digitalocean/gocop/sample/pass       0.018s  coverage: 100.0% of statements   
ok      github.com/digitalocean/gocop/sample/pass/nostatements  0.025s  coverage: [no statements]
FAIL

The same behavior is observed via json formatting.

@christophermancini christophermancini changed the title cmd/go: -cover stdout format is not consistent between failed & pawss cmd/go: -cover stdout format is not consistent between failed & pass May 14, 2020
@christophermancini
Copy link
Author

@christophermancini christophermancini commented May 14, 2020

We wrote a tool, GoCop, to enable us to identify and track flaky tests. It is designed to parse stdout of go test so then we can redirect the output to file during CI runs, use the tool to identify packages which failed and attempt to rerun them N times to determine if they are flaky. We also use this tool to persist the test outcome per package to a Postgres database.

When tests fail, the coverage % output being inconsistently placed leads to data that is missed by the tool. When considering flakiness of a package, the coverage info is something we want to track regardless of the package result.

@christophermancini
Copy link
Author

@christophermancini christophermancini commented May 14, 2020

I submitted a change to address this issue, I will update the change with integration tests as requested.

https://go-review.googlesource.com/c/go/+/233617/1

@cagedmantis cagedmantis added this to the Backlog milestone May 18, 2020
@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented May 18, 2020

@gopherbot
Copy link

@gopherbot gopherbot commented May 18, 2020

Change https://golang.org/cl/233617 mentions this issue: cmd/go: include cover percent on failures

@bcmills
Copy link
Member

@bcmills bcmills commented May 19, 2020

As I asked on the CL review: why do we believe that the coverage numbers are meaningful for failed tests?

I suspect that in practice the coverage for failed tests will be systematically lower than for passing tests, because many tests use t.Fatalf or similar on failure — skipping the remainder of the test (and any associated coverage for that test).

So it generally will not be meaningful to compare coverage between two test runs with different failing tests or different failure modes — and if you have a flaky test with a consistent failure mode over time, why not fix the test instead of worrying about its coverage?

@christophermancini
Copy link
Author

@christophermancini christophermancini commented May 21, 2020

@bcmills Thanks for the question. The way I see it, the coverage data reported for a test run is unique to each test run and should be recorded regardless of the outcome of a test. The coverage being potentially less for a failed test run is no different than coverage being less for a passing run if -short is used. Or the coverage being less based on the build constraints that were used to execute the tests. Test coverage can also differ for different platforms. Each run of a suite of tests has the possibility of having a different coverage % due to many factors.

If the expectation is that the coverage % is always reflective of the parameters/factors of the test execution, then there is no logical reason to have inconsistencies between reporting this value based on the outcome of the execution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.