Always report exceptions thrown by tests even if JenkinsRule cleanup fails as well #236
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
slonopotamus
approved these changes
Aug 14, 2020
Contributor
slonopotamus
left a comment
slonopotamus
left a comment
There was a problem hiding this comment.
Hello from the author of #166, your change is definitely an improvement.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While trying to investigate test failures in jenkinsci/workflow-durable-task-step-plugin#139 I realized that #166 made it so that any exceptions thrown by
env.dispose()supersede any exceptions thrown by the test itself.This means that if a regular test assertion fails while the test is in a state that also causes
env.dispose()to fail (e.g. an assertion fails while a build is running, which means that the build cannot be cleaned up Windows because the log is still open), only the cleanup failure is reported, which makes it difficult to diagnose the real issue.This PR tracks exceptions thrown by tests and makes sure that they are always thrown as the top-level exception. Additional exceptions thrown by
JenkinsRule.afterare reported as suppressed exceptions.Tested in jenkinsci/workflow-durable-task-step-plugin#139, see this commit and this comment showing that the assertion is the top-level exception and the cleanup failure is suppressed.