From f34574d63bea4896ca0b95ad689af5d9d67816b5 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sat, 26 Dec 2020 17:45:09 +0000 Subject: [PATCH 1/7] Add test for withChecks support --- plugin/pom.xml | 16 ++++++++++ .../steps/WarningChecksPublisherITest.java | 29 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/plugin/pom.xml b/plugin/pom.xml index ed73ef5318..9180d1119c 100644 --- a/plugin/pom.xml +++ b/plugin/pom.xml @@ -80,6 +80,7 @@ 1.10 6.15 2.6.3 + 1.2.0 -Djava.awt.headless=true -Xmx1024m -Djenkins.test.timeout=1000 @@ -293,6 +294,7 @@ io.jenkins.plugins checks-api + ${checks-api.version} com.github.spotbugs @@ -392,6 +394,20 @@ test-jar + + io.jenkins.plugins + checks-api + ${checks-api.version} + test + test-jar + + + com.github.spotbugs + spotbugs-annotations + + + + org.jenkins-ci.plugins diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java index de4e01517f..a74ff308a5 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java @@ -1,9 +1,13 @@ package io.jenkins.plugins.analysis.core.steps; +import java.util.List; import java.util.Optional; import java.util.function.Consumer; +import io.jenkins.plugins.checks.util.CapturingChecksPublisher; import org.apache.commons.lang3.StringUtils; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.junit.After; import org.junit.Test; import com.google.errorprone.annotations.CanIgnoreReturnValue; @@ -27,6 +31,7 @@ import io.jenkins.plugins.checks.api.ChecksOutput; import io.jenkins.plugins.checks.api.ChecksOutput.ChecksOutputBuilder; import io.jenkins.plugins.checks.api.ChecksStatus; +import org.jvnet.hudson.test.TestExtension; import static io.jenkins.plugins.analysis.core.assertions.Assertions.*; @@ -39,6 +44,15 @@ public class WarningChecksPublisherITest extends IntegrationTestWithJenkinsPerSu private static final String OLD_CHECKSTYLE_REPORT = "checkstyle.xml"; private static final String NEW_CHECKSTYLE_REPORT = "checkstyle1.xml"; + @TestExtension + public static final CapturingChecksPublisher.Factory PUBLISHER_FACTORY = new CapturingChecksPublisher.Factory(); + + @After + public void clearPublisher() { + PUBLISHER_FACTORY.getPublishedChecks().clear(); + } + + /** * Verifies that {@link WarningChecksPublisher} constructs the {@link ChecksDetails} correctly * with only new issues. @@ -189,6 +203,21 @@ public void shouldIgnoreColumnsWhenBuildMultipleLineAnnotation() { .hasFieldOrPropertyWithValue("endColumn", Optional.empty()); } + @Test + public void shouldHonorWithChecksContext() { + WorkflowJob project = createPipeline(); + copySingleFileToWorkspace(project, NEW_CHECKSTYLE_REPORT); + project.setDefinition(asStage("withChecks('Custom Checks Name') {", createScanForIssuesStep(new CheckStyle()), PUBLISH_ISSUES_STEP, "}")); + buildSuccessfully(project); + + List publishedChecks = PUBLISHER_FACTORY.getPublishedChecks(); + + assertThat(publishedChecks.size()).isEqualTo(2); + + publishedChecks.forEach(check -> assertThat(check.getName()).isPresent().get().isEqualTo("Custom Checks Name")); + + } + private ChecksDetails createExpectedCheckStyleDetails() { ChecksDetailsBuilder builder = new ChecksDetailsBuilder() .withName("CheckStyle") From c37169cd730d8e5853a4ee04d388ade4f6a506cd Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sat, 26 Dec 2020 17:49:37 +0000 Subject: [PATCH 2/7] Support withChecks --- .../plugins/analysis/core/steps/PublishIssuesStep.java | 5 ++++- .../analysis/core/steps/WarningChecksPublisher.java | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/PublishIssuesStep.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/PublishIssuesStep.java index 506929c936..42d92a7257 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/PublishIssuesStep.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/PublishIssuesStep.java @@ -4,8 +4,10 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.Set; +import io.jenkins.plugins.checks.steps.ChecksInfo; import org.apache.commons.lang3.StringUtils; import org.eclipse.collections.impl.factory.Sets; @@ -855,7 +857,8 @@ protected ResultAction run() throws IOException, InterruptedException, IllegalSt ResultAction action = publisher.attachAction(step.getTrendChartType()); if (!step.isSkipPublishingChecks()) { - WarningChecksPublisher checksPublisher = new WarningChecksPublisher(action, getTaskListener()); + String checksName = Optional.ofNullable(getContext().get(ChecksInfo.class)).map(ChecksInfo::getName).orElse(null); + WarningChecksPublisher checksPublisher = new WarningChecksPublisher(action, getTaskListener(), checksName); checksPublisher.publishChecks(); } diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java index 64a6d2a316..e52fec01e0 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java @@ -40,10 +40,16 @@ class WarningChecksPublisher { private final ResultAction action; private final TaskListener listener; + private final String checksName; - WarningChecksPublisher(final ResultAction action, final TaskListener listener) { + WarningChecksPublisher(ResultAction action, TaskListener listener, String checksName) { this.action = action; this.listener = listener; + this.checksName = checksName; + } + + WarningChecksPublisher(final ResultAction action, final TaskListener listener) { + this(action, listener, null); } /** @@ -63,7 +69,7 @@ ChecksDetails extractChecksDetails() { StaticAnalysisLabelProvider labelProvider = action.getLabelProvider(); return new ChecksDetailsBuilder() - .withName(labelProvider.getName()) + .withName(StringUtils.defaultIfEmpty(checksName, labelProvider.getName())) .withStatus(ChecksStatus.COMPLETED) .withConclusion(extractChecksConclusion(result.getQualityGateStatus())) .withOutput(new ChecksOutputBuilder() From 4110debe6ab19e76c76ee0fa671a61584b77082a Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sat, 26 Dec 2020 18:01:07 +0000 Subject: [PATCH 3/7] Update readme --- doc/Documentation.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/Documentation.md b/doc/Documentation.md index ce876988fd..0d828a71ca 100644 --- a/doc/Documentation.md +++ b/doc/Documentation.md @@ -1055,6 +1055,13 @@ In order to disable the checks feature, set the property `skipPublishingChecks` recordIssues skipPublishingChecks: true, tool: java(pattern: '*.log') ``` +The publisher will respect a `withChecks` context: +```groovy +withChecks('My Custom Checks Name') { + recordIssues tool: java(pattern: '*.log') +} +``` + ## Transition from the static analysis suite Previously the same set of features has been provided by the plugins of the static analysis suite From 57703098d897573102f94326972028f36e26ae91 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Sat, 26 Dec 2020 19:21:49 +0000 Subject: [PATCH 4/7] Satisfy linters --- .../analysis/core/steps/WarningChecksPublisher.java | 2 +- .../analysis/core/steps/WarningChecksPublisherITest.java | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java index e52fec01e0..770f01364d 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java @@ -42,7 +42,7 @@ class WarningChecksPublisher { private final TaskListener listener; private final String checksName; - WarningChecksPublisher(ResultAction action, TaskListener listener, String checksName) { + WarningChecksPublisher(final ResultAction action, final TaskListener listener, final String checksName) { this.action = action; this.listener = listener; this.checksName = checksName; diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java index a74ff308a5..720a0c4dc2 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java @@ -44,9 +44,15 @@ public class WarningChecksPublisherITest extends IntegrationTestWithJenkinsPerSu private static final String OLD_CHECKSTYLE_REPORT = "checkstyle.xml"; private static final String NEW_CHECKSTYLE_REPORT = "checkstyle1.xml"; + /** + * Capturing checks publisher for inspection of checks created during a run. + */ @TestExtension public static final CapturingChecksPublisher.Factory PUBLISHER_FACTORY = new CapturingChecksPublisher.Factory(); + /** + * Resets captured checks after each test. + */ @After public void clearPublisher() { PUBLISHER_FACTORY.getPublishedChecks().clear(); @@ -203,6 +209,9 @@ public void shouldIgnoreColumnsWhenBuildMultipleLineAnnotation() { .hasFieldOrPropertyWithValue("endColumn", Optional.empty()); } + /** + * Test that publishIssues honors the checks name provided by a withChecks context. + */ @Test public void shouldHonorWithChecksContext() { WorkflowJob project = createPipeline(); From 36ee313dbded35e6bc1a3b016d454a3240339929 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Mon, 28 Dec 2020 18:15:04 +0000 Subject: [PATCH 5/7] Pass around checksInfo, not checksName; support recordIssues --- .../analysis/core/steps/IssuesRecorder.java | 9 ++++++++- .../analysis/core/steps/PublishIssuesStep.java | 3 +-- .../analysis/core/steps/RecordIssuesStep.java | 2 ++ .../core/steps/WarningChecksPublisher.java | 18 ++++++++++-------- .../steps/WarningChecksPublisherITest.java | 16 ++++++++-------- 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java index 0708ad090b..8f43278847 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java @@ -57,6 +57,7 @@ import io.jenkins.plugins.analysis.core.util.RunResultHandler; import io.jenkins.plugins.analysis.core.util.StageResultHandler; import io.jenkins.plugins.analysis.core.util.TrendChartType; +import io.jenkins.plugins.checks.steps.ChecksInfo; import io.jenkins.plugins.util.JenkinsFacade; /** @@ -112,6 +113,8 @@ public class IssuesRecorder extends Recorder { private boolean skipPublishingChecks; // by default, checks will be published + private ChecksInfo checksInfo; + private String id; private String name; @@ -591,6 +594,10 @@ public void setFilters(final List filters) { this.filters = new ArrayList<>(filters); } + public void setChecksInfo(final ChecksInfo checksInfo) { + this.checksInfo = checksInfo; + } + @Override public BuildStepMonitor getRequiredMonitorService() { return BuildStepMonitor.NONE; @@ -737,7 +744,7 @@ reportName, getReferenceJobName(), getReferenceBuildId(), ignoreQualityGate, ign ResultAction action = publisher.attachAction(trendChartType); if (!skipPublishingChecks) { - WarningChecksPublisher checksPublisher = new WarningChecksPublisher(action, listener); + WarningChecksPublisher checksPublisher = new WarningChecksPublisher(action, listener, checksInfo); checksPublisher.publishChecks(); } } diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/PublishIssuesStep.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/PublishIssuesStep.java index 42d92a7257..90fbee2ae0 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/PublishIssuesStep.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/PublishIssuesStep.java @@ -857,8 +857,7 @@ protected ResultAction run() throws IOException, InterruptedException, IllegalSt ResultAction action = publisher.attachAction(step.getTrendChartType()); if (!step.isSkipPublishingChecks()) { - String checksName = Optional.ofNullable(getContext().get(ChecksInfo.class)).map(ChecksInfo::getName).orElse(null); - WarningChecksPublisher checksPublisher = new WarningChecksPublisher(action, getTaskListener(), checksName); + WarningChecksPublisher checksPublisher = new WarningChecksPublisher(action, getTaskListener(), getContext().get(ChecksInfo.class)); checksPublisher.publishChecks(); } diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/RecordIssuesStep.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/RecordIssuesStep.java index 1a687263fa..64d677e3c8 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/RecordIssuesStep.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/RecordIssuesStep.java @@ -41,6 +41,7 @@ import io.jenkins.plugins.analysis.core.util.QualityGateEvaluator; import io.jenkins.plugins.analysis.core.util.StageResultHandler; import io.jenkins.plugins.analysis.core.util.TrendChartType; +import io.jenkins.plugins.checks.steps.ChecksInfo; /** * Pipeline step that scans report files or the console log for issues. Stores the created issues in an {@link @@ -1054,6 +1055,7 @@ protected Void run() throws IOException, InterruptedException { recorder.setFailOnError(step.getFailOnError()); recorder.setTrendChartType(step.getTrendChartType()); recorder.setSourceDirectory(step.getSourceDirectory()); + recorder.setChecksInfo(getContext().get(ChecksInfo.class)); StageResultHandler statusHandler = new PipelineResultHandler(getRun(), getContext().get(FlowNode.class)); diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java index 770f01364d..cd9f6e439e 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java @@ -3,6 +3,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import org.apache.commons.lang3.StringUtils; @@ -31,6 +32,7 @@ import io.jenkins.plugins.checks.api.ChecksPublisher; import io.jenkins.plugins.checks.api.ChecksPublisherFactory; import io.jenkins.plugins.checks.api.ChecksStatus; +import io.jenkins.plugins.checks.steps.ChecksInfo; /** * Publishes warnings as checks to scm platforms. @@ -40,16 +42,12 @@ class WarningChecksPublisher { private final ResultAction action; private final TaskListener listener; - private final String checksName; + private final ChecksInfo checksInfo; - WarningChecksPublisher(final ResultAction action, final TaskListener listener, final String checksName) { + WarningChecksPublisher(final ResultAction action, final TaskListener listener, final ChecksInfo checksInfo) { this.action = action; this.listener = listener; - this.checksName = checksName; - } - - WarningChecksPublisher(final ResultAction action, final TaskListener listener) { - this(action, listener, null); + this.checksInfo = checksInfo; } /** @@ -68,8 +66,12 @@ ChecksDetails extractChecksDetails() { StaticAnalysisLabelProvider labelProvider = action.getLabelProvider(); + String checksName = Optional.ofNullable(checksInfo).map(ChecksInfo::getName) + .filter(StringUtils::isNotEmpty) + .orElse(labelProvider.getName()); + return new ChecksDetailsBuilder() - .withName(StringUtils.defaultIfEmpty(checksName, labelProvider.getName())) + .withName(checksName) .withStatus(ChecksStatus.COMPLETED) .withConclusion(extractChecksConclusion(result.getQualityGateStatus())) .withOutput(new ChecksOutputBuilder() diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java index 720a0c4dc2..bdd489f980 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java @@ -79,7 +79,7 @@ public void shouldCreateChecksDetailsWithNewIssuesAsAnnotations() { .hasTotalSize(6) .hasNewSize(2); - WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(run), TaskListener.NULL); + WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(run), TaskListener.NULL, null); assertThat(publisher.extractChecksDetails()) .hasFieldOrPropertyWithValue("detailsURL", Optional.of(getResultAction(run).getAbsoluteUrl())) .usingRecursiveComparison() @@ -94,7 +94,7 @@ public void shouldConcludeChecksAsSuccessWhenQualityGateIsPassed() { recorder -> recorder.addQualityGate(10, QualityGateType.TOTAL, QualityGateResult.UNSTABLE)); Run build = buildSuccessfully(project); - WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(build), TaskListener.NULL); + WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(build), TaskListener.NULL, null); assertThat(publisher.extractChecksDetails().getConclusion()) .isEqualTo(ChecksConclusion.SUCCESS); @@ -120,7 +120,7 @@ public void shouldParseHtmlMessage() { copySingleFileToWorkspace(project, "PVSReport.xml", "PVSReport.plog"); Run run = buildSuccessfully(project); - WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(run), TaskListener.NULL); + WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(run), TaskListener.NULL, null); ChecksDetails details = publisher.extractChecksDetails(); assertThat(details.getOutput().get().getChecksAnnotations()) @@ -142,7 +142,7 @@ public void shouldReportNoIssuesInTitle() { .hasTotalSize(0) .hasNewSize(0); - assertThat(new WarningChecksPublisher(getResultAction(run), TaskListener.NULL) + assertThat(new WarningChecksPublisher(getResultAction(run), TaskListener.NULL, null) .extractChecksDetails().getOutput()) .isPresent() .get() @@ -159,7 +159,7 @@ public void shouldReportOnlyTotalIssuesInTitleWhenNoNewIssues() { .hasTotalSize(4) .hasNewSize(0); - assertThat(new WarningChecksPublisher(getResultAction(run), TaskListener.NULL) + assertThat(new WarningChecksPublisher(getResultAction(run), TaskListener.NULL, null) .extractChecksDetails().getOutput()) .isPresent() .get() @@ -182,7 +182,7 @@ public void shouldReportOnlyNewIssuesInTitleWhenAllIssuesAreNew() { .hasTotalSize(6) .hasNewSize(6); - assertThat(new WarningChecksPublisher(getResultAction(run), TaskListener.NULL) + assertThat(new WarningChecksPublisher(getResultAction(run), TaskListener.NULL, null) .extractChecksDetails().getOutput()) .isPresent() .get() @@ -199,7 +199,7 @@ public void shouldIgnoreColumnsWhenBuildMultipleLineAnnotation() { copySingleFileToWorkspace(project, "pmd.xml"); Run run = buildSuccessfully(project); - WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(run), TaskListener.NULL); + WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(run), TaskListener.NULL, null); ChecksDetails details = publisher.extractChecksDetails(); assertThat(details.getOutput().get().getChecksAnnotations().get(0)) @@ -308,7 +308,7 @@ private void assertChecksConclusionIsFailureWithQualityGateResult(final QualityG .hasTotalSize(6) .hasQualityGateStatus(qualityGateResult.getStatus()); - WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(build), TaskListener.NULL); + WarningChecksPublisher publisher = new WarningChecksPublisher(getResultAction(build), TaskListener.NULL, null); assertThat(publisher.extractChecksDetails().getConclusion()) .isEqualTo(ChecksConclusion.FAILURE); } From 979f79225faeee4b22095cf0f2db79e234cdb130 Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Mon, 28 Dec 2020 18:55:41 +0000 Subject: [PATCH 6/7] Add test for withChecks/recordIssues --- .../steps/WarningChecksPublisherITest.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java index bdd489f980..4e02100536 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java @@ -213,7 +213,7 @@ public void shouldIgnoreColumnsWhenBuildMultipleLineAnnotation() { * Test that publishIssues honors the checks name provided by a withChecks context. */ @Test - public void shouldHonorWithChecksContext() { + public void shouldHonorWithChecksContextPublishIssues() { WorkflowJob project = createPipeline(); copySingleFileToWorkspace(project, NEW_CHECKSTYLE_REPORT); project.setDefinition(asStage("withChecks('Custom Checks Name') {", createScanForIssuesStep(new CheckStyle()), PUBLISH_ISSUES_STEP, "}")); @@ -224,7 +224,26 @@ public void shouldHonorWithChecksContext() { assertThat(publishedChecks.size()).isEqualTo(2); publishedChecks.forEach(check -> assertThat(check.getName()).isPresent().get().isEqualTo("Custom Checks Name")); + } + /** + * Test that recordIssues honors the checks name provided by a withChecks context. + */ + @Test + public void shouldHonorWithChecksContextRecordIssues() { + WorkflowJob project = createPipeline(); + copySingleFileToWorkspace(project, NEW_CHECKSTYLE_REPORT); + project.setDefinition(asStage(String.format("" + + "withChecks('Custom Checks Name') {\n" + + " recordIssues(tools: [%s(pattern: '%s')])\n" + + "}", new CheckStyle().getSymbolName(), NEW_CHECKSTYLE_REPORT))); + buildSuccessfully(project); + + List publishedChecks = PUBLISHER_FACTORY.getPublishedChecks(); + + assertThat(publishedChecks.size()).isEqualTo(2); + + publishedChecks.forEach(check -> assertThat(check.getName()).isPresent().get().isEqualTo("Custom Checks Name")); } private ChecksDetails createExpectedCheckStyleDetails() { From c46e911ae7d76e0f67483ffce4820a3cf5ca57ad Mon Sep 17 00:00:00 2001 From: Bill Collins Date: Tue, 29 Dec 2020 16:48:21 +0000 Subject: [PATCH 7/7] Review feedback --- .../analysis/core/steps/IssuesRecorder.java | 3 +- .../core/steps/WarningChecksPublisher.java | 4 +- .../steps/WarningChecksPublisherITest.java | 61 ++++++++++++++++--- .../core/testutil/IntegrationTest.java | 12 ++++ 4 files changed, 68 insertions(+), 12 deletions(-) diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java index 8f43278847..e3e08832b3 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesRecorder.java @@ -113,6 +113,7 @@ public class IssuesRecorder extends Recorder { private boolean skipPublishingChecks; // by default, checks will be published + @CheckForNull private ChecksInfo checksInfo; private String id; @@ -594,7 +595,7 @@ public void setFilters(final List filters) { this.filters = new ArrayList<>(filters); } - public void setChecksInfo(final ChecksInfo checksInfo) { + public void setChecksInfo(@CheckForNull final ChecksInfo checksInfo) { this.checksInfo = checksInfo; } diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java index cd9f6e439e..0ccf66658d 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisher.java @@ -14,6 +14,7 @@ import edu.hm.hafner.analysis.Issue; import edu.hm.hafner.analysis.Report; import edu.hm.hafner.util.VisibleForTesting; +import edu.umd.cs.findbugs.annotations.CheckForNull; import hudson.model.TaskListener; @@ -42,9 +43,10 @@ class WarningChecksPublisher { private final ResultAction action; private final TaskListener listener; + @CheckForNull private final ChecksInfo checksInfo; - WarningChecksPublisher(final ResultAction action, final TaskListener listener, final ChecksInfo checksInfo) { + WarningChecksPublisher(final ResultAction action, final TaskListener listener, @CheckForNull final ChecksInfo checksInfo) { this.action = action; this.listener = listener; this.checksInfo = checksInfo; diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java index 4e02100536..63ada65dd0 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/core/steps/WarningChecksPublisherITest.java @@ -209,21 +209,63 @@ public void shouldIgnoreColumnsWhenBuildMultipleLineAnnotation() { .hasFieldOrPropertyWithValue("endColumn", Optional.empty()); } + + + /** + * Test that publishIssues uses correct default name. + */ + @Test + public void shouldUseDefaultChecksNamePublishIssues() { + WorkflowJob project = createPipelineWithWorkspaceFiles(NEW_CHECKSTYLE_REPORT); + project.setDefinition(asStage(createScanForIssuesStep(new CheckStyle()), PUBLISH_ISSUES_STEP)); + buildSuccessfully(project); + + List publishedChecks = PUBLISHER_FACTORY.getPublishedChecks(); + + assertThat(publishedChecks).hasSize(1); + + assertThat(publishedChecks.get(0).getName()).isPresent().get().isEqualTo("CheckStyle"); + + assertThat(publishedChecks.get(0).getOutput()).isPresent().hasValueSatisfying( + output -> assertThat(output.getTitle()).isPresent().get().isEqualTo("No new issues, 6 total.")); + } + + /** + * Test that recordIssues uses correct default name. + */ + @Test + public void shouldUseDefaultChecksNameRecordIssues() { + WorkflowJob project = createPipelineWithWorkspaceFiles(NEW_CHECKSTYLE_REPORT); + project.setDefinition(asStage(createRecordIssuesStep(new CheckStyle()))); + buildSuccessfully(project); + + List publishedChecks = PUBLISHER_FACTORY.getPublishedChecks(); + + assertThat(publishedChecks).hasSize(1); + + assertThat(publishedChecks.get(0).getName()).isPresent().get().isEqualTo("CheckStyle"); + + assertThat(publishedChecks.get(0).getOutput()).isPresent().hasValueSatisfying( + output -> assertThat(output.getTitle()).isPresent().get().isEqualTo("No new issues, 6 total.")); + } + /** * Test that publishIssues honors the checks name provided by a withChecks context. */ @Test public void shouldHonorWithChecksContextPublishIssues() { - WorkflowJob project = createPipeline(); - copySingleFileToWorkspace(project, NEW_CHECKSTYLE_REPORT); + WorkflowJob project = createPipelineWithWorkspaceFiles(NEW_CHECKSTYLE_REPORT); project.setDefinition(asStage("withChecks('Custom Checks Name') {", createScanForIssuesStep(new CheckStyle()), PUBLISH_ISSUES_STEP, "}")); buildSuccessfully(project); List publishedChecks = PUBLISHER_FACTORY.getPublishedChecks(); - assertThat(publishedChecks.size()).isEqualTo(2); + assertThat(publishedChecks).hasSize(2); // First from 'In progress' check provided by withChecks, second from publishIssues publishedChecks.forEach(check -> assertThat(check.getName()).isPresent().get().isEqualTo("Custom Checks Name")); + + assertThat(publishedChecks.get(1).getOutput()).isPresent().hasValueSatisfying( + output -> assertThat(output.getTitle()).isPresent().get().isEqualTo("No new issues, 6 total.")); } /** @@ -231,19 +273,18 @@ public void shouldHonorWithChecksContextPublishIssues() { */ @Test public void shouldHonorWithChecksContextRecordIssues() { - WorkflowJob project = createPipeline(); - copySingleFileToWorkspace(project, NEW_CHECKSTYLE_REPORT); - project.setDefinition(asStage(String.format("" - + "withChecks('Custom Checks Name') {\n" - + " recordIssues(tools: [%s(pattern: '%s')])\n" - + "}", new CheckStyle().getSymbolName(), NEW_CHECKSTYLE_REPORT))); + WorkflowJob project = createPipelineWithWorkspaceFiles(NEW_CHECKSTYLE_REPORT); + project.setDefinition(asStage("withChecks('Custom Checks Name') {", createRecordIssuesStep(new CheckStyle()), "}")); buildSuccessfully(project); List publishedChecks = PUBLISHER_FACTORY.getPublishedChecks(); - assertThat(publishedChecks.size()).isEqualTo(2); + assertThat(publishedChecks).hasSize(2); // First from 'In progress' check provided by withChecks, second from recordIssues publishedChecks.forEach(check -> assertThat(check.getName()).isPresent().get().isEqualTo("Custom Checks Name")); + + assertThat(publishedChecks.get(1).getOutput()).isPresent().hasValueSatisfying( + output -> assertThat(output.getTitle()).isPresent().get().isEqualTo("No new issues, 6 total.")); } private ChecksDetails createExpectedCheckStyleDetails() { diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/core/testutil/IntegrationTest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/core/testutil/IntegrationTest.java index 8b5647242a..646691329a 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/core/testutil/IntegrationTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/core/testutil/IntegrationTest.java @@ -583,6 +583,18 @@ protected String createScanForIssuesStep(final ReportScanningTool tool, final St issuesName, tool.getSymbolName(), join(arguments)); } + /** + * Creates a pipeline step that records issues of the specified tool. + * + * @param tool + * the class of the tool to use + * + * @return the pipeline step + */ + protected String createRecordIssuesStep(final ReportScanningTool tool) { + return String.format("recordIssues(tools: [%s(pattern: '**/*issues.txt', reportEncoding:'UTF-8')])", tool.getSymbolName()); + } + /** * Wraps the specified steps into a stage. *