Skip to content

[JENKINS-24076] Add a configuration option for stable build comparison #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[JENKINS-24076] Add a configuration option for stable build comparison #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckPublisher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class CppcheckPublisher extends Recorder {
public CppcheckPublisher(String pattern,
boolean ignoreBlankFiles, String threshold,
boolean allowNoReport,
boolean stableBuild,
String newThreshold, String failureThreshold,
String newFailureThreshold, String healthy, String unHealthy,
boolean severityError,
Expand All @@ -66,6 +67,7 @@ public CppcheckPublisher(String pattern,
cppcheckConfig.setPattern(pattern);
cppcheckConfig.setAllowNoReport(allowNoReport);
cppcheckConfig.setIgnoreBlankFiles(ignoreBlankFiles);
cppcheckConfig.setStableBuild(stableBuild);
CppcheckConfigSeverityEvaluation configSeverityEvaluation = new CppcheckConfigSeverityEvaluation(
threshold, newThreshold, failureThreshold, newFailureThreshold, healthy, unHealthy,
severityError,
Expand Down Expand Up @@ -143,7 +145,8 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher,
= new CppcheckSourceContainer(listener, build.getWorkspace(),
build.getModuleRoot(), cppcheckReport.getAllErrors());

CppcheckResult result = new CppcheckResult(cppcheckReport.getStatistics(), build);
CppcheckResult result = new CppcheckResult(cppcheckReport.getStatistics(), build,
cppcheckConfig.getStableBuild());
CppcheckConfigSeverityEvaluation severityEvaluation
= cppcheckConfig.getConfigSeverityEvaluation();

Expand Down
34 changes: 34 additions & 0 deletions src/main/java/org/jenkinsci/plugins/cppcheck/CppcheckResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ public class CppcheckResult implements Serializable {
* @since 1.15
*/
private CppcheckStatistics statistics;

/**
* Use stable builds only.
*
* @since 1.20
*/
private boolean stableBuild;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stableBuild would be stored to build.xml together with all non-transient fields now. This data file should contain only data from the particular build, stableBuild is global configuration flag.

<org.jenkinsci.plugins.cppcheck.CppcheckBuildAction>
  <owner class="build" reference="../../.."/>
  <result>
    <owner class="build" reference="../../../.."/>
    <statistics>
      <errorCount>0</errorCount>
      <warningCount>1</warningCount>
      <styleCount>22</styleCount>
      <performanceCount>2</performanceCount>
      <informationCount>1</informationCount>
      <noCategoryCount>0</noCategoryCount>
      <portabilityCount>0</portabilityCount>
      <versions>
        <string>1.61</string>
      </versions>
    </statistics>
    <stableBuild>false</stableBuild>    <!-- *** HERE *** -->
  </result>
  <healthReportPercentage>-1</healthReportPercentage>
</org.jenkinsci.plugins.cppcheck.CppcheckBuildAction>


/**
* Constructor.
Expand All @@ -68,6 +75,25 @@ public class CppcheckResult implements Serializable {
public CppcheckResult(CppcheckStatistics statistics, AbstractBuild<?, ?> owner) {
this.statistics = statistics;
this.owner = owner;
this.stableBuild = false;
}

/**
* Constructor.
*
* @param statistics
* the Cppcheck report statistics
* @param owner
* the build owner
* @param stableBuild
* use only stable builds for deltas
*
* @since 1.15
*/
public CppcheckResult(CppcheckStatistics statistics, AbstractBuild<?, ?> owner, boolean stableBuild) {
this.statistics = statistics;
this.owner = owner;
this.stableBuild = stableBuild;
}

/**
Expand Down Expand Up @@ -230,6 +256,14 @@ public CppcheckResult getPreviousResult() {
*/
private CppcheckBuildAction getPreviousAction() {
AbstractBuild<?, ?> previousBuild = owner.getPreviousBuild();

if (this.stableBuild) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each build has its own stableBuild configuration at the moment, global configuration flag should be used here instead.

// Iterate the build chain and find the last successful build.
while (null != previousBuild && previousBuild.getResult().isWorseThan(hudson.model.Result.SUCCESS)) {
previousBuild = previousBuild.getPreviousBuild();
}
}

if (previousBuild != null) {
return previousBuild.getAction(CppcheckBuildAction.class);
}
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/org/jenkinsci/plugins/cppcheck/config/CppcheckConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class CppcheckConfig implements Serializable {
private String pattern;
private boolean ignoreBlankFiles;
private boolean allowNoReport;
private boolean stableBuild;
private CppcheckConfigSeverityEvaluation configSeverityEvaluation = new CppcheckConfigSeverityEvaluation();
private CppcheckConfigGraph configGraph = new CppcheckConfigGraph();

Expand All @@ -27,6 +28,10 @@ public void setIgnoreBlankFiles(boolean ignoreBlankFiles) {
public void setAllowNoReport(boolean allowNoReport) {
this.allowNoReport = allowNoReport;
}

public void setStableBuild(boolean stableBuild) {
this.stableBuild = stableBuild;
}

public void setConfigSeverityEvaluation(CppcheckConfigSeverityEvaluation configSeverityEvaluation) {
this.configSeverityEvaluation = configSeverityEvaluation;
Expand Down Expand Up @@ -65,6 +70,10 @@ public boolean getAllowNoReport() {
return allowNoReport;
}

public boolean getStableBuild() {
return stableBuild;
}

public CppcheckConfigSeverityEvaluation getConfigSeverityEvaluation() {
return configSeverityEvaluation;
}
Expand Down
2 changes: 1 addition & 1 deletion ...ain/resources/com/thalesgroup/hudson/plugins/cppcheck/CppcheckPublisher/config.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ description.pattern=Cppcheck must be configured to generate XML reports for this
setting that specifies the generated cppcheck XML report files, such as '**/cppcheck-result-*.xml'. \
Basedir of the fileset is relative to the workspace root. \
If no value is set, then the default '**/cppcheck-result.xml' is used. Be sure not to include any \
non-report files into this pattern.\
non-report files into this pattern.\

5 changes: 5 additions & 0 deletions src/main/resources/org/jenkinsci/plugins/cppcheck/CppcheckPublisher/config.jelly
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
<label>${%Do not fail the build if the Cppcheck report is not found}</label>
</f:entry>

<f:entry field="stableBuild">
<f:checkbox name="cppcheck.stableBuild" checked="${config.stableBuild}"/>
<label>${%Compare the Cppcheck report to the last stable build's Cppcheck report}</label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compare the report to the previous stable build's report

</f:entry>

<f:advanced>
<u:thresholds id="cppcheck"/>
</f:advanced>
Expand Down
12 changes: 12 additions & 0 deletions src/main/resources/org/jenkinsci/plugins/cppcheck/CppcheckPublisher/help-stableBuild.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<p style="margin-top: 0px;">By default, Cppcheck compares the report warnings
and errors to that of the last build, which could be a build that was deemed
unstable or that failed. To compare the results of this build against the last
"stable" build instead, check this option.</p>

<p>Note: When comparing against unstable builds, your builds could be marked as
stable even when new errors or warnings have been introduced, because the delta
between the last unstable build and current build could be zero. If the
unstable build introduced a new error but resolve an error, then the total
error count will still be within the permitted threshold. Another build with
the same results will be marked as stable, allowing the new error to go
unnoticed. Selecting this option will ensure all errors are accounted for.</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the help text, it is not fully correct in several statements.

By default, the issues comparison is done between a build and the previous one independently to their status. To compare a build against the previous stable build, check this option.

When computing a build status based on threshold of newly introduced issues without this option being checked, the build after an unstable one can be marked as stable even if no issue was fixed - the delta is zero. Enabling this option will ensure all builds after an unstable one will be unstable too until the threshold of new issues related to the last stable build is met.

The plugin doesn't consider the source of build instability, it may and may not be the threshold. For example unit tests or code coverage can make the build unstable too.