-
-
Notifications
You must be signed in to change notification settings - Fork 667
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
JUnit: include more detailed information about panic #519
Conversation
I'm getting a test failure also without this PR:
|
The https://travis-ci.org/onsi/ginkgo/builds/427319025?utm_source=github_status&utm_medium=notification test run failed because gopkg.in seems to be down ( |
I have restarted the build, let's see if it becomes green. |
@pohly do you think you would be able to add a test for the new functionality? |
Andrea Nodari <notifications@github.com> writes:
@pohly do you think you would be able to add a test for the new
functionality?
Yes, but not this week.
|
Sure no worries @pohly. Feel free to ping me if you need help. |
When a test aborts with a panic, the generic information ("Test Panicked", single source line) is not enough. We also better include the error that was handed to panic() and a full stacktrace to determine the root cause.
@node finally got around to extending the tests. I hope this is what you had in mind. |
@nodo Can you review? It seems like you last had context. |
@nodo ping |
Thanks @pohly ! Looks good to me 👍 |
When a test aborts with a panic, the generic information ("Test
Panicked", single source line) is not enough. We also better include
the error that was handed to panic() and a full stacktrace to
determine the root cause.