Default checks name to be ' / '-joined enclosing blocks names (#211)

Co-authored-by: Tim Jacomb <[email protected]> Co-authored-by: Bill Collins <[email protected]>
jenkinsci · Dec 3, 2020 · 3c18200 · 3c18200
1 parent 19e951c
commit 3c18200
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 55 deletions.
diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java
@@ -28,6 +28,7 @@
 import hudson.Extension;
 import hudson.FilePath;
 import hudson.Launcher;
+import hudson.Util;
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractProject;
 import hudson.model.BuildListener;
@@ -42,24 +43,27 @@
 import hudson.tasks.Publisher;
 import hudson.tasks.Recorder;
 import hudson.tasks.junit.TestResultAction.Data;
-import io.jenkins.plugins.junit.checks.JUnitChecksPublisher;
-import io.jenkins.plugins.junit.storage.FileJunitTestResultStorage;
-import io.jenkins.plugins.junit.storage.JunitTestResultStorage;
 import hudson.tasks.test.PipelineTestDetails;
 import hudson.util.DescribableList;
 import hudson.util.FormValidation;
+import io.jenkins.plugins.junit.checks.JUnitChecksPublisher;
+import io.jenkins.plugins.junit.storage.FileJunitTestResultStorage;
+import io.jenkins.plugins.junit.storage.JunitTestResultStorage;
+import jenkins.tasks.SimpleBuildStep;
+import org.apache.commons.collections.iterators.ReverseListIterator;
+import org.apache.commons.lang.StringUtils;
 import org.apache.tools.ant.DirectoryScanner;
 import org.apache.tools.ant.types.FileSet;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.QueryParameter;
 
+import javax.annotation.Nonnull;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
-import javax.annotation.Nonnull;
-import jenkins.tasks.SimpleBuildStep;
-import org.kohsuke.stapler.DataBoundSetter;
 
 /**
  * Generates HTML report from JUnit test result XML files.
@@ -95,6 +99,8 @@ public class JUnitResultArchiver extends Recorder implements SimpleBuildStep, JU
     private boolean skipPublishingChecks;
     private String checksName;
 
+    private static final String DEFAULT_CHECKS_NAME = "Tests";
+
     @DataBoundConstructor
     public JUnitResultArchiver(String testResults) {
         this.testResults = testResults;
@@ -282,7 +288,19 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel
             }
 
             if (!task.isSkipPublishingChecks()) {
-                new JUnitChecksPublisher(build, task.getChecksName(), result, summary).publishChecks(listener);
+                // If we haven't been provided with a checks name, and we have pipeline test details, set the checks name
+                // to be a ' / '-joined string of the enclosing blocks names, plus 'Tests' at the start. If there are no
+                // enclosing blocks, you'll end up with just 'Tests'.
+                String checksName = task.getChecksName();
+                if (checksName == null && pipelineTestDetails != null) {
+                    List<String> checksComponents = new ArrayList<>(pipelineTestDetails.getEnclosingBlockNames());
+                    checksComponents.add(DEFAULT_CHECKS_NAME);
+                    checksName = StringUtils.join(new ReverseListIterator(checksComponents), " / ");
+                }
+                if (Util.fixEmpty(checksName) == null) {
+                    checksName = DEFAULT_CHECKS_NAME;
+                }
+                new JUnitChecksPublisher(build, checksName, result, summary).publishChecks(listener);
             }
 
             return summary;

diff --git a/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStep.java b/src/main/java/hudson/tasks/junit/pipeline/JUnitResultsStep.java
@@ -135,11 +135,7 @@ public void setSkipPublishingChecks(boolean skipPublishingChecks) {
 
     @Override
     public String getChecksName() {
-        if (Util.fixEmpty(checksName) == null) {
-            return "Tests";
-        }
-
-        return checksName;
+        return Util.fixEmpty(checksName);
     }
 
     @DataBoundSetter

diff --git a/src/main/java/io/jenkins/plugins/junit/checks/JUnitChecksPublisher.java b/src/main/java/io/jenkins/plugins/junit/checks/JUnitChecksPublisher.java
@@ -1,25 +1,24 @@
 package io.jenkins.plugins.junit.checks;
 
 import edu.hm.hafner.util.VisibleForTesting;
-import hudson.Util;
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import hudson.tasks.junit.CaseResult;
 import hudson.tasks.junit.TestResult;
-import hudson.tasks.junit.TestResultAction;
 import hudson.tasks.junit.TestResultSummary;
 import io.jenkins.plugins.checks.api.ChecksConclusion;
 import io.jenkins.plugins.checks.api.ChecksDetails;
 import io.jenkins.plugins.checks.api.ChecksOutput;
 import io.jenkins.plugins.checks.api.ChecksPublisher;
 import io.jenkins.plugins.checks.api.ChecksPublisherFactory;
 import io.jenkins.plugins.checks.api.ChecksStatus;
-import java.util.List;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.displayurlapi.DisplayURLProvider;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
+import java.util.List;
+
 @Restricted(NoExternalUse.class)
 public class JUnitChecksPublisher {
     public static final String SEPARATOR = ", ";
@@ -33,7 +32,7 @@ public class JUnitChecksPublisher {
 
     public JUnitChecksPublisher(final Run run, final String checksName, final TestResult result, final TestResultSummary summary) {
         this.run = run;
-        this.checksName = Util.fixEmpty(checksName) == null ? "Tests" : checksName;
+        this.checksName = checksName;
         this.result = result;
         this.summary = summary;
     }

diff --git a/src/main/resources/hudson/tasks/junit/JUnitResultArchiver/config.jelly b/src/main/resources/hudson/tasks/junit/JUnitResultArchiver/config.jelly
@@ -51,6 +51,6 @@ THE SOFTWARE.
         <f:checkbox title="${%If unchecked, then issues will be published to SCM provider platforms}" />
     </f:entry>
     <f:entry title="${%Checks name}" field="checksName">
-        <f:textbox default="Tests" />
+        <f:textbox />
     </f:entry>
 </j:jelly>
diff --git a/src/main/resources/hudson/tasks/junit/pipeline/JUnitResultsStep/help-checksName.html b/src/main/resources/hudson/tasks/junit/pipeline/JUnitResultsStep/help-checksName.html
@@ -1,4 +1,4 @@
 <div>
     If provided, and publishing checks enabled, the plugin will use this name when publishing results to corresponding
-    SCM hosting platforms. If not, a default of "Tests" will be used.
+    SCM hosting platforms. If not, a default including the current stage / branch names will be used.
 </div>
diff --git a/src/test/java/io/jenkins/plugins/junit/checks/JUnitChecksPublisherTest.java b/src/test/java/io/jenkins/plugins/junit/checks/JUnitChecksPublisherTest.java
@@ -1,30 +1,69 @@
 package io.jenkins.plugins.junit.checks;
 
+import hudson.ExtensionList;
 import hudson.FilePath;
+import hudson.model.Job;
 import hudson.model.Result;
-import hudson.tasks.junit.TestResultAction;
-import hudson.tasks.junit.TestResultSummary;
+import hudson.model.Run;
+import hudson.model.TaskListener;
 import hudson.tasks.junit.TestResultTest;
 import io.jenkins.plugins.checks.api.ChecksConclusion;
 import io.jenkins.plugins.checks.api.ChecksDetails;
 import io.jenkins.plugins.checks.api.ChecksOutput;
+import io.jenkins.plugins.checks.api.ChecksPublisher;
+import io.jenkins.plugins.checks.api.ChecksPublisherFactory;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
-import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.TestExtension;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
 
 import static java.util.Objects.requireNonNull;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
-import static org.junit.Assert.assertNotNull;
 
 public class JUnitChecksPublisherTest {
 
     @Rule
     public final JenkinsRule rule = new JenkinsRule();
 
+    static class InterceptingChecksPublisher extends ChecksPublisher {
+
+        final List<ChecksDetails> details = new ArrayList<>();
+
+        @Override
+        public void publish(ChecksDetails checksDetails) {
+            details.add(checksDetails);
+        }
+    }
+
+    @TestExtension
+    public static class InterceptingChecksPublisherFactory extends ChecksPublisherFactory {
+
+        InterceptingChecksPublisher publisher = new InterceptingChecksPublisher();
+
+        @Override
+        protected Optional<ChecksPublisher> createPublisher(Run<?, ?> run, TaskListener listener) {
+            return Optional.of(publisher);
+        }
+
+        @Override
+        protected Optional<ChecksPublisher> createPublisher(Job<?, ?> job, TaskListener listener) {
+            return Optional.of(publisher);
+        }
+    }
+
+    private ChecksDetails getDetail() {
+        List<ChecksDetails> details = ExtensionList.lookupSingleton(InterceptingChecksPublisherFactory.class).publisher.details;
+        assertThat(details.size(), is(1));
+        return details.get(0);
+    }
+
     @Test
     @SuppressWarnings("OptionalGetWithoutIsPresent")
     public void extractChecksDetailsPassingTestResults() throws Exception {
@@ -39,21 +78,18 @@ public void extractChecksDetailsPassingTestResults() throws Exception {
         FilePath testFile = requireNonNull(ws).child("test-result.xml");
         testFile.copyFrom(TestResultTest.class.getResource("junit-report-1463.xml"));
 
-        WorkflowRun r = rule.buildAndAssertSuccess(j);
-        TestResultAction action = r.getAction(TestResultAction.class);
-        assertNotNull(action);
+        rule.buildAndAssertSuccess(j);
 
-        TestResultSummary summary = new TestResultSummary(0, 0, 6, 6);
-        JUnitChecksPublisher publisher = new JUnitChecksPublisher(r, null, action.getResult(), summary);
-        ChecksDetails checksDetails = publisher.extractChecksDetails();
+        ChecksDetails checksDetails = getDetail();
 
         assertThat(checksDetails.getConclusion(), is(ChecksConclusion.SUCCESS));
-        assertThat(checksDetails.getName().get(), is("Tests"));
+        assertThat(checksDetails.getName().get(), is("Tests / first"));
 
         ChecksOutput output = checksDetails.getOutput().get();
 
         assertThat(output.getTitle().get(), is("passed: 6"));
         assertThat(output.getText().get(), is(""));
+
     }
 
     @Test
@@ -70,20 +106,17 @@ public void extractChecksDetailsFailingSingleTestResult() throws Exception {
         FilePath testFile = requireNonNull(ws).child("test-result.xml");
         testFile.copyFrom(TestResultTest.class.getResource("junit-report-errror-details.xml"));
 
-        WorkflowRun r = rule.buildAndAssertStatus(Result.FAILURE, j);
-        TestResultAction action = r.getAction(TestResultAction.class);
-        assertNotNull(action);
+        rule.buildAndAssertStatus(Result.FAILURE, j);
 
-        TestResultSummary summary = new TestResultSummary(1, 0, 1, 2);
-        JUnitChecksPublisher publisher = new JUnitChecksPublisher(r, "", action.getResult(), summary);
-        ChecksDetails checksDetails = publisher.extractChecksDetails();
+        ChecksDetails checksDetails = getDetail();
 
         assertThat(checksDetails.getConclusion(), is(ChecksConclusion.FAILURE));
-        assertThat(checksDetails.getName().get(), is("Tests"));
+        assertThat(checksDetails.getName().get(), is("Tests / first"));
 
         ChecksOutput output = checksDetails.getOutput().get();
 
         assertThat(output.getTitle().get(), is("some.package.somewhere.WhooHoo.testHudsonReporting failed"));
+
     }
 
     @Test
@@ -100,47 +133,80 @@ public void extractChecksDetailsFailingMultipleTests() throws Exception {
         FilePath testFile = requireNonNull(ws).child("test-result.xml");
         testFile.copyFrom(TestResultTest.class.getResource("junit-report-20090516.xml"));
 
-        WorkflowRun r = rule.buildAndAssertStatus(Result.FAILURE, j);
-        TestResultAction action = r.getAction(TestResultAction.class);
-        assertNotNull(action);
+        rule.buildAndAssertStatus(Result.FAILURE, j);
 
-        TestResultSummary summary = new TestResultSummary(3, 0, 5, 8);
-        JUnitChecksPublisher publisher = new JUnitChecksPublisher(r, "Tests", action.getResult(), summary);
-        ChecksDetails checksDetails = publisher.extractChecksDetails();
+        ChecksDetails checksDetails = getDetail();
 
         assertThat(checksDetails.getConclusion(), is(ChecksConclusion.FAILURE));
-        assertThat(checksDetails.getName().get(), is("Tests"));
+        assertThat(checksDetails.getName().get(), is("Tests / first"));
 
         ChecksOutput output = checksDetails.getOutput().get();
 
         assertThat(output.getTitle().get(), is("failed: 3, passed: 5"));
+
     }
 
     @Test
     @SuppressWarnings("OptionalGetWithoutIsPresent")
-    public void setCustomCheckName() throws Exception {
+    public void extractChecksDetailsCustomCheckName() throws Exception {
         WorkflowJob j = rule.jenkins.createProject(WorkflowJob.class, "singleStep");
         j.setDefinition(new CpsFlowDefinition("stage('first') {\n" +
-              "  node {\n" +
-              "    def results = junit(testResults: '*.xml', checksName: 'Custom Checks Name')\n" +
-              "    assert results.totalCount == 6\n" +
-              "  }\n" +
-              "}\n", true));
+                "  node {\n" +
+                "    def results = junit(testResults: '*.xml', checksName: 'Custom Checks Name')\n" +
+                "    assert results.totalCount == 6\n" +
+                "  }\n" +
+                "}\n", true));
         FilePath ws = rule.jenkins.getWorkspaceFor(j);
         FilePath testFile = requireNonNull(ws).child("test-result.xml");
         testFile.copyFrom(TestResultTest.class.getResource("junit-report-1463.xml"));
 
-        WorkflowRun r = rule.buildAndAssertSuccess(j);
-        TestResultAction action = r.getAction(TestResultAction.class);
-        assertNotNull(action);
+        rule.buildAndAssertSuccess(j);
 
-        TestResultSummary summary = new TestResultSummary(0, 0, 6, 6);
-        JUnitChecksPublisher publisher = new JUnitChecksPublisher(r, "Custom Checks Name", action.getResult(), summary);
-        ChecksDetails checksDetails = publisher.extractChecksDetails();
+        ChecksDetails checksDetails = getDetail();
 
         assertThat(checksDetails.getConclusion(), is(ChecksConclusion.SUCCESS));
 
         assertThat(checksDetails.getName().get(), is("Custom Checks Name"));
+    }
+
+
+    @Test
+    @SuppressWarnings("OptionalGetWithoutIsPresent")
+    public void extractChecksDetailsNoStageContext() throws Exception {
+        WorkflowJob j = rule.jenkins.createProject(WorkflowJob.class, "singleStep");
+        j.setDefinition(new CpsFlowDefinition("node {\n" +
+                "  def results = junit(testResults: '*.xml')\n" +
+                "  assert results.totalCount == 6\n" +
+                "}\n", true));
+        FilePath ws = rule.jenkins.getWorkspaceFor(j);
+        FilePath testFile = requireNonNull(ws).child("test-result.xml");
+        testFile.copyFrom(TestResultTest.class.getResource("junit-report-1463.xml"));
+
+        rule.buildAndAssertSuccess(j);
+
+        ChecksDetails checksDetails = getDetail();
+
+        assertThat(checksDetails.getName().get(), is("Tests"));
+    }
+
+    @Test
+    @SuppressWarnings("OptionalGetWithoutIsPresent")
+    public void extractChecksDetailsNestedStages() throws Exception {
+        WorkflowJob j = rule.jenkins.createProject(WorkflowJob.class, "singleStep");
+        j.setDefinition(new CpsFlowDefinition("stage('first') { stage('second') {\n" +
+                "  node {\n" +
+                "    def results = junit(testResults: '*.xml')\n" +
+                "    assert results.totalCount == 6\n" +
+                "  }\n" +
+                "}}\n", true));
+        FilePath ws = rule.jenkins.getWorkspaceFor(j);
+        FilePath testFile = requireNonNull(ws).child("test-result.xml");
+        testFile.copyFrom(TestResultTest.class.getResource("junit-report-1463.xml"));
+
+        rule.buildAndAssertSuccess(j);
+
+        ChecksDetails checksDetails = getDetail();
 
+        assertThat(checksDetails.getName().get(), is("Tests / first / second"));
     }
 }