Publish only results included in current invocation to checks api - f…

…ixes #198 (#199)
jenkinsci · Nov 10, 2020 · 13de22e · 13de22e
1 parent 1a5ecd7
commit 13de22e
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 29 deletions.
diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java
@@ -186,7 +186,6 @@ public static TestResultAction parseAndAttach(@Nonnull JUnitTask task, PipelineT
                 action.mergeResult(result, listener);
             }
             action.setHealthScaleFactor(task.getHealthScaleFactor()); // overwrites previous value if appending
-            action.setChecksName(task.getChecksName());
             if (result.isEmpty()) {
                 if (build.getResult() == Result.FAILURE) {
                     // most likely a build failed before it gets to the test phase.
@@ -262,7 +261,6 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel
                 summary = new TestResultSummary(result);
             }
             action.setHealthScaleFactor(task.getHealthScaleFactor()); // overwrites previous value if appending
-            action.setChecksName(task.getChecksName());
             if (summary.getTotalCount() == 0 && /* maybe a secondary effect */ build.getResult() != Result.FAILURE) {
                 assert task.isAllowEmptyResults();
                 listener.getLogger().println(Messages.JUnitResultArchiver_ResultIsEmpty());
@@ -284,7 +282,7 @@ public static TestResultSummary parseAndSummarize(@Nonnull JUnitTask task, Pipel
             }
 
             if (!task.isSkipPublishingChecks()) {
-                new JUnitChecksPublisher(action, summary).publishChecks(listener);
+                new JUnitChecksPublisher(build, task.getChecksName(), result, summary).publishChecks(listener);
             }
 
             return summary;

diff --git a/src/main/java/hudson/tasks/junit/TestResultAction.java b/src/main/java/hudson/tasks/junit/TestResultAction.java
@@ -75,7 +75,6 @@ public class TestResultAction extends AbstractTestResultAction<TestResultAction>
     private @Nullable Integer totalCount;
     private Double healthScaleFactor;
     private List<Data> testData = new ArrayList<>();
-    private String checksName;
 
     @Deprecated
     public TestResultAction(AbstractBuild owner, TestResult result, BuildListener listener) {
@@ -287,18 +286,6 @@ public void addData(Data data) {
         }
     }
 
-    public String getChecksName() {
-        if (Util.fixEmpty(checksName) == null) {
-            return "Tests";
-        }
-
-        return checksName;
-    }
-
-    public void setChecksName(String checksName) {
-        this.checksName = checksName;
-    }
-
     /**
      * Merges an additional test result into this one.
      */

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,8 +1,11 @@
 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;
@@ -23,30 +26,34 @@ public class JUnitChecksPublisher {
 
     // cap to avoid hitting check API message limit
     private static final int MAX_MSG_SIZE_TO_CHECKS_API = 65535;
-    private final TestResultAction action;
+    private final Run run;
+    private final String checksName;
+    private final TestResult result;
     private final TestResultSummary summary;
 
-    public JUnitChecksPublisher(final TestResultAction action, final TestResultSummary summary) {
-        this.action = action;
+    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.result = result;
         this.summary = summary;
     }
 
     public void publishChecks(TaskListener listener) {
-        ChecksPublisher publisher = ChecksPublisherFactory.fromRun(action.run, listener);
+        ChecksPublisher publisher = ChecksPublisherFactory.fromRun(run, listener);
         publisher.publish(extractChecksDetails());
     }
 
     @VisibleForTesting
     ChecksDetails extractChecksDetails() {
-        String testsURL = DisplayURLProvider.get().getTestsURL(action.run);
+        String testsURL = DisplayURLProvider.get().getTestsURL(run);
         ChecksOutput output = new ChecksOutput.ChecksOutputBuilder()
                 .withTitle(extractChecksTitle())
                 .withSummary("<sub>Send us [feedback](https://github.com/jenkinsci/junit-plugin/issues)")
                 .withText(extractChecksText(testsURL))
                 .build();
 
         return new ChecksDetails.ChecksDetailsBuilder()
-                .withName(action.getChecksName())
+                .withName(checksName)
                 .withStatus(ChecksStatus.COMPLETED)
                 .withConclusion(summary.getFailCount() > 0 ? ChecksConclusion.FAILURE : ChecksConclusion.SUCCESS)
                 .withDetailsURL(testsURL)
@@ -57,7 +64,7 @@ ChecksDetails extractChecksDetails() {
     private String extractChecksText(String testsURL) {
         StringBuilder builder = new StringBuilder();
         if (summary.getFailCount() > 0) {
-            List<CaseResult> failedTests = action.getResult().getFailedTests();
+            List<CaseResult> failedTests = result.getFailedTests();
 
             for (CaseResult failedTest: failedTests) {
                 String testReport = mapFailedTestToTestReport(failedTest);
@@ -114,7 +121,7 @@ private String extractChecksTitle() {
         StringBuilder builder = new StringBuilder();
 
         if (summary.getFailCount() == 1) {
-            CaseResult failedTest = action.getResult().getFailedTests().get(0);
+            CaseResult failedTest = result.getFailedTests().get(0);
             builder.append(failedTest.getTransformedFullDisplayName()).append(" failed");
             return builder.toString();
         }

diff --git a/src/test/java/io/jenkins/plugins/junit/checks/JUnitChecksPublisherTest.java b/src/test/java/io/jenkins/plugins/junit/checks/JUnitChecksPublisherTest.java
@@ -44,7 +44,7 @@ public void extractChecksDetailsPassingTestResults() throws Exception {
         assertNotNull(action);
 
         TestResultSummary summary = new TestResultSummary(0, 0, 6, 6);
-        JUnitChecksPublisher publisher = new JUnitChecksPublisher(action, summary);
+        JUnitChecksPublisher publisher = new JUnitChecksPublisher(r, null, action.getResult(), summary);
         ChecksDetails checksDetails = publisher.extractChecksDetails();
 
         assertThat(checksDetails.getConclusion(), is(ChecksConclusion.SUCCESS));
@@ -75,10 +75,11 @@ public void extractChecksDetailsFailingSingleTestResult() throws Exception {
         assertNotNull(action);
 
         TestResultSummary summary = new TestResultSummary(1, 0, 1, 2);
-        JUnitChecksPublisher publisher = new JUnitChecksPublisher(action, summary);
+        JUnitChecksPublisher publisher = new JUnitChecksPublisher(r, "", action.getResult(), summary);
         ChecksDetails checksDetails = publisher.extractChecksDetails();
 
         assertThat(checksDetails.getConclusion(), is(ChecksConclusion.FAILURE));
+        assertThat(checksDetails.getName().get(), is("Tests"));
 
         ChecksOutput output = checksDetails.getOutput().get();
 
@@ -104,10 +105,11 @@ public void extractChecksDetailsFailingMultipleTests() throws Exception {
         assertNotNull(action);
 
         TestResultSummary summary = new TestResultSummary(3, 0, 5, 8);
-        JUnitChecksPublisher publisher = new JUnitChecksPublisher(action, summary);
+        JUnitChecksPublisher publisher = new JUnitChecksPublisher(r, "Tests", action.getResult(), summary);
         ChecksDetails checksDetails = publisher.extractChecksDetails();
 
         assertThat(checksDetails.getConclusion(), is(ChecksConclusion.FAILURE));
+        assertThat(checksDetails.getName().get(), is("Tests"));
 
         ChecksOutput output = checksDetails.getOutput().get();
 
@@ -133,7 +135,7 @@ public void setCustomCheckName() throws Exception {
         assertNotNull(action);
 
         TestResultSummary summary = new TestResultSummary(0, 0, 6, 6);
-        JUnitChecksPublisher publisher = new JUnitChecksPublisher(action, summary);
+        JUnitChecksPublisher publisher = new JUnitChecksPublisher(r, "Custom Checks Name", action.getResult(), summary);
         ChecksDetails checksDetails = publisher.extractChecksDetails();
 
         assertThat(checksDetails.getConclusion(), is(ChecksConclusion.SUCCESS));

diff --git a/src/test/java/io/jenkins/plugins/junit/storage/TestResultStorageJunitTest.java b/src/test/java/io/jenkins/plugins/junit/storage/TestResultStorageJunitTest.java
@@ -157,7 +157,7 @@ public class TestResultStorageJunitTest {
                     childNames.add(((Element) item).getTagName());
                 }
             }
-            assertEquals(buildXml, ImmutableSet.of("checksName", "healthScaleFactor", "testData", "descriptions"), childNames);
+            assertEquals(buildXml, ImmutableSet.of("healthScaleFactor", "testData", "descriptions"), childNames);
         }
         Impl.queriesPermitted = true;
         {