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

Successful benchmark run is marked as failed #332

Open
noBlubb opened this issue May 25, 2023 · 2 comments
Open

Successful benchmark run is marked as failed #332

noBlubb opened this issue May 25, 2023 · 2 comments
Labels
test2json-bug A bug in test2json which impacts gotestsum

Comments

@noBlubb
Copy link
Contributor

noBlubb commented May 25, 2023

Hey all,

we observed gotestsum fail our benchmarks despite the benchmarks running fine. I tried to reproduce the issue and this was the smallest setup I could reproduce the issue with (using the latest gotestsum release v1.10.0):

Given a simple benchmark

import "testing"

func BenchmarkFuu(b *testing.B) {
	l := 0
	for i := 0; i < b.N; i++ {
		l++
	}
}

when run as e.g.

gotestsum --format standard-verbose --junitfile junit-results.xml --rerun-fails --rerun-fails-max-failures 10 --packages=. -- --bench=.      

goos: darwin
goarch: arm64
=== RUN   BenchmarkFuu
BenchmarkFuu
BenchmarkFuu-8   	1000000000	         0.3320 ns/op
PASS
ok  	...	0.571s

=== Failed
=== FAIL: . BenchmarkFuu (unknown)
=== RUN   BenchmarkFuu
BenchmarkFuu
BenchmarkFuu-8   	1000000000	         0.3320 ns/op

DONE 1 tests, 1 failure in 1.007s

it should not mark the test as failed. go version is go version go1.20.4 darwin/arm64.

I found #62, is this related? Or do we use some incompatible configuration?

@noBlubb noBlubb changed the title Trivial benchmark is marked as failed Successful benchmark run is marked as failed May 25, 2023
@dnephin
Copy link
Member

dnephin commented May 25, 2023

Thank you for the bug report! I ran this example with go test -json -bench=. to see what test2json output was received by gotestsum. The output looks something like this:

{"Action":"start","Package":"example.com"}
{"Action":"output","Package":"example.com","Output":"goos: linux\n"}
{"Action":"output","Package":"example.com","Output":"goarch: amd64\n"}
{"Action":"output","Package":"example.com","Output":"pkg: example.com\n"}
{"Action":"output","Package":"example.com","Output":"cpu: ...\n"}
{"Action":"run","Package":"example.com","Test":"BenchmarkFuu"}
{"Action":"output","Package":"example.com","Test":"BenchmarkFuu","Output":"=== RUN   BenchmarkFuu\n"}
{"Action":"output","Package":"example.com","Test":"BenchmarkFuu","Output":"BenchmarkFuu\n"}
{"Action":"output","Package":"example.com","Test":"BenchmarkFuu","Output":"BenchmarkFuu-20    \t1000000000\t         0.1101 ns/op\n"}
{"Action":"output","Package":"example.com","Output":"PASS\n"}
{"Action":"output","Package":"example.com","Output":"ok  \texample.com\t0.126s\n"}
{"Action":"pass","Package":"example.com","Elapsed":0.126}

The problem in #62 does still appear to present, but I think this is a new regression in go1.20 (#322 is a similar problem). The test2json output changed quite a bit in Go 1.20, and it looks like one of those changes is that there's no longer a pass or fail event for the benchmark, which is supposed to report the elapsed time.

gotestsum marks the test as failed in these cases because there's no way to determine if the test or benchmark passed or failed when the terminating event is missing.

I'm not sure what to do about this. I haven't yet searched the Go issue tracker to see if someone else has reported the problem.

@dnephin dnephin added the test2json-bug A bug in test2json which impacts gotestsum label May 25, 2023
@lmb
Copy link

lmb commented Dec 1, 2023

This is golang/go#61767

lmb added a commit to lmb/ebpf that referenced this issue Dec 1, 2023
gotestsum can't properly process benchmark results due to a go toolchain
bug. Remove the postprocessing, since we don't benefit as much now that
we don't use Semaphore CI anymore (which had nice visualisation for
JUnit output).

See gotestyourself/gotestsum#332

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
lmb added a commit to lmb/ebpf that referenced this issue Dec 5, 2023
Execute benchmarks once on CI, to prevent bitrot from setting in.
We do this as a separate target without gotestsum filtering since
there is a bug in the Go toolchain which prevents benchmark output
from being parsed properly.

See gotestyourself/gotestsum#332

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
lmb added a commit to cilium/ebpf that referenced this issue Dec 5, 2023
Execute benchmarks once on CI, to prevent bitrot from setting in.
We do this as a separate target without gotestsum filtering since
there is a bug in the Go toolchain which prevents benchmark output
from being parsed properly.

See gotestyourself/gotestsum#332

Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
vincentbernat added a commit to akvorado/akvorado that referenced this issue Dec 6, 2023
The value added is quite low and there is currently a bug marking the
tests as failed, which is confusing. See
gotestyourself/gotestsum#332.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test2json-bug A bug in test2json which impacts gotestsum
Projects
None yet
Development

No branches or pull requests

3 participants