From 13de22e6a50d9f35bb01d397ed188cbd67de2757 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Tue, 10 Nov 2020 15:00:10 +0000 Subject: [PATCH] Publish only results included in current invocation to checks api - fixes #198 (#199) --- .../tasks/junit/JUnitResultArchiver.java | 4 +--- .../hudson/tasks/junit/TestResultAction.java | 13 ----------- .../junit/checks/JUnitChecksPublisher.java | 23 ++++++++++++------- .../checks/JUnitChecksPublisherTest.java | 10 ++++---- .../storage/TestResultStorageJunitTest.java | 2 +- 5 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java b/src/main/java/hudson/tasks/junit/JUnitResultArchiver.java index 2f1643419..18f2781a6 100644 --- 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 index 20c024e43..ca1e13d75 100644 --- 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 private @Nullable Integer totalCount; private Double healthScaleFactor; private List 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 index 70b5e774c..20e47c8b9 100644 --- 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,22 +26,26 @@ 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("Send us [feedback](https://github.com/jenkinsci/junit-plugin/issues)") @@ -46,7 +53,7 @@ ChecksDetails extractChecksDetails() { .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 failedTests = action.getResult().getFailedTests(); + List 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 index 48af9c6cf..4cb826bd6 100644 --- 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 index f490cbe06..ef78ea66c 100644 --- 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; {