Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -116,15 +124,21 @@ 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))
.orElseGet(() -> new AnalysisResult(run, getId(), deltaReport, report.getBlames(),
report.getStatistics(), qualityGateResult, report.getSizeOfOrigin()));
var action = new ResultAction(run, result, healthDescriptor, getId(), name, icon,
sourceCodeEncoding, trendChartType);
run.addAction(action);

if (run.getActions(ResultAction.class).isEmpty()) {
run.addOrReplaceAction(action);
}
else {
run.addAction(action);
}
Comment thread
uhafner marked this conversation as resolved.
Outdated

if (trendChartType == TrendChartType.TOOLS_AGGREGATION || trendChartType == TrendChartType.AGGREGATION_ONLY) {
run.addOrReplaceAction(new AggregationAction());
Expand All @@ -137,8 +151,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();
Expand Down Expand Up @@ -189,16 +202,6 @@ private void markIssuesInModifiedFiles(final Run<?, ?> referenceBuild, final Rep
}
}

private ResultSelector ensureThatIdIsUnique() {
var selector = new ByIdResultSelector(getId());
Optional<ResultAction> 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()) {
Expand Down Expand Up @@ -303,4 +306,4 @@ private Result getRequiredResult() {
private QualityGateEvaluationMode determineQualityGateEvaluationMode() {
return qualityGateEvaluationMode;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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}.
Expand Down Expand Up @@ -339,64 +339,63 @@ 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<AnalysisResult> results = getAnalysisResults(build);
assertThat(results).hasSize(2);
assertThat(results).hasSize(1);

Set<String> 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() {
Run<?, ?> build = runJobWithCheckStyleTwice(true, Result.SUCCESS);

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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,5 @@
<error line="22" column="5" severity="error" message="Die Methode &apos;detectPackageName&apos; ist nicht fr Vererbung entworfen - muss abstract, final oder leer sein." source="com.puppycrawl.tools.checkstyle.checks.design.DesignForExtensionCheck"/>
<error line="29" severity="error" message="Zeile länger als 80 Zeichen" source="com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck"/>
<error line="30" column="21" severity="error" message="&apos;}&apos; sollte in derselben Zeile stehen." source="com.puppycrawl.tools.checkstyle.checks.blocks.RightCurlyCheck"/>
<error line="37" column="9" severity="error" message="&apos;}&apos; sollte in derselben Zeile stehen." source="com.puppycrawl.tools.checkstyle.checks.blocks.RightCurlyCheck"/>
</file>
</checkstyle>
Loading