-
Notifications
You must be signed in to change notification settings - Fork 18k
testing: t.Failed() returns false during panic #49929
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
https://go.dev/play/p/THKjlNkot6b
Changing the panic to t.Fatal("foo") does say it failed, so panic probably ideally would do the same. |
On further thought, there's a clear distinction between defer and cleanup. Deferred func can recover the panic and stop the failure. The cleanup cannot. So it makes sense for cleanup to see t.Failed and the defer not to. I see the bug in the Cleanup case and will send a CL (but not for Go 1.18). |
Thanks for looking into this, @rsc!
I agree. Also, currently we are being a bit inconsistent since https://go.dev/play/p/G_BE1zxkcEG
Prints: |
Rolling forward to 1.20. Please comment if you disagree. Thanks. |
Does this issue reproduce with the latest release?
Yes (1.17.3)
What operating system and processor architecture are you using (
go env
)?Any
What did you do?
Called
t.Failed()
from a deferred cleanup function registered for a test that panicked (see https://go.dev/play/p/f0Xu8OyA--h).What did you expect to see?
t.Failed()
return true.What did you see instead?
t.Failed()
returned false.The issue isn't verified if the test has parallel subtests (see https://go.dev/play/p/37YXOUyv5-z). I believe that's because in that case this branch will be ignored, and this closure will call
t.Fail()
before invoking the cleanup functions .The text was updated successfully, but these errors were encountered: