Add attachments from the junit-attachments plugin into Jira attachments #144

imonteroperez · 2024-04-19T16:01:01Z

Highlights

Before this PR the attachments added to the Jira tickets are always the stderr, stdout and error stacktrace and details
After this PR, if the test result has additional attachments provided by third-party tools (like the junit-attachments publisher) these would be also added to the Jira ticket.

Testing done

Tested with Jira and Jenkins locally and working as expected
Jenkins attachment

Jira attachment (contains the one from the junit-attachment in addition to the current ones - stderr, stdout, etc.)

Submitter checklist

Give feedback

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue
Options

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java

…aTestDataPublisher.java

cathychan

IMO this should be an opt-in feature. If I have some builds that have massive test attachments, I may not want to include them in the Jira tickets.

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraUtils.java

…aUtils.java Co-authored-by: Cathy Chan <[email protected]>

imonteroperez · 2024-04-22T19:17:05Z

IMO this should be an opt-in feature. If I have some builds that have massive test attachments, I may not want to include them in the Jira tickets.

LGTM, will add a configuration option

imonteroperez · 2024-04-23T07:34:45Z

IMO this should be an opt-in feature. If I have some builds that have massive test attachments, I may not want to include them in the Jira tickets.

LGTM, will add a configuration option

Added a configuration option via environment variable on ce6f0c7. I discarded to include an option to the step to keep it simple and keep backward compatibility. Also, the step options are there to specify the behavior (raise, resolve, etc.) but here we want a specific additional operation on raising Jira issues, so that is the reason why I decided to go through an env var.

Also documented in 2353033

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java

…aTestDataPublisher.java

jglick · 2024-04-23T11:06:07Z

README.md

+You can use additional environment variables to define additional configuration needs:
+
+ * `JIRA_INCLUDE_JUNIT_ATTACHMENTS`: it will include as attachments of a Jira issue additional files from [junit-attachments](https://github.com/jenkinsci/junit-attachments-plugin) plugin. Only on raising new issues.


Why an environment variable and not just a regular boolean data bound setter on the publisher?

At the very beginning considered this #144 (comment) but I'm reconsidering this option and return to the boolean data bound setter on the publisher

All configuration of Pipeline steps, or structs used inside steps (this case), should be via regular data binding. It should be self-documenting as well, using config.jelly. See docs.

Done in a384e3c

jglick · 2024-04-23T11:10:51Z

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java

@@ -199,6 +225,10 @@ public TestResultAction.Data contributeTestData(Run<?, ?> run, @Nonnull FilePath
        }

        if(JobConfigMapping.getInstance().getAutoRaiseIssue(project)) {
+            if (Boolean.getBoolean(System.getenv(JIRA_INCLUDE_JUNIT_ATTACHMENTS))) {
+                GetTestDataMethodObject methodObject = new GetTestDataMethodObject(run, workspace, launcher, listener, testResult);


TIL: https://github.com/jenkinsci/junit-attachments-plugin/blob/7ca5b7a98b1c215fcd2aa21a1510c03f1b488466/src/main/java/hudson/plugins/junitattachments/GetTestDataMethodObject.java#L31-L32 does not seem to accurately describe what that class does at all.

It retrieves all the attachments per test class and method

jglick · 2024-04-23T11:12:31Z

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java

@@ -178,7 +204,7 @@ public TestResultAction.Data contributeTestData(Run<?, ?> run, @Nonnull FilePath
                                                    throws IOException, InterruptedException {

 	EnvVars envVars = run.getEnvironment(listener);


This gets the top-level environment for the build. withEnv does not affect it.

You are right, actually, I'm testing this right now and detected this issue. TBH rethinking on to setup a boolean data bound setter better than this.

Addressed in a384e3c

jglick · 2024-04-23T19:12:35Z

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java

@@ -130,7 +153,8 @@ private JobConfigMapping.JobConfigEntry getJobConfig() {
     */
 	@DataBoundConstructor
 	public JiraTestDataPublisher(List<AbstractFields> configs, String projectKey, String issueType,
-                                 boolean autoRaiseIssue, boolean autoResolveIssue, boolean autoUnlinkIssue, boolean overrideResolvedIssues) {
+                                 boolean autoRaiseIssue, boolean autoResolveIssue, boolean autoUnlinkIssue, boolean overrideResolvedIssues, 
+                                 boolean additionalAttachments) {


Better to use @DataBoundSetter. https://www.jenkins.io/doc/developer/plugin-development/pipeline-integration/#constructor-vs-setters

I will do this refactoring outside the scope of this PR. Not sure which parameters can be delegated to a setter given documentation said this is for optional ones.

Created #145 to track this request

Well the new option you just added is clearly optional and so it should be added via a @DataBoundSetter. Refactoring existing options is another matter, but you should never be adding parameters to an existing @DataBoundConstructor.

Addresed for the new optional field in d96ed87

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java

jglick · 2024-04-25T12:48:26Z

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java

+    @DataBoundSetter
+    public void setAdditionalAttachments(boolean additionalAttachments) {


This seems to be missing the matching getter, which is required. (Do not recall offhand whether that will break the snippet generator, but it is prudent to test that too.)

Addressed on 86b3679

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java

…aTestDataPublisher.java Co-authored-by: Jesse Glick <[email protected]>

jglick

Looks OK.

imonteroperez requested review from cathychan and kerogers-cloudbees April 19, 2024 16:01

imonteroperez force-pushed the additional-attachments branch 3 times, most recently from 28922e1 to 619dea9 Compare April 21, 2024 19:24

Add attachments from the junit-attachments plugin into Jira attachments

d06f9e6

imonteroperez force-pushed the additional-attachments branch from 619dea9 to d06f9e6 Compare April 21, 2024 19:26

imonteroperez marked this pull request as ready for review April 21, 2024 19:32

imonteroperez commented Apr 22, 2024

View reviewed changes

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java Outdated Show resolved Hide resolved

Update src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/Jir…

f4b5f72

…aTestDataPublisher.java

cathychan reviewed Apr 22, 2024

View reviewed changes

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraUtils.java Outdated Show resolved Hide resolved

Update src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/Jir…

de6c151

…aUtils.java Co-authored-by: Cathy Chan <[email protected]>

Configure the inclusion of attachments via EnvVars

ce6f0c7

Incorporate a description of the env var in README

2353033

imonteroperez requested a review from cathychan April 23, 2024 07:38

imonteroperez commented Apr 23, 2024

View reviewed changes

src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/JiraTestDataPublisher.java Outdated Show resolved Hide resolved

imonteroperez and others added 3 commits April 23, 2024 09:40

Update src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/Jir…

e148115

…aTestDataPublisher.java

Use system.getenv instead of envvars

788ce6f

Return to use envvars

ac3cb84

jglick suggested changes Apr 23, 2024

View reviewed changes

imonteroperez marked this pull request as draft April 23, 2024 11:15

Use databound setter instead of env var

a384e3c

imonteroperez requested a review from jglick April 23, 2024 11:42

imonteroperez marked this pull request as ready for review April 23, 2024 11:43

jglick reviewed Apr 23, 2024

View reviewed changes

imonteroperez mentioned this pull request Apr 24, 2024

Use @DataBoundSetter for the JiraTestDataPublisher constructor #145

Open

imonteroperez requested a review from jglick April 24, 2024 08:12

Use DataBoundSetter for additionalAttachments

d96ed87

jglick reviewed Apr 25, 2024

View reviewed changes

imonteroperez and others added 3 commits April 25, 2024 15:05

Update src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/Jir…

68277c6

…aTestDataPublisher.java Co-authored-by: Jesse Glick <[email protected]>

Update src/main/java/org/jenkinsci/plugins/JiraTestResultReporter/Jir…

b27b7a3

…aTestDataPublisher.java Co-authored-by: Jesse Glick <[email protected]>

Add getter

86b3679

imonteroperez requested a review from jglick April 25, 2024 14:08

jglick approved these changes Apr 25, 2024

View reviewed changes

imonteroperez merged commit 13ba2d0 into jenkinsci:master Apr 26, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add attachments from the junit-attachments plugin into Jira attachments #144

Add attachments from the junit-attachments plugin into Jira attachments #144

imonteroperez commented Apr 19, 2024 •

edited

Loading

Submitter checklist

cathychan left a comment

imonteroperez commented Apr 22, 2024

imonteroperez commented Apr 23, 2024 •

edited

Loading

jglick Apr 23, 2024

imonteroperez Apr 23, 2024

jglick Apr 23, 2024

imonteroperez Apr 23, 2024

jglick Apr 23, 2024

imonteroperez Apr 23, 2024

jglick Apr 23, 2024

imonteroperez Apr 23, 2024

imonteroperez Apr 23, 2024

jglick Apr 23, 2024

imonteroperez Apr 24, 2024

imonteroperez Apr 24, 2024

jglick Apr 24, 2024

imonteroperez Apr 25, 2024

jglick Apr 25, 2024

imonteroperez Apr 25, 2024

jglick left a comment

		You can use additional environment variables to define additional configuration needs:

		* `JIRA_INCLUDE_JUNIT_ATTACHMENTS`: it will include as attachments of a Jira issue additional files from [junit-attachments](https://github.com/jenkinsci/junit-attachments-plugin) plugin. Only on raising new issues.

		@@ -178,7 +204,7 @@ public TestResultAction.Data contributeTestData(Run<?, ?> run, @Nonnull FilePath
		throws IOException, InterruptedException {

		EnvVars envVars = run.getEnvironment(listener);

		@DataBoundSetter
		public void setAdditionalAttachments(boolean additionalAttachments) {

Add attachments from the junit-attachments plugin into Jira attachments #144

Add attachments from the junit-attachments plugin into Jira attachments #144

Conversation

imonteroperez commented Apr 19, 2024 • edited Loading

Highlights

Testing done

Submitter checklist

cathychan left a comment

Choose a reason for hiding this comment

imonteroperez commented Apr 22, 2024

imonteroperez commented Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jglick left a comment

Choose a reason for hiding this comment

imonteroperez commented Apr 19, 2024 •

edited

Loading

imonteroperez commented Apr 23, 2024 •

edited

Loading