Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Bugfix: Ensure multiple runs invariant #30

Merged
merged 12 commits into from Mar 20, 2017

Conversation

mr-archano
Copy link
Contributor

@mr-archano mr-archano commented Mar 19, 2017

Scope of the PR

As @ouchadam mentioned in issue #11:

Given a previously failed result, when running the task again the build does not fail.

This shouldn't happen because code is identical, therefore should yield the same results. As already mentioned in #11, after a quick investigation:

The problem seems related to the fact the violations are now collected only if static analysis tasks (eg: checkstyleMain) are run. When one of these tasks is UP-TO-DATE because inputs are the same then we don't evaluate the reports (already generated) and this yields a wrong result for evaluateViolations.

Considerations and implementation

Each analysis task (Checkstyle, Findbugs, Pmd) is now depending on a specific implementation of CollectionViolationsTask that will perform the same action before provided as doLast. This will allow to evaluate the reports and collect the violations even when the analysis are skipped because their input haven't changed.

Test(s) added

For each tool supported by the plugin an integration test has been added to ensure that 2 consecutive runs without any change in the code yield the same result.

this.xmlReportFile = xmlReportFile
}

void setViolations(Violations violations) {
Copy link
Contributor

Choose a reason for hiding this comment

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

heh this reminds me of the PMD source code

}

File getHtmlReportFile() {
new File(xmlReportFile.absolutePath - '.xml' + '.html')
Copy link
Contributor

Choose a reason for hiding this comment

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

cool syntax

@Override
void collectViolations(File xmlReportFile, File htmlReportFile, Violations violations) {
PmdViolationsEvaluator evaluator = new PmdViolationsEvaluator(xmlReportFile)
int errors = 0, warnings = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

just because you can, doesn't mean you should

Copy link
Contributor Author

Choose a reason for hiding this comment

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

LoL, I tried.

@@ -44,6 +44,29 @@ public class CheckstyleIntegrationTest {
}

@Test
public void shouldFailBuildAgainWhenCheckstyleWarningsStillOverTheThresholdAfterSecondRun() {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

might be nice to word it differently tho

afterSecondRunWhenStillOverTheThresholdThenTheBuildFailsAgain

or if you insist on starting with should:

shouldFailTheBuildAfterSecondRunWhenStillOverTheThreshold


import org.gradle.api.tasks.JavaExec

class GenerateFindBugsHtmlReport extends JavaExec {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ouchadam
Copy link
Contributor

nice set of tests for each plugin, LGTM ! 👍

@ouchadam ouchadam merged commit e6ed64d into master Mar 20, 2017
@ouchadam ouchadam deleted the bugfix/multiple_runs_invariant branch March 20, 2017 18:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants