From e515a4cec9158b38c55bc473f08cc636c325eea7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20Strandlie?= Date: Thu, 23 May 2019 10:29:41 +0200 Subject: [PATCH 1/2] Made it possible to specify baseline to compare diff against, and also removed some transient definitions to fix JENKINS-55549. --- README.md | 2 + .../plugins/cppcheck/CppcheckPublisher.java | 201 ++++++++++++++---- .../plugins/cppcheck/CppcheckResult.java | 73 +++++-- .../cppcheck/config/CppcheckConfig.java | 14 ++ .../plugins/cppcheck/CppcheckResultTest.java | 53 +++++ 5 files changed, 283 insertions(+), 60 deletions(-) create mode 100644 src/test/java/org/jenkinsci/plugins/cppcheck/CppcheckResultTest.java diff --git a/README.md b/README.md index 374bac5..dd00ba3 100644 --- a/README.md +++ b/README.md @@ -3,4 +3,6 @@ Cppcheck Plugin This [Jenkins CI](http://jenkins-ci.org/) plug-in generates the trend report for [Cppcheck](http://cppcheck.sourceforge.net/), a tool for static C/C++ code analysis. +To setup your environment for development of this plugin, see the "Setting up a productive environment with your IDE" part of: + For more information, visit the wiki page . diff --git a/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java b/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java index a39100d..d845e76 100644 --- a/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java +++ b/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java @@ -46,6 +46,13 @@ public class CppcheckPublisher extends Recorder implements SimpleBuildStep { * @since 1.15 */ public static final String XML_FILE_DETAILS = "cppcheck_details.xml"; + + /** + * XML file with source container data for baseline data. Also used in lazy loading. + */ + public static final String XML_FILE_BASELINE_DETAILS = "cppcheck_baseline_details.xml"; + + private static final String BASELINE_SUBDIRECTORY = "/baseline"; private CppcheckConfig config; @@ -121,6 +128,15 @@ public void setPattern(String pattern) { public String getPattern() { return config.getPattern(); } + + @DataBoundSetter + public void setBaselinePattern(String baselinePattern) { + config.setBaselinePattern(baselinePattern); + } + public String getBaselinePattern() { + return config.getBaselinePattern(); + } + @DataBoundSetter public void setThreshold(String threshold) { config.getConfigSeverityEvaluation().setThreshold(threshold); @@ -297,6 +313,7 @@ public int getYSize() { public void setNumBuildsInGraph(int numBuildsInGraph) { config.getConfigGraph().setNumBuildsInGraph(numBuildsInGraph); } + public int getNumBuildsInGraph() { return config.getConfigGraph().getNumBuildsInGraph(); } @@ -326,20 +343,7 @@ public void perform(@Nonnull Run build, @Nonnull FilePath workspace, @Nonnu if (this.canContinue(build.getResult())) { CppcheckLogger.log(listener, "Starting the cppcheck analysis."); - EnvVars env = build.getEnvironment(listener); - String expandedPattern = env.expand(config.getPattern()); - - - CppcheckParserResult parser = new CppcheckParserResult(listener, - expandedPattern, config.isIgnoreBlankFiles()); - CppcheckReport cppcheckReport; - try { - cppcheckReport = workspace.act(parser); - } catch (Exception e) { - CppcheckLogger.log(listener, "Error on cppcheck analysis: " + e); - build.setResult(Result.FAILURE); - return; - } + CppcheckReport cppcheckReport = generateCppcheckReport(build, workspace, listener, config.getPattern()); if (cppcheckReport == null) { // Check if we're configured to allow not having a report @@ -356,6 +360,8 @@ public void perform(@Nonnull Run build, @Nonnull FilePath workspace, @Nonnu workspace, cppcheckReport.getAllErrors()); CppcheckResult result = new CppcheckResult(cppcheckReport.getStatistics(), build); + result.setSourceContainerPath(XML_FILE_DETAILS); + CppcheckConfigSeverityEvaluation severityEvaluation = config.getConfigSeverityEvaluation(); @@ -373,13 +379,28 @@ public void perform(@Nonnull Run build, @Nonnull FilePath workspace, @Nonnu build.addAction(buildAction); - XmlFile xmlSourceContainer = new XmlFile(new File(build.getRootDir(), - XML_FILE_DETAILS)); - xmlSourceContainer.write(cppcheckSourceContainer); - - copyFilesToBuildDirectory(build.getRootDir(), launcher.getChannel(), - cppcheckSourceContainer.getInternalMap().values()); - + writeAndCopyXmlFiles(build.getRootDir(), null, launcher, result.getSourceContainerPath(), + cppcheckSourceContainer); + + /** + * If config has a baseline pattern (optional), do the required parsing, + * report creation and writing to the build/baseline subdirectory + */ + if(config.isHasBaselinePattern()) { + + CppcheckReport cppcheckBaselineReport = generateCppcheckReport(build, workspace, listener, config.getBaselinePattern()); + + if (cppcheckBaselineReport != null) { + CppcheckResult baselineResult = new CppcheckResult(cppcheckBaselineReport.getStatistics(), build); + baselineResult.setSourceContainerPath(XML_FILE_BASELINE_DETAILS); + result.setBaselineResult(baselineResult); + CppcheckSourceContainer cppcheckBaselineSourceContainer = new CppcheckSourceContainer(listener, workspace.withSuffix(BASELINE_SUBDIRECTORY), + workspace.withSuffix(BASELINE_SUBDIRECTORY), cppcheckBaselineReport.getAllErrors()); + + writeAndCopyXmlFiles(build.getRootDir(), BASELINE_SUBDIRECTORY, launcher, baselineResult.getSourceContainerPath(), cppcheckBaselineSourceContainer); + } + } + CppcheckLogger.log(listener, "Ending the cppcheck analysis."); } return; @@ -393,25 +414,8 @@ public boolean perform(AbstractBuild build, Launcher launcher, if (this.canContinue(build.getResult())) { CppcheckLogger.log(listener, "Starting the cppcheck analysis."); - EnvVars env = build.getEnvironment(listener); - String expandedPattern = env.expand(config.getPattern()); - - - CppcheckParserResult parser = new CppcheckParserResult(listener, - expandedPattern, config.isIgnoreBlankFiles()); - CppcheckReport cppcheckReport = null; - try { - FilePath oWorkspacePath = build.getWorkspace(); - if( oWorkspacePath != null) { - cppcheckReport = oWorkspacePath.act(parser); - } - - } catch (Exception e) { - CppcheckLogger.log(listener, "Error on cppcheck analysis: " + e); - build.setResult(Result.FAILURE); - return false; - } - + CppcheckReport cppcheckReport = generateCppcheckReport(build, listener, config.getPattern()); + if (cppcheckReport == null) { // Check if we're configured to allow not having a report if (config.getAllowNoReport()) { @@ -421,12 +425,14 @@ public boolean perform(AbstractBuild build, Launcher launcher, return false; } } - + CppcheckSourceContainer cppcheckSourceContainer = new CppcheckSourceContainer(listener, build.getWorkspace(), build.getModuleRoot(), cppcheckReport.getAllErrors()); CppcheckResult result = new CppcheckResult(cppcheckReport.getStatistics(), build); + result.setSourceContainerPath(XML_FILE_DETAILS); + CppcheckConfigSeverityEvaluation severityEvaluation = config.getConfigSeverityEvaluation(); @@ -443,18 +449,119 @@ public boolean perform(AbstractBuild build, Launcher launcher, CppcheckBuildAction.computeHealthReportPercentage(result, severityEvaluation)); build.addAction(buildAction); + + writeAndCopyXmlFiles(build.getRootDir(), null, launcher, result.getSourceContainerPath(), + cppcheckSourceContainer); - XmlFile xmlSourceContainer = new XmlFile(new File(build.getRootDir(), - XML_FILE_DETAILS)); - xmlSourceContainer.write(cppcheckSourceContainer); - - copyFilesToBuildDirectory(build.getRootDir(), launcher.getChannel(), - cppcheckSourceContainer.getInternalMap().values()); + + /** + * If config has a baseline pattern (optional), do the required parsing, + * report creation and writing to the build directory + */ + if(config.isHasBaselinePattern()) { + CppcheckReport cppcheckBaselineReport = generateCppcheckReport(build, listener, config.getBaselinePattern()); + + if (cppcheckBaselineReport != null) { + CppcheckResult baselineResult = new CppcheckResult(cppcheckBaselineReport.getStatistics(), build); + baselineResult.setSourceContainerPath(XML_FILE_BASELINE_DETAILS); + + result.setBaselineResult(new CppcheckResult(cppcheckBaselineReport.getStatistics(), build)); + + CppcheckSourceContainer cppcheckBaselineSourceContainer = new CppcheckSourceContainer(listener, + build.getWorkspace().withSuffix(BASELINE_SUBDIRECTORY), + build.getModuleRoot().withSuffix(BASELINE_SUBDIRECTORY), + cppcheckBaselineReport.getAllErrors()); + + writeAndCopyXmlFiles(build.getRootDir(), BASELINE_SUBDIRECTORY, launcher, baselineResult.getSourceContainerPath(), + cppcheckBaselineSourceContainer); + + } + } CppcheckLogger.log(listener, "Ending the cppcheck analysis."); } return true; } + + /** + * + * @param build The current build + * @param workspace The workspace the build is performing in + * @param launcher + * @param listener + * @param pattern The pattern found from either config.getPattern() or config.getBaselinePattern() + * @return The corresponding CppcheckReport + * @throws IOException If the files could not be read + * @throws InterruptedException If the user cancels the processing + */ + private CppcheckReport generateCppcheckReport(@Nonnull Run build, @Nonnull FilePath workspace, + @Nonnull TaskListener listener, String pattern) throws IOException, InterruptedException { + + EnvVars env = build.getEnvironment(listener); + String expandedPattern = env.expand(pattern); + + CppcheckParserResult parser = new CppcheckParserResult(listener, + expandedPattern, config.isIgnoreBlankFiles()); + + CppcheckReport cppcheckReport; + + try { + cppcheckReport = workspace.act(parser); + } catch (Exception e) { + CppcheckLogger.log(listener, "Error on cppcheck analysis: " + e); + build.setResult(Result.FAILURE); + return null; + } + + return cppcheckReport; + + + } + + private CppcheckReport generateCppcheckReport(AbstractBuild build, BuildListener listener, String pattern) { + + try { + FilePath oWorkspacePath = build.getWorkspace(); + if (oWorkspacePath != null) { + return generateCppcheckReport(build, oWorkspacePath, listener, pattern); + } + + } catch (Exception e) { + CppcheckLogger.log(listener, "Error on cppcheck analysis: " + e); + build.setResult(Result.FAILURE); + return null; + + } + + return null; + } + + + /** + * Handles writing and copying of Xml files generated from the Cppcheck analysis to the workspace. + * + * @param rootDir The root directory of the build + * @param child An optional child parameter. Used when baseline is specified. + * @param launcher + * @param sourceContainerPath The name of the file on disk for the sourceContainer + * @param sourceContainer + * @throws IOException If the files could not be written + * @throws InterruptedException If the user cancels the processing + */ + private void writeAndCopyXmlFiles(@Nonnull File rootDir, String child, @Nonnull Launcher launcher, @Nonnull String sourceContainerPath, + @Nonnull CppcheckSourceContainer sourceContainer) throws IOException, InterruptedException { + + XmlFile xmlSourceContainer = new XmlFile(new File(rootDir, sourceContainerPath)); + xmlSourceContainer.write(sourceContainer); + + if (child != null) { + rootDir = new File(rootDir, child); + rootDir.mkdirs(); + } + + copyFilesToBuildDirectory(rootDir, launcher.getChannel(), sourceContainer.getInternalMap().values()); + + } /** diff --git a/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java b/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java index 5eb6133..692003f 100644 --- a/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java +++ b/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java @@ -49,7 +49,7 @@ public class CppcheckResult implements Serializable { /** * The build owner. */ - private transient Run owner; + private Run owner; /** * The Cppcheck report statistics. @@ -57,6 +57,19 @@ public class CppcheckResult implements Serializable { * @since 1.15 */ private CppcheckStatistics statistics; + + /** + * A reference to the baseline result we compare against in getDiffs(), + * if we wish to specify this explicitly. Can be null, and is then computed + * based on previous builds. + */ + private CppcheckResult baselineResult; + + /** + * The path to the sourceContainer for this result in the file system. + * If it is null, the default CppcheckPublisher.XML_FILE_DETAILS will be used. + */ + private String sourceContainerPath; /** * Constructor. @@ -126,6 +139,28 @@ public synchronized void setOwner(Run owner) { public CppcheckSourceContainer getCppcheckSourceContainer() { return lazyLoadSourceContainer(); } + + /** + * Used in cases where we don't want to use the default + * CppcheckPublisher.XML_FILE_DETAILS path. E.g. when + * this instance represents a baseline result. + * + * @param path The path to be set + */ + public void setSourceContainerPath(String path) { + if (path == null) { + this.sourceContainerPath = CppcheckPublisher.XML_FILE_DETAILS; + } else { + this.sourceContainerPath = path; + } + } + + public String getSourceContainerPath() { + if (this.sourceContainerPath != null) { + return this.sourceContainerPath; + } + return CppcheckPublisher.XML_FILE_DETAILS; + } /** * Gets the dynamic result of the selection element. @@ -215,17 +250,30 @@ private int parseIntWithDefault(String str, int defaultValue) { return defaultValue; } } + + /** + * Sets the baseline result, that we can compare aginst + * as opposed to comparing to a result found from previous builds. + * + * @param baselineResult the result we want to compare against + */ + public void setBaselineResult(CppcheckResult baselineResult) { + this.baselineResult = baselineResult; + } /** - * Gets the previous Cppcheck result for the build result. + * Gets the Cppcheck result for the build result, either + * set explicitly or the result of the previous build * * @return the previous Cppcheck result or null */ public CppcheckResult getPreviousResult() { - CppcheckBuildAction previousAction = getPreviousAction(); - CppcheckResult previousResult = null; - if (previousAction != null) { - previousResult = previousAction.getResult(); + CppcheckResult previousResult = this.baselineResult; + if (previousResult == null) { + CppcheckBuildAction previousAction = getPreviousAction(); + if (previousAction != null) { + previousResult = previousAction.getResult(); + } } return previousResult; @@ -248,19 +296,19 @@ private CppcheckBuildAction getPreviousAction() { } /** - * Get differences between current and previous statistics. + * Get differences between current and previous statistics. These can be computed based on previous build + * or specified explicitly using setPreviousStatistics(). * * @return the differences */ public CppcheckStatistics getDiff(){ CppcheckStatistics current = getStatistics(); CppcheckResult previousResult = getPreviousResult(); - if(previousResult == null) { - return new CppcheckStatistics(); + return new CppcheckStatistics(); } - CppcheckStatistics previous = previousResult.getStatistics(); + CppcheckStatistics previous = previousResult.getStatistics(); return new CppcheckStatistics( current.getNumberErrorSeverity() - previous.getNumberErrorSeverity(), @@ -397,7 +445,6 @@ public Collection diffCurrentAndPrevious( // Exact match first for(CppcheckWorkspaceFile curFile : curValues) { CppcheckFile curCppFile = curFile.getCppcheckFile(); - for(CppcheckWorkspaceFile prevFile : prevValues) { CppcheckFile prevCppFile = prevFile.getCppcheckFile(); @@ -410,7 +457,7 @@ public Collection diffCurrentAndPrevious( } } } - + // Approximate match of the rest (ignore line numbers) for(CppcheckWorkspaceFile curFile : curValues) { if(curFile.getDiffState() != null) { @@ -526,7 +573,7 @@ private CppcheckSourceContainer lazyLoadSourceContainer() { if(owner != null) { XmlFile xmlSourceContainer = null; xmlSourceContainer = new XmlFile(new File(owner.getRootDir(), - CppcheckPublisher.XML_FILE_DETAILS)); + getSourceContainerPath())); return (CppcheckSourceContainer) xmlSourceContainer.read(); } diff --git a/src/main/java/org/jenkinsci/plugins/cppcheck/config/CppcheckConfig.java b/src/main/java/org/jenkinsci/plugins/cppcheck/config/CppcheckConfig.java index c92db26..227bc6d 100644 --- a/src/main/java/org/jenkinsci/plugins/cppcheck/config/CppcheckConfig.java +++ b/src/main/java/org/jenkinsci/plugins/cppcheck/config/CppcheckConfig.java @@ -12,6 +12,7 @@ public class CppcheckConfig implements Serializable { private static final long serialVersionUID = 1L; private String pattern; + private String baselinePattern; private boolean ignoreBlankFiles; private boolean allowNoReport; private CppcheckConfigSeverityEvaluation configSeverityEvaluation = new CppcheckConfigSeverityEvaluation(); @@ -21,6 +22,11 @@ public class CppcheckConfig implements Serializable { public void setPattern(String pattern) { this.pattern = pattern; } + + @DataBoundSetter + public void setBaselinePattern(String baselinePattern) { + this.baselinePattern = baselinePattern; + } @DataBoundSetter public void setIgnoreBlankFiles(boolean ignoreBlankFiles) { @@ -50,6 +56,14 @@ public void setUseWorkspaceAsRootPath(boolean useWorkspaceAsRootPath) { public String getPattern() { return pattern; } + + public String getBaselinePattern() { + return baselinePattern; + } + + public boolean isHasBaselinePattern() { + return getBaselinePattern() != null; + } @Deprecated public String getCppcheckReportPattern() { diff --git a/src/test/java/org/jenkinsci/plugins/cppcheck/CppcheckResultTest.java b/src/test/java/org/jenkinsci/plugins/cppcheck/CppcheckResultTest.java new file mode 100644 index 0000000..76910e9 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/cppcheck/CppcheckResultTest.java @@ -0,0 +1,53 @@ +package org.jenkinsci.plugins.cppcheck; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; + +import org.junit.Before; +import org.junit.Test; + +import hudson.model.AbstractBuild; + +public class CppcheckResultTest { + + private AbstractBuild owner; + private CppcheckResult result1; + private CppcheckResult result2; + + @Before + public void setUp() throws Exception { + owner = mock(AbstractBuild.class); + result1 = new CppcheckResult(new CppcheckStatistics(3, 3, 3, 3, 3, 3, 3, null), owner); + result2 = new CppcheckResult(new CppcheckStatistics(1, 1, 1, 1, 1, 1, 1, null), owner); + } + + @Test + public void testCorrectNumberOfDiffsCalculatedWithoutBaseline() { + CppcheckStatistics diff1 = result1.getDiff(); + + assertEquals(0, diff1.getNumberErrorSeverity()); + assertEquals(0, diff1.getNumberWarningSeverity()); + assertEquals(0, diff1.getNumberStyleSeverity()); + assertEquals(0, diff1.getNumberPerformanceSeverity()); + assertEquals(0, diff1.getNumberInformationSeverity()); + assertEquals(0, diff1.getNumberNoCategorySeverity()); + assertEquals(0, diff1.getNumberPortabilitySeverity()); + assertEquals(0, diff1.getNumberTotal()); + } + + @Test + public void testCorrectNumberOfDiffsCalculatedWithBaseline() { + result1.setBaselineResult(result2); + CppcheckStatistics diff1 = result1.getDiff(); + + assertEquals(2, diff1.getNumberErrorSeverity()); + assertEquals(2, diff1.getNumberWarningSeverity()); + assertEquals(2, diff1.getNumberStyleSeverity()); + assertEquals(2, diff1.getNumberPerformanceSeverity()); + assertEquals(2, diff1.getNumberInformationSeverity()); + assertEquals(2, diff1.getNumberNoCategorySeverity()); + assertEquals(2, diff1.getNumberPortabilitySeverity()); + assertEquals(14, diff1.getNumberTotal()); + } + +} From ac1fc1e1b302bd3fd3d2bf854c84c594dd03110c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kon=20Strandlie?= Date: Wed, 21 Aug 2019 13:11:15 +0200 Subject: [PATCH 2/2] Fixed bugs found by SpotBugs analysis. No longer fixes JENKINS-55549 --- .../plugins/cppcheck/CppcheckPublisher.java | 21 ++++++++++++------- .../plugins/cppcheck/CppcheckResult.java | 2 +- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java b/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java index d845e76..60e6529 100644 --- a/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java +++ b/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java @@ -426,9 +426,13 @@ public boolean perform(AbstractBuild build, Launcher launcher, } } + FilePath workspace = build.getWorkspace(); + FilePath moduleRoot = build.getModuleRoot(); + + CppcheckSourceContainer cppcheckSourceContainer - = new CppcheckSourceContainer(listener, build.getWorkspace(), - build.getModuleRoot(), cppcheckReport.getAllErrors()); + = new CppcheckSourceContainer(listener, workspace, + moduleRoot, cppcheckReport.getAllErrors()); CppcheckResult result = new CppcheckResult(cppcheckReport.getStatistics(), build); result.setSourceContainerPath(XML_FILE_DETAILS); @@ -458,7 +462,7 @@ public boolean perform(AbstractBuild build, Launcher launcher, * If config has a baseline pattern (optional), do the required parsing, * report creation and writing to the build directory */ - if(config.isHasBaselinePattern()) { + if(config.isHasBaselinePattern() && workspace != null && moduleRoot != null) { CppcheckReport cppcheckBaselineReport = generateCppcheckReport(build, listener, config.getBaselinePattern()); if (cppcheckBaselineReport != null) { @@ -468,8 +472,8 @@ public boolean perform(AbstractBuild build, Launcher launcher, result.setBaselineResult(new CppcheckResult(cppcheckBaselineReport.getStatistics(), build)); CppcheckSourceContainer cppcheckBaselineSourceContainer = new CppcheckSourceContainer(listener, - build.getWorkspace().withSuffix(BASELINE_SUBDIRECTORY), - build.getModuleRoot().withSuffix(BASELINE_SUBDIRECTORY), + workspace.withSuffix(BASELINE_SUBDIRECTORY), + moduleRoot.withSuffix(BASELINE_SUBDIRECTORY), cppcheckBaselineReport.getAllErrors()); writeAndCopyXmlFiles(build.getRootDir(), BASELINE_SUBDIRECTORY, launcher, baselineResult.getSourceContainerPath(), @@ -553,13 +557,16 @@ private void writeAndCopyXmlFiles(@Nonnull File rootDir, String child, @Nonnull XmlFile xmlSourceContainer = new XmlFile(new File(rootDir, sourceContainerPath)); xmlSourceContainer.write(sourceContainer); + boolean allDirectoriesExist = rootDir.isDirectory(); if (child != null) { rootDir = new File(rootDir, child); - rootDir.mkdirs(); + allDirectoriesExist = rootDir.mkdirs(); } - copyFilesToBuildDirectory(rootDir, launcher.getChannel(), sourceContainer.getInternalMap().values()); + if(allDirectoriesExist) { + copyFilesToBuildDirectory(rootDir, launcher.getChannel(), sourceContainer.getInternalMap().values()); + } } diff --git a/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java b/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java index 692003f..9d74b49 100644 --- a/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java +++ b/src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java @@ -49,7 +49,7 @@ public class CppcheckResult implements Serializable { /** * The build owner. */ - private Run owner; + private transient Run owner; /** * The Cppcheck report statistics.