-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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: tests that panic or exit are marked as passing when -json flag is used #37555
Comments
Does |
Yes |
To give more context. This behavior was working in Go 1.13 and previous releases and was broken in Go 1.14. |
Thank you for reporting. I can reproduce this 1.14 regression with
/cc @jayconrod @matloob |
Given the reliance of |
This reproduces on tip as well. I'll change this issue to Go 1.15 milestone, and ask @gopherbot to please backport this to Go 1.14. Based on information so far, this looks like a serious bug that likely doesn't have a viable workaround. |
Backport issue(s) opened: #37671 (for 1.14), Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://golang.org/wiki/MinorReleases. |
I think this was triggered by CL 192104. If the test binary fails for any reason (exits non-zero, fails to run), Will have a CL soon. |
Change https://golang.org/cl/222243 mentions this issue: |
I've seen this issue before too since I maintain a testrunner tool that uses the "-json" functionality (https://github.com/grasparv/testie) - did not have time to to report it though at the time it happened and forgot about it. But up until now, these tools have had to check for the |
Change https://golang.org/cl/222658 mentions this issue: |
…r panicking/exiting tests 'go test -json' should report that a test failed if the test binary did not exit normally with status 0. This covers panics, non-zero exits, and abnormal terminations. These tests don't print a final result when run with -test.v (which is used by 'go test -json'). The final result should be "PASS" or "FAIL" on a line by itself. 'go test' prints "FAIL" in this case, but includes error information. test2json was changed in CL 192104 to report that a test passed if it does not report a final status. This caused 'go test -json' to report that a test passed after a panic or non-zero exit. With this change, test2json treats "FAIL" with error information the same as "FAIL" on a line by itself. This is intended to be a minimal fix for backporting, but it will likely be replaced by a complete solution for #29062. Fixes #37671 Updates #37555 Updates #29062 Updates #31969 Change-Id: Icb67bcd36bed97e6a8d51f4d14bf71f73c83ac3d Reviewed-on: https://go-review.googlesource.com/c/go/+/222243 Run-TryBot: Jay Conrod <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> (cherry picked from commit 5ea58c6) Reviewed-on: https://go-review.googlesource.com/c/go/+/222658 Run-TryBot: Dmitri Shuralyov <[email protected]> Reviewed-by: Jay Conrod <[email protected]>
Does this fix cover cases where the (test) code does not compile too? |
@grasparv, if the test does not compile, we should not even make it to the point where we run the test (or the |
It will be hard/impossible to differentiate between:
... in the test runner after running 'go test -json'. Should not all output be JSON if you run with -json? And yes, I understand that it is a wrapper that runs on go test output, but still asking the same question. |
Probably yes. That's what #35169 is about. This issue was about a specific regression in the
|
Thanks for the reply. |
For anyone still seeing this with go1.14.2, there is a related issue when the panic is in a goroutine: #38382 |
I see this in go version go1.14 linux/amd64 (1.14.0, 1.14.1, 1.14.2) |
@jayconrod @bcmills the problem is still reproducible while running
Notice the last event, it claims that |
@zolotov Could you open a new issue for that? That seems like an additional problem beyond the one reported here. It looks like you're running a binary produced with |
@jayconrod sure, I've filed the #40132.
Yes, the same set up is described in this issue so I wonder why it's marked as fixed. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat did you do?
What did you expect to see?
I would expect to see that
TestName2
fails since there's a panic there.See the output here: https://play.golang.org/p/-9xQPOYa5iN
What did you see instead?
Test is marked as
pass
in the resulting json:The test position doesn't seem to matter, if it's the first, the last, or in the middle of the test suite.
This is confirmed to happen both on Windows and Ubuntu.
The text was updated successfully, but these errors were encountered: