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.

Sign up for GitHub

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

tolerate run without pass/failed for benchmarks #438

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

tolerate run without pass/failed for benchmarks #438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions testjson/execution.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ func (n TestName) IsSubTest() bool {
return strings.Contains(string(n), "/")
}

// IsBenchmark returns true if the name indicates that the test is a benchmark.
func (n TestName) IsBenchmark() bool {
return strings.HasPrefix(string(n), "Bench")
}

func (n TestName) Name() string {
return string(n)
}
Expand Down Expand Up @@ -276,6 +281,13 @@ func (p *Package) end() []TestEvent {
continue
}

if tc.Test.IsBenchmark() && !p.panicked {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's debatable whether this should check for a panic. I opted for staying closer to the current behavior unless a benchmark completed successfully.

Alternatively, the exit code could also be considered - but it is not always available?

// Mitigate go >= 1.20 issuing `run` events for benchmarks without
// a matching `pass` or `fail` event (https://github.com/gotestyourself/gotestsum/issues/413#issuecomment-2343206787,
// https://github.com/golang/go/issues/66825#issuecomment-2343229005).
continue
}

tc.Elapsed = neverFinished
p.Failed = append(p.Failed, tc)

Expand Down
10 changes: 10 additions & 0 deletions testjson/summary_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,16 @@ func TestPrintSummary(t *testing.T) {
},
expectedOut: "summary/with-run-id",
},
{
name: "Go 1.20 benchmark",
config: scanConfigFromGolden("input/go-1-20-benchmark.out"),
expectedOut: "summary/go-1-20-benchmark",
},
{
name: "Go 1.20 benchmark panic",
config: scanConfigFromGolden("input/go-1-20-panicked-benchmark.out"),
expectedOut: "summary/go-1-20-benchmark-panic",
},
}

for _, tc := range testCases {
Expand Down
12 changes: 12 additions & 0 deletions testjson/testdata/input/go-1-20-benchmark.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{"Time":"2024-09-11T12:03:43.407422644+02:00","Action":"start","Package":"github.com/go-logr/logr/benchmark"}
{"Time":"2024-09-11T12:03:43.412315359+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"goos: linux\n"}
{"Time":"2024-09-11T12:03:43.41235901+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"goarch: amd64\n"}
{"Time":"2024-09-11T12:03:43.412368425+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"pkg: github.com/go-logr/logr/benchmark\n"}
{"Time":"2024-09-11T12:03:43.412375551+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"cpu: Intel(R) Core(TM) i9-7980XE CPU @ 2.60GHz\n"}
{"Time":"2024-09-11T12:03:43.412385223+02:00","Action":"run","Package":"github.com/go-logr/logr/benchmark","Test":"BenchmarkDiscardLogInfoOneArg"}
{"Time":"2024-09-11T12:03:43.412392388+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Test":"BenchmarkDiscardLogInfoOneArg","Output":"=== RUN BenchmarkDiscardLogInfoOneArg\n"}
{"Time":"2024-09-11T12:03:43.412399638+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Test":"BenchmarkDiscardLogInfoOneArg","Output":"BenchmarkDiscardLogInfoOneArg\n"}
{"Time":"2024-09-11T12:03:44.671541518+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Test":"BenchmarkDiscardLogInfoOneArg","Output":"BenchmarkDiscardLogInfoOneArg-36 \t12695464\t 91.33 ns/op\n"}
{"Time":"2024-09-11T12:03:44.671596547+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"PASS\n"}
{"Time":"2024-09-11T12:03:44.672577312+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"ok \tgithub.com/go-logr/logr/benchmark\t1.265s\n"}
{"Time":"2024-09-11T12:03:44.67260672+02:00","Action":"pass","Package":"github.com/go-logr/logr/benchmark","Elapsed":1.2650000000000001}
12 changes: 12 additions & 0 deletions testjson/testdata/input/go-1-20-panicked-benchmark.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{"Time":"2024-09-11T12:03:43.407422644+02:00","Action":"start","Package":"github.com/go-logr/logr/benchmark"}
{"Time":"2024-09-11T12:03:43.412315359+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"goos: linux\n"}
{"Time":"2024-09-11T12:03:43.41235901+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"goarch: amd64\n"}
{"Time":"2024-09-11T12:03:43.412368425+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"pkg: github.com/go-logr/logr/benchmark\n"}
{"Time":"2024-09-11T12:03:43.412375551+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"cpu: Intel(R) Core(TM) i9-7980XE CPU @ 2.60GHz\n"}
{"Time":"2024-09-11T12:03:43.412385223+02:00","Action":"run","Package":"github.com/go-logr/logr/benchmark","Test":"BenchmarkDiscardLogInfoOneArg"}
{"Time":"2024-09-11T12:03:43.412392388+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Test":"BenchmarkDiscardLogInfoOneArg","Output":"=== RUN BenchmarkDiscardLogInfoOneArg\n"}
{"Time":"2024-09-11T12:03:43.412399638+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Test":"BenchmarkDiscardLogInfoOneArg","Output":"BenchmarkDiscardLogInfoOneArg\n"}
{"Time":"2024-09-11T12:03:44.671541518+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Test":"BenchmarkDiscardLogInfoOneArg","Output":"BenchmarkDiscardLogInfoOneArg-36 \t12695464\t 91.33 ns/op\n"}
{"Time":"2024-09-11T12:03:44.671596547+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"PASS\n"}
{"Time":"2024-09-11T12:03:44.672577312+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"ok \tgithub.com/go-logr/logr/benchmark\t1.265s\n"}
{"Time":"2024-09-11T12:03:44.67260672+02:00","Action":"output","Package":"github.com/go-logr/logr/benchmark","Output":"panic: fabricated panic"}
2 changes: 2 additions & 0 deletions testjson/testdata/summary/go-1-20-benchmark
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

DONE 1 tests in 0.000s
8 changes: 8 additions & 0 deletions testjson/testdata/summary/go-1-20-benchmark-panic
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

=== Failed
=== FAIL: github.com/go-logr/logr/benchmark BenchmarkDiscardLogInfoOneArg (unknown)
=== RUN BenchmarkDiscardLogInfoOneArg
BenchmarkDiscardLogInfoOneArg
BenchmarkDiscardLogInfoOneArg-36 12695464 91.33 ns/op

DONE 1 tests, 1 failure in 0.000s
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the default format is not informative enough when there was a panic: that panic output is not shown.

Other formats don't have that problem:

$ go run ./ --format=standard-verbose --raw-command cat testjson/testdata/input/go-1-20-panicked-benchmark.out
goos: linux
goarch: amd64
pkg: github.com/go-logr/logr/benchmark
cpu: Intel(R) Core(TM) i9-7980XE CPU @ 2.60GHz
=== RUN   BenchmarkDiscardLogInfoOneArg
BenchmarkDiscardLogInfoOneArg
BenchmarkDiscardLogInfoOneArg-36    	12695464	        91.33 ns/op
PASS
ok  	github.com/go-logr/logr/benchmark	1.265s
panic: fabricated panic
=== Failed
=== FAIL: github.com/go-logr/logr/benchmark BenchmarkDiscardLogInfoOneArg (unknown)
=== RUN   BenchmarkDiscardLogInfoOneArg
BenchmarkDiscardLogInfoOneArg
BenchmarkDiscardLogInfoOneArg-36    	12695464	        91.33 ns/op

DONE 1 tests, 1 failure in 0.001s

This is unrelated to this PR, I just noticed because the tests use the default format.