Skip to content

Commit

Permalink
Merge pull request #721 from mrginglymus/with-checks
Browse files Browse the repository at this point in the history
Support `withChecks`, see jenkinsci/checks-api-plugin#49.
  • Loading branch information
uhafner authored Dec 29, 2020
2 parents 83811d5 + c46e911 commit f641419
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 12 deletions.
7 changes: 7 additions & 0 deletions doc/Documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions plugin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
<form-element-path.version>1.10</form-element-path.version>
<folder.version>6.15</folder.version>
<scm-api.version>2.6.3</scm-api.version>
<checks-api.version>1.2.0</checks-api.version>

<!-- Maven Surefire ArgLine -->
<argLine>-Djava.awt.headless=true -Xmx1024m -Djenkins.test.timeout=1000</argLine>
Expand Down Expand Up @@ -293,6 +294,7 @@
<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>checks-api</artifactId>
<version>${checks-api.version}</version>
<exclusions>
<exclusion>
<groupId>com.github.spotbugs</groupId>
Expand Down Expand Up @@ -392,6 +394,20 @@
<type>test-jar</type>
</dependency>

<dependency>
<groupId>io.jenkins.plugins</groupId>
<artifactId>checks-api</artifactId>
<version>${checks-api.version}</version>
<scope>test</scope>
<type>test-jar</type>
<exclusions>
<exclusion>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-annotations</artifactId>
</exclusion>
</exclusions>
</dependency>

<!-- Form element path plugin to simplify location of UI elements in HtmlUnit tests -->
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -112,6 +113,9 @@ public class IssuesRecorder extends Recorder {

private boolean skipPublishingChecks; // by default, checks will be published

@CheckForNull
private ChecksInfo checksInfo;

private String id;
private String name;

Expand Down Expand Up @@ -605,6 +609,10 @@ public void setFilters(final List<RegexpFilter> filters) {
this.filters = new ArrayList<>(filters);
}

public void setChecksInfo(@CheckForNull final ChecksInfo checksInfo) {
this.checksInfo = checksInfo;
}

@Override
public BuildStepMonitor getRequiredMonitorService() {
return BuildStepMonitor.NONE;
Expand Down Expand Up @@ -751,7 +759,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();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -855,7 +857,7 @@ protected ResultAction run() throws IOException, InterruptedException, IllegalSt
ResultAction action = publisher.attachAction(step.getTrendChartType());

if (!step.isSkipPublishingChecks()) {
WarningChecksPublisher checksPublisher = new WarningChecksPublisher(action, getTaskListener());
WarningChecksPublisher checksPublisher = new WarningChecksPublisher(action, getTaskListener(), getContext().get(ChecksInfo.class));
checksPublisher.publishChecks();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1063,6 +1064,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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -13,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;

Expand All @@ -31,6 +33,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.
Expand All @@ -40,10 +43,13 @@
class WarningChecksPublisher {
private final ResultAction action;
private final TaskListener listener;
@CheckForNull
private final ChecksInfo checksInfo;

WarningChecksPublisher(final ResultAction action, final TaskListener listener) {
WarningChecksPublisher(final ResultAction action, final TaskListener listener, @CheckForNull final ChecksInfo checksInfo) {
this.action = action;
this.listener = listener;
this.checksInfo = checksInfo;
}

/**
Expand All @@ -62,8 +68,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(labelProvider.getName())
.withName(checksName)
.withStatus(ChecksStatus.COMPLETED)
.withConclusion(extractChecksConclusion(result.getQualityGateStatus()))
.withOutput(new ChecksOutputBuilder()
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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.*;

Expand All @@ -39,6 +44,21 @@ 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();
}


/**
* Verifies that {@link WarningChecksPublisher} constructs the {@link ChecksDetails} correctly
* with only new issues.
Expand All @@ -59,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()
Expand All @@ -74,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);
Expand All @@ -100,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())
Expand All @@ -122,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()
Expand All @@ -139,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()
Expand All @@ -162,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()
Expand All @@ -179,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))
Expand All @@ -189,6 +209,84 @@ 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<ChecksDetails> 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<ChecksDetails> 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 = createPipelineWithWorkspaceFiles(NEW_CHECKSTYLE_REPORT);
project.setDefinition(asStage("withChecks('Custom Checks Name') {", createScanForIssuesStep(new CheckStyle()), PUBLISH_ISSUES_STEP, "}"));
buildSuccessfully(project);

List<ChecksDetails> publishedChecks = PUBLISHER_FACTORY.getPublishedChecks();

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."));
}

/**
* Test that recordIssues honors the checks name provided by a withChecks context.
*/
@Test
public void shouldHonorWithChecksContextRecordIssues() {
WorkflowJob project = createPipelineWithWorkspaceFiles(NEW_CHECKSTYLE_REPORT);
project.setDefinition(asStage("withChecks('Custom Checks Name') {", createRecordIssuesStep(new CheckStyle()), "}"));
buildSuccessfully(project);

List<ChecksDetails> publishedChecks = PUBLISHER_FACTORY.getPublishedChecks();

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() {
ChecksDetailsBuilder builder = new ChecksDetailsBuilder()
.withName("CheckStyle")
Expand Down Expand Up @@ -270,7 +368,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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down

0 comments on commit f641419

Please sign in to comment.