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

Fix panic from missing root test #416

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix panic from missing root test #416

wants to merge 4 commits into from

Conversation

dnephin
Copy link
Member

@dnephin dnephin commented Jun 17, 2024

Fixes #413

I should add a test case for this before merging.

@pohly
Copy link

pohly commented Jun 17, 2024

It might be worth checking more thoroughly what go test -bench -json produces, if time permits. As a hotfix this seems okay.

@dnephin
Copy link
Member Author

dnephin commented Sep 16, 2024

Alternatives to fixing this problem could be to remove the hasSubTestFailed map to avoid the problem. Maybe those upstream bugs are fixed now.

As you suggested in another issue, using the exit status to figure out if we need to print output for tests might be better than checking subtests.

@pohly
Copy link

pohly commented Sep 17, 2024

In #438 (comment) you said that this PR "is the right fix for" #413.

I'm not sure whether that means that it should already be fixed or whether this approach will eventually lead to a fix, so I tried it based on 49b2002 - it was still producing odd results.

$ git fetch origin 49b20021da25dcbeb5b7abc1d67c9ca2c7762a09
From github.com:gotestyourself/gotestsum
 * branch            49b20021da25dcbeb5b7abc1d67c9ca2c7762a09 -> FETCH_HEAD
$ git checkout FETCH_HEAD 
...
$ cat >test.log
{"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}

$ go run ./ --raw-command cat test.log
✓  github.com/go-logr/logr/benchmark (1.265s)

=== 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
$ echo $?
0

Note that it prints "Failed", but then exits with a zero exit code, indicating success.

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

Successfully merging this pull request may close these issues.

panic when converting "go test -bench" output
2 participants