diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesPublisher.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesPublisher.java index f929c5359a..9d9eb53cbc 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesPublisher.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/steps/IssuesPublisher.java @@ -91,7 +91,15 @@ private String getId() { */ ResultAction attachAction(final TrendChartType trendChartType) { var issues = report.getReport(); - var deltaReport = computeDelta(issues); + var selector = new ByIdResultSelector(getId()); + var existingAction = selector.get(run); + if (existingAction.isPresent()) { + issues.logError("Removing existing result action with ID '%s' (duplicate ID: restart or configuration error?)", + getId()); + run.removeAction(existingAction.get()); + } + + var deltaReport = computeDelta(issues, selector); var qualityGateResult = evaluateQualityGate(issues, deltaReport); reportHealth(issues); @@ -116,7 +124,7 @@ ResultAction attachAction(final TrendChartType trendChartType) { logger.logInfoMessages(issues.getInfoMessages()); logger.logErrorMessages(issues.getErrorMessages()); - var result = new AnalysisHistory(run, ensureThatIdIsUnique()).getResult() + var result = new AnalysisHistory(run, selector).getResult() .map(previous -> new AnalysisResult(run, getId(), deltaReport, report.getBlames(), report.getStatistics(), qualityGateResult, report.getSizeOfOrigin(), previous)) @@ -124,6 +132,7 @@ ResultAction attachAction(final TrendChartType trendChartType) { report.getStatistics(), qualityGateResult, report.getSizeOfOrigin())); var action = new ResultAction(run, result, healthDescriptor, getId(), name, icon, sourceCodeEncoding, trendChartType); + run.addAction(action); if (trendChartType == TrendChartType.TOOLS_AGGREGATION || trendChartType == TrendChartType.AGGREGATION_ONLY) { @@ -137,8 +146,7 @@ private long count(final Report issues) { return issues.stream().filter(Issue::isPartOfModifiedCode).count(); } - private DeltaReport computeDelta(final Report issues) { - var selector = ensureThatIdIsUnique(); + private DeltaReport computeDelta(final Report issues, final ResultSelector selector) { var possibleReferenceBuild = findReferenceBuild(selector, issues); if (possibleReferenceBuild.isPresent()) { Run build = possibleReferenceBuild.get(); @@ -189,16 +197,6 @@ private void markIssuesInModifiedFiles(final Run referenceBuild, final Rep } } - private ResultSelector ensureThatIdIsUnique() { - var selector = new ByIdResultSelector(getId()); - Optional other = selector.get(run); - if (other.isPresent()) { - throw new IllegalStateException( - "ID %s is already used by another action: %s%n".formatted(getId(), other.get())); - } - return selector; - } - private void reportHealth(final Report filtered) { if (healthDescriptor.isEnabled()) { if (healthDescriptor.isValid()) { @@ -303,4 +301,4 @@ private Result getRequiredResult() { private QualityGateEvaluationMode determineQualityGateEvaluationMode() { return qualityGateEvaluationMode; } -} +} \ No newline at end of file diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/MiscIssuesRecorderITest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/MiscIssuesRecorderITest.java index 5d86cc12de..6bcffbf66b 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/MiscIssuesRecorderITest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/MiscIssuesRecorderITest.java @@ -9,10 +9,8 @@ import java.io.IOException; import java.util.Collections; import java.util.List; -import java.util.Set; import java.util.function.Supplier; import java.util.regex.Pattern; -import java.util.stream.Collectors; import hudson.model.FreeStyleProject; import hudson.model.Result; @@ -36,9 +34,11 @@ import io.jenkins.plugins.analysis.warnings.tasks.OpenTasks; import io.jenkins.plugins.forensics.reference.SimpleReferenceRecorder; import io.jenkins.plugins.util.QualityGateStatus; +import io.jenkins.plugins.util.JenkinsFacade; import static io.jenkins.plugins.analysis.core.assertions.Assertions.*; import static net.javacrumbs.jsonunit.assertj.JsonAssertions.*; +import static org.mockito.Mockito.*; /** * Integration tests of the warnings plug-in in freestyle jobs. Tests the new recorder {@link IssuesRecorder}. @@ -339,43 +339,27 @@ void shouldFailBuildWhenFailBuildOnErrorsIsSet() { } /** - * Enables CheckStyle tool twice for two different files with varying amount of issues: should produce a failure. + * Enables CheckStyle tool twice for two different files with varying number of issues. The second action should + * replace the first one, and the result must reflect the second file's warnings. */ @Test - void shouldThrowExceptionIfSameToolIsConfiguredTwice() { - Run build = runJobWithCheckStyleTwice(false, Result.FAILURE); - - var result = getAnalysisResult(build); - assertThat(getConsoleLog(result)).contains("ID checkstyle is already used by another action: " - + "io.jenkins.plugins.analysis.core.model.ResultAction for CheckStyle"); - assertThat(result).hasId(CHECKSTYLE); - assertThat(result).hasTotalSize(6); - } - - /** - * Enables CheckStyle tool twice for two different files with varying amount of issues. Uses a different ID for both - * tools so that no exception will be thrown. - */ - @Test - void shouldUseSameToolTwice() { - var project = createFreestyleJob("checkstyle.xml", "checkstyle-twice.xml"); - var first = createTool(new CheckStyle(), "**/checkstyle-issues.txt"); - var second = createTool(new CheckStyle(), "**/checkstyle-twice-issues.txt"); - second.setId("second"); - enableWarnings(project, recorder -> recorder.setAggregatingResults(false), first, second); - - Run build = buildWithResult(project, Result.SUCCESS); + void shouldReplaceExistingAction() { + Run build = runJobWithCheckStyleTwice(false, Result.SUCCESS); List results = getAnalysisResults(build); - assertThat(results).hasSize(2); + assertThat(results).hasSize(1); - Set ids = results.stream().map(AnalysisResult::getId).collect(Collectors.toSet()); - assertThat(ids).containsExactly(CHECKSTYLE, "second"); + var result = results.get(0); + + assertThat(getConsoleLog(result)).contains( + "Removing existing result action with ID 'checkstyle' (duplicate ID: restart or configuration error?)"); + assertThat(result).hasId(CHECKSTYLE); + assertThat(result).hasTotalSize(5); } /** * Runs the CheckStyle tool twice for two different files with varying amount of issues: due to enabled aggregation, - * the build should report 6 issues. + * the build should report 11 issues. */ @Test void shouldAggregateMultipleConfigurationsOfSameTool() { @@ -383,20 +367,35 @@ void shouldAggregateMultipleConfigurationsOfSameTool() { var result = getAnalysisResult(build); - assertThat(result).hasTotalSize(12); + assertThat(result).hasTotalSize(11); assertThat(result).hasId("analysis"); assertThat(result).hasQualityGateStatus(QualityGateStatus.INACTIVE); } private Run runJobWithCheckStyleTwice(final boolean isAggregationEnabled, final Result result) { var project = createFreestyleJob("checkstyle.xml", "checkstyle-twice.xml"); - enableWarnings(project, recorder -> recorder.setAggregatingResults(isAggregationEnabled), - createTool(new CheckStyle(), "**/checkstyle-issues.txt"), - createTool(new CheckStyle(), "**/checkstyle-twice-issues.txt")); + var first = createTool(new CheckStyle(), "**/checkstyle-issues.txt"); + configureCheckStyleDescriptor(first); + + var second = createTool(new CheckStyle(), "**/checkstyle-twice-issues.txt"); + configureCheckStyleDescriptor(second); + + enableWarnings(project, recorder -> recorder.setAggregatingResults(isAggregationEnabled), first, second); return buildWithResult(project, result); } + private void configureCheckStyleDescriptor(final ReportScanningTool tool) { + if (tool instanceof CheckStyle checkStyle) { + checkStyle.setId(CHECKSTYLE); + checkStyle.setName("CheckStyle"); + + var jenkinsFacade = mock(JenkinsFacade.class); + when(jenkinsFacade.getDescriptorOrDie(CheckStyle.class)).thenReturn(new CheckStyle.Descriptor()); + checkStyle.setJenkinsFacade(jenkinsFacade); + } + } + /** * Runs the Eclipse parser on two output file. The first file contains 8 warnings, the second 5 warnings. Then the * first file is for the first build to define the baseline. The second build with the second file generates the diff --git a/plugin/src/test/resources/io/jenkins/plugins/analysis/warnings/steps/checkstyle-twice.xml b/plugin/src/test/resources/io/jenkins/plugins/analysis/warnings/steps/checkstyle-twice.xml index 1cff492cb3..6e252b5210 100644 --- a/plugin/src/test/resources/io/jenkins/plugins/analysis/warnings/steps/checkstyle-twice.xml +++ b/plugin/src/test/resources/io/jenkins/plugins/analysis/warnings/steps/checkstyle-twice.xml @@ -6,6 +6,5 @@ -