-
-
Notifications
You must be signed in to change notification settings - Fork 303
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
Move the junit publisher step to the end of the post stage #4541
Conversation
buildenv/jenkins/JenkinsfileBase
Outdated
@@ -848,6 +846,8 @@ def post(output_name) { | |||
} | |||
String failedTests = addFailedTestsGrinderLink() | |||
triggerRerunJob(failedTests) | |||
|
|||
junit allowEmptyResults: true, keepLongStdio: true, testResults: '**/work/**/*.jtr.xml, **/result/**/*.jtr.xml, **/junitreports/**/*.xml, **/external_test_reports/**/*.xml' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should use try catch instead. Other steps can fail as well. If they fail, we do not want junit report to be missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other steps failures mean bugs we need to fix. This failure might be the issue with the junit plugin, which we don't have control.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other steps failures mean bugs we need to fix.
That is not always the case. For example, we could have a network issue that results in a failure to archive the artifacts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Jenkins Junit Plugin itself doesn't generate/throw exception, it only prepare/display unit tests report. Not sure if the try catch can catch the exception or not. Updated and see how it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The suggestion of adding a try/catch is good, expecting to see that and then this will be good to merge.
@sophia-guo - would you be willing to update your branch based on the review comments for a try/catch? |
Avoid the junit publisher failures affecting other functions. Signed-off-by: Sophia Guo <sophia.gwf@gmail.com>
Avoid the junit publisher failures affecting other functions.
Fix #4540
Signed-off-by: Sophia Guo sophia.gwf@gmail.com