From bc6644b89437d0447dc8f6db30969598173a221c Mon Sep 17 00:00:00 2001 From: Ulli Hafner Date: Fri, 3 Dec 2021 23:21:34 +0100 Subject: [PATCH 1/3] Rename the `id` parameter for the `analysisModel` tool to `analysisModelId`. Otherwise, one cannot replace the tool ID with a custom ID. --- doc/Documentation.md | 2 +- .../analysis/warnings/RegisteredParser.java | 20 +++++++++++-------- .../analysis/warnings/steps/ParsersITest.java | 2 +- .../analysis/warnings/steps/StepsITest.java | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/doc/Documentation.md b/doc/Documentation.md index ac03224d61..9ede7ed113 100644 --- a/doc/Documentation.md +++ b/doc/Documentation.md @@ -180,7 +180,7 @@ recordIssues tool: checkStyle(pattern: 'checkstyle-result.xml') #### Pipeline step with a generic symbol ```groovy -recordIssues tool: analysisParser(pattern: 'checkstyle-result.xml', id: 'checkstyle') +recordIssues tool: analysisParser(pattern: 'checkstyle-result.xml', analysisModelId: 'checkstyle') ``` ### Creating support for a custom tool diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/warnings/RegisteredParser.java b/plugin/src/main/java/io/jenkins/plugins/analysis/warnings/RegisteredParser.java index 452e8caf9f..2d5e689bc0 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/warnings/RegisteredParser.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/warnings/RegisteredParser.java @@ -26,26 +26,30 @@ public class RegisteredParser extends ReportScanningTool { private static final ParserRegistry REGISTRY = new ParserRegistry(); - private final String id; + private final String analysisModelId; /** * Creates a new instance of {@link RegisteredParser}. * - * @param id - * the unique ID of the tool + * @param analysisModelId + * the unique ID of the tool in the analysis-model module */ @DataBoundConstructor - public RegisteredParser(final String id) { + public RegisteredParser(final String analysisModelId) { super(); - this.id = id; - if (!REGISTRY.contains(id)) { - throw new NoSuchElementException("No such parser found with the specified ID: " + id); + this.analysisModelId = analysisModelId; + if (!REGISTRY.contains(analysisModelId)) { + throw new NoSuchElementException("No such parser found with the specified ID: " + analysisModelId); } } + public String getAnalysisModelId() { + return analysisModelId; + } + private ParserDescriptor getParserDescriptor() { - return REGISTRY.get(id); + return REGISTRY.get(analysisModelId); } @Override diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/ParsersITest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/ParsersITest.java index 55c3a08a70..d447491d53 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/ParsersITest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/ParsersITest.java @@ -124,7 +124,7 @@ public void shouldFindAllCodeCheckerIssues() { "recordIssues tool:analysisParser(" + "pattern:'**/%s', " + "reportEncoding:'UTF-8', " - + "id:'code-checker')", logFile))); + + "analysisModelId:'code-checker')", logFile))); AnalysisResult result = scheduleSuccessfulBuild(job); diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/StepsITest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/StepsITest.java index cf41e35174..dc870a1feb 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/StepsITest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/steps/StepsITest.java @@ -78,7 +78,7 @@ public void shouldParseCheckstyleUsingTheParserRegistry() { job.setDefinition(new CpsFlowDefinition("node {\n" + " stage ('Integration Test') {\n" - + " recordIssues tool: analysisParser(id: 'checkstyle', pattern: '**/" + "checkstyle1" + "*')\n" + + " recordIssues tool: analysisParser(analysisModelId: 'checkstyle', pattern: '**/" + "checkstyle1" + "*')\n" + " }\n" + "}", true)); From c5b681cb51f82118a5662533fc165234d2d4272d Mon Sep 17 00:00:00 2001 From: Ulli Hafner Date: Sat, 4 Dec 2021 11:04:46 +0100 Subject: [PATCH 2/3] Add tests for `RegisteredParser`. --- plugin/pom.xml | 2 ++ .../warnings/RegisteredParserTest.java | 34 +++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 plugin/src/test/java/io/jenkins/plugins/analysis/warnings/RegisteredParserTest.java diff --git a/plugin/pom.xml b/plugin/pom.xml index 17a5e8f2c8..36d63ad6bc 100644 --- a/plugin/pom.xml +++ b/plugin/pom.xml @@ -566,9 +566,11 @@ io.jenkins.plugins.analysis.core.scm io.jenkins.plugins.analysis.core.model io.jenkins.plugins.analysis.core.util + io.jenkins.plugins.analysis.warnings .*Thresholds + .*TaskScanner.* io.jenkins.plugins.analysis.core.assertions diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/RegisteredParserTest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/RegisteredParserTest.java new file mode 100644 index 0000000000..d947a5a878 --- /dev/null +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/RegisteredParserTest.java @@ -0,0 +1,34 @@ +package io.jenkins.plugins.analysis.warnings; + +import java.util.NoSuchElementException; + +import org.junit.jupiter.api.Test; + +import edu.hm.hafner.analysis.parser.checkstyle.CheckStyleParser; + +import static io.jenkins.plugins.analysis.core.assertions.Assertions.*; + +/** + * Tests the class {@link RegisteredParser}. + * + * @author Ullrich Hafner + */ +class RegisteredParserTest { + private static final String CHECKSTYLE_ID = "checkstyle"; + + @Test + void shouldThrowExceptionIfThereIsNoParserAvailable() { + assertThatExceptionOfType(NoSuchElementException.class).isThrownBy(() -> new RegisteredParser("-unknown-")); + } + + @Test + void shouldAllowChangingId() { + RegisteredParser parser = new RegisteredParser(CHECKSTYLE_ID); + + assertThat(parser.createParser()).isInstanceOf(CheckStyleParser.class); + assertThat(parser).hasId(CHECKSTYLE_ID); + assertThat(parser.getLabelProvider()).hasId(CHECKSTYLE_ID); + assertThat(parser.getLabelProvider()).hasName("CheckStyle"); + + } +} From 44ac5b696dd0bd6c15c4c5b06d2982651fd45edb Mon Sep 17 00:00:00 2001 From: Ulli Hafner Date: Sat, 4 Dec 2021 11:47:31 +0100 Subject: [PATCH 3/3] Improve tests for `RegisteredParser`. --- plugin/pom.xml | 2 +- .../plugins/analysis/core/model/Tool.java | 10 +++++- .../analysis/warnings/RegisteredParser.java | 2 +- .../warnings/RegisteredParserTest.java | 35 +++++++++++++++++-- 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/plugin/pom.xml b/plugin/pom.xml index 36d63ad6bc..945ee868d1 100644 --- a/plugin/pom.xml +++ b/plugin/pom.xml @@ -29,7 +29,7 @@ ${analysis-model-api.version} 1.7.0 - 2.5.1 + 2.6.0 1.11.3-4 5.2.2-1 diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/core/model/Tool.java b/plugin/src/main/java/io/jenkins/plugins/analysis/core/model/Tool.java index fd775b9550..ce20e43552 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/core/model/Tool.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/core/model/Tool.java @@ -8,6 +8,7 @@ import edu.hm.hafner.analysis.ParsingCanceledException; import edu.hm.hafner.analysis.ParsingException; import edu.hm.hafner.analysis.Report; +import edu.hm.hafner.util.VisibleForTesting; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundSetter; @@ -40,6 +41,13 @@ public abstract class Tool extends AbstractDescribableImpl implements Seri private String id = StringUtils.EMPTY; private String name = StringUtils.EMPTY; + private JenkinsFacade jenkins = new JenkinsFacade(); + + @VisibleForTesting + public void setJenkinsFacade(final JenkinsFacade jenkinsFacade) { + this.jenkins = jenkinsFacade; + } + /** * Overrides the default ID of the results. The ID is used as URL of the results and as identifier in UI elements. * If no ID is given, then the default ID is used, see corresponding {@link ToolDescriptor}. @@ -122,7 +130,7 @@ public StaticAnalysisLabelProvider getLabelProvider() { @Override public ToolDescriptor getDescriptor() { - return (ToolDescriptor) super.getDescriptor(); + return (ToolDescriptor) jenkins.getDescriptorOrDie(getClass()); } /** diff --git a/plugin/src/main/java/io/jenkins/plugins/analysis/warnings/RegisteredParser.java b/plugin/src/main/java/io/jenkins/plugins/analysis/warnings/RegisteredParser.java index 2d5e689bc0..1bd6050e17 100644 --- a/plugin/src/main/java/io/jenkins/plugins/analysis/warnings/RegisteredParser.java +++ b/plugin/src/main/java/io/jenkins/plugins/analysis/warnings/RegisteredParser.java @@ -71,7 +71,7 @@ public IssueParser createParser() { public StaticAnalysisLabelProvider getLabelProvider() { ParserDescriptor descriptor = getParserDescriptor(); - return new StaticAnalysisLabelProvider(descriptor.getId(), descriptor.getName(), descriptor::getDescription); + return new StaticAnalysisLabelProvider(descriptor.getId(), getName(), descriptor::getDescription); } @Override diff --git a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/RegisteredParserTest.java b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/RegisteredParserTest.java index d947a5a878..2cbaccfc91 100644 --- a/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/RegisteredParserTest.java +++ b/plugin/src/test/java/io/jenkins/plugins/analysis/warnings/RegisteredParserTest.java @@ -6,7 +6,10 @@ import edu.hm.hafner.analysis.parser.checkstyle.CheckStyleParser; +import io.jenkins.plugins.util.JenkinsFacade; + import static io.jenkins.plugins.analysis.core.assertions.Assertions.*; +import static org.mockito.Mockito.*; /** * Tests the class {@link RegisteredParser}. @@ -15,6 +18,8 @@ */ class RegisteredParserTest { private static final String CHECKSTYLE_ID = "checkstyle"; + private static final String CHECK_STYLE_NAME = "CheckStyle"; + private static final String CHECKSTYLE_PATTERN = "**/checkstyle-result.xml"; @Test void shouldThrowExceptionIfThereIsNoParserAvailable() { @@ -25,10 +30,36 @@ void shouldThrowExceptionIfThereIsNoParserAvailable() { void shouldAllowChangingId() { RegisteredParser parser = new RegisteredParser(CHECKSTYLE_ID); + JenkinsFacade jenkins = mock(JenkinsFacade.class); + when(jenkins.getDescriptorOrDie(RegisteredParser.class)).thenReturn(new RegisteredParser.Descriptor()); + parser.setJenkinsFacade(jenkins); + assertThat(parser.createParser()).isInstanceOf(CheckStyleParser.class); - assertThat(parser).hasId(CHECKSTYLE_ID); + assertThat(parser) + .hasAnalysisModelId(CHECKSTYLE_ID) + .hasId(CHECKSTYLE_ID) + .hasActualId(CHECKSTYLE_ID) + .hasActualName(CHECK_STYLE_NAME) + .hasActualPattern(CHECKSTYLE_PATTERN); + assertThat(parser.getLabelProvider()).hasId(CHECKSTYLE_ID); - assertThat(parser.getLabelProvider()).hasName("CheckStyle"); + assertThat(parser.getLabelProvider()).hasName(CHECK_STYLE_NAME); + + String customId = "customId"; + parser.setId(customId); + String customName = "Custom Name"; + parser.setName(customName); + String customPattern = "Custom Pattern"; + parser.setPattern(customPattern); + + assertThat(parser) + .hasAnalysisModelId(CHECKSTYLE_ID) + .hasId(customId) + .hasActualId(customId) + .hasActualName(customName) + .hasActualPattern(customPattern); + assertThat(parser.getLabelProvider()).hasId(CHECKSTYLE_ID); // get decorations for checkstyle + assertThat(parser.getLabelProvider()).hasName(customName); } }