-
Notifications
You must be signed in to change notification settings - Fork 13
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
Pass task listener to JUnit parsing #18
Conversation
@@ -42,13 +52,21 @@ | |||
private final String workspace; | |||
private final boolean keepLongStdio; | |||
private final String glob; | |||
private final StepContext context; |
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.
Just make sure this is not serialized to build.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.
Does not appear to be:
<org.jenkinsci.plugins.junitrealtimetestreporter.PipelineRealtimeTestResultAction plugin="junit-realtime-test-reporter@999999-SNAPSHOT">
<descriptions class="concurrent-hash-map"/>
<id>14</id>
<node></node>
<workspace>/Users/timja/code/jenkins/junit-realtime-test-reporter-plugin/work/workspace/junit-attachments-test_PR-8@2</workspace>
<keepLongStdio>false</keepLongStdio>
<glob>**/target/surefire-reports/TEST-*.xml</glob>
</org.jenkinsci.plugins.junitrealtimetestreporter.PipelineRealtimeTestResultAction>
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.
Hmm, but PipelineRealtimeTestResultAction
is? Then this looks dangerous. Suggest making it transient
and studying the conditions under which it might be saved.
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.
Hmm, but PipelineRealtimeTestResultAction
is? Then this looks dangerous. Suggest making it transient
and studying the conditions under which it might be saved.
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.
Javadoc says it's safe to save it? https://javadoc.jenkins.io/plugin/workflow-step-api/org/jenkinsci/plugins/workflow/steps/StepContext.html
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.
Empty results, the Junit plugin assumes it gets a non null task listener and it logs something when there's no results I think (although probably isn't annotating the methods as it's not really expected to have these methods called outside of the plugin).
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 Launcher
is never used. The TaskListener
you could perhaps replace with https://javadoc.jenkins.io/plugin/workflow-api/org/jenkinsci/plugins/workflow/flow/FlowExecutionOwner.html#getListener--; not ideal but it is only used in this corner case anyway.
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'll take a look tomorrow, how do I get to the FlowExecutionOwner
?
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.
checkcast Run
to FEO.Executable
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.
See #27
Fixes #13