From 86d15236b54e6e24ac36ff10db5cf4f7921700d0 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Thu, 15 Feb 2018 13:12:00 +0100 Subject: [PATCH 1/3] Check if the output is defined when configuring detekt task --- .../internal/detekt/DetektConfigurator.groovy | 36 ++++----- .../detekt/DetektIntegrationTest.groovy | 76 ++++++++++--------- .../groovy/com/novoda/test/LogsSubject.groovy | 5 +- 3 files changed, 63 insertions(+), 54 deletions(-) diff --git a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy index 1ff7116..f06437f 100644 --- a/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy +++ b/plugin/src/main/groovy/com/novoda/staticanalysis/internal/detekt/DetektConfigurator.groovy @@ -12,6 +12,7 @@ class DetektConfigurator implements Configurator { private static final String DETEKT_PLUGIN = 'io.gitlab.arturbosch.detekt' private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.\nFor more information see https://github.com/arturbosch/detekt.' + private static final String OUTPUT_NOT_DEFINED = 'Output not defined! To analyze the results, `output` needs to be defined in Detekt profile.' private final Project project private final Violations violations @@ -32,7 +33,7 @@ class DetektConfigurator implements Configurator { @Override void execute() { - project.extensions.findByType(StaticAnalysisExtension).ext.'detekt' = { Closure config -> + project.extensions.findByType(StaticAnalysisExtension).ext.detekt = { Closure config -> if (!isKotlinProject(project)) { return } @@ -41,34 +42,35 @@ class DetektConfigurator implements Configurator { throw new GradleException(DETEKT_NOT_APPLIED) } - project.extensions.findByName('detekt').with { - // apply configuration closure to detekt extension - config.delegate = it - config() - } - - configureToolTask() + def detekt = project.extensions.findByName('detekt') + config.delegate = detekt + config() + configureToolTask(detekt) } } - private void configureToolTask() { + private void configureToolTask(detekt) { def detektTask = project.tasks['detektCheck'] + detektTask.group = 'verification' + // run detekt as part of check project.tasks['check'].dependsOn(detektTask) // evaluate violations after detekt - detektTask.group = 'verification' - def collectViolations = createCollectViolationsTask(violations) + def output = detekt.systemOrDefaultProfile().output + if (!output) { + throw new IllegalArgumentException(OUTPUT_NOT_DEFINED) + } + def collectViolations = createCollectViolationsTask(violations, project.file(output)) evaluateViolations.dependsOn collectViolations collectViolations.dependsOn detektTask } - private CollectDetektViolationsTask createCollectViolationsTask(Violations violations) { - def outputFolder = project.file(project.extensions.findByName('detekt').systemOrDefaultProfile().output) - project.tasks.create('collectDetektViolations', CollectDetektViolationsTask) { collectViolations -> - collectViolations.xmlReportFile = new File(outputFolder, 'detekt-checkstyle.xml') - collectViolations.htmlReportFile = new File(outputFolder, 'detekt-report.html') - collectViolations.violations = violations + private CollectDetektViolationsTask createCollectViolationsTask(Violations violations, File outputFolder) { + project.tasks.create('collectDetektViolations', CollectDetektViolationsTask) { task -> + task.xmlReportFile = new File(outputFolder, 'detekt-checkstyle.xml') + task.htmlReportFile = new File(outputFolder, 'detekt-report.html') + task.violations = violations } } diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index 0378f77..b990f0a 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -13,6 +13,9 @@ import static com.novoda.test.LogsSubject.assertThat @RunWith(Parameterized.class) class DetektIntegrationTest { + private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.' + private static final String OUTPUT_NOT_DEFINED = 'Output not defined! To analyze the results, `output` needs to be defined in Detekt profile.' + @Parameterized.Parameters(name = "{0}") static Iterable rules() { return [TestProjectRule.forKotlinProject(), TestProjectRule.forAndroidKotlinProject()] @@ -25,6 +28,26 @@ class DetektIntegrationTest { this.projectRule = projectRule } + @Test + void givenOutputNotDefinedShouldFailBuildOnConfiguration() { + def emptyConfiguration = detektWith("") + + def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_WARNINGS) + .withToolsConfig(emptyConfiguration) + .buildAndFail('check', '--dry-run') + + assertThat(result.logs).contains(OUTPUT_NOT_DEFINED) + } + + @Test + void shouldFailBuildOnConfigurationWhenDetektConfiguredButNotApplied() { + def result = projectRule.newProject() + .withToolsConfig(detektConfiguration(Fixtures.Detekt.SOURCES_WITH_ERRORS)) + .buildAndFail('check', '--dry-run') + + assertThat(result.logs).contains(DETEKT_NOT_APPLIED) + } + @Test void shouldFailBuildWhenDetektWarningsOverTheThreshold() { def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_WARNINGS) @@ -79,7 +102,7 @@ class DetektIntegrationTest { maxWarnings = 0 maxErrors = 0 }''') - testProject = testProject.withToolsConfig(detektConfigurationWithoutInput(testProject)) + .withToolsConfig(detektConfigurationWithoutInput()) TestProject.Result result = testProject .build('check') @@ -88,36 +111,19 @@ class DetektIntegrationTest { assertThat(result.logs).doesNotContainDetektViolations() } - @Test - void shouldFailBuildWhenDetektConfiguredButNotApplied() { - def testProject = projectRule.newProject() - .withPenalty('''{ - maxWarnings = 0 - maxErrors = 0 - }''') - - testProject = testProject.withToolsConfig(detektConfiguration(testProject, Fixtures.Detekt.SOURCES_WITH_ERRORS)) - - TestProject.Result result = testProject - .buildAndFail('check') - - assertThat(result.logs).containsDetektNotApplied() - } - private TestProject createProjectWithZeroThreshold(File sources) { createProjectWith(sources) } private TestProject createProjectWith(File sources, int maxWarnings = 0, int maxErrors = 0) { - def testProject = projectRule.newProject() + projectRule.newProject() .withPlugin("io.gitlab.arturbosch.detekt", "1.0.0.RC6-2") .withSourceSet('main', sources) .withPenalty("""{ maxWarnings = ${maxWarnings} maxErrors = ${maxErrors} }""") - - testProject.withToolsConfig(detektConfiguration(testProject, sources)) + .withToolsConfig(detektConfiguration(sources)) } private TestProject createProjectWithoutDetekt() { @@ -130,27 +136,29 @@ class DetektIntegrationTest { }''') } - private static GString detektConfiguration(TestProject testProject, File input) { + private static String detektConfiguration(File input) { + detektWith """ + config = '${Fixtures.Detekt.RULES}' + output = "\$buildDir/reports" + // The input just needs to be configured for the tests. + // Probably detekt doesn't pick up the changed source sets. + // In a example project it was not needed. + input = "${input}" """ - detekt { - profile('main') { - config = "${Fixtures.Detekt.RULES}" - output = "${testProject.projectDir()}/build/reports" - // The input just needs to be configured for the tests. - // Probably detekt doesn't pick up the changed source sets. - // In a example project it was not needed. - input = "${input}" - } - } + } + + private static String detektConfigurationWithoutInput() { + detektWith """ + config = '${Fixtures.Detekt.RULES}' + output = "\$buildDir/reports" """ } - private static GString detektConfigurationWithoutInput(TestProject testProject) { + private static String detektWith(String mainProfile) { """ detekt { profile('main') { - config = "${Fixtures.Detekt.RULES}" - output = "${testProject.projectDir()}/build/reports" + ${mainProfile.stripIndent()} } } """ diff --git a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy index 1abeb75..5fde8cf 100644 --- a/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy +++ b/plugin/src/test/groovy/com/novoda/test/LogsSubject.groovy @@ -14,7 +14,6 @@ import static com.novoda.test.TestProject.Result.Logs; class LogsSubject extends Subject { private static final String VIOLATIONS_LIMIT_EXCEEDED = 'Violations limit exceeded' - private static final String DETEKT_NOT_APPLIED = 'The Detekt plugin is configured but not applied. Please apply the plugin in your build script.' private static final String CHECKSTYLE_VIOLATIONS_FOUND = 'Checkstyle violations found' private static final String PMD_VIOLATIONS_FOUND = 'PMD violations found' private static final String FINDBUGS_VIOLATIONS_FOUND = 'Findbugs violations found' @@ -44,8 +43,8 @@ class LogsSubject extends Subject { check().that(actual().output) } - public void containsDetektNotApplied() { - outputSubject.contains(DETEKT_NOT_APPLIED) + public void contains(log) { + outputSubject.contains(log) } public void doesNotContainLimitExceeded() { From 2ffae014e06bdd1cb09b31301c908488547a7181 Mon Sep 17 00:00:00 2001 From: Said Tahsin Dane Date: Thu, 15 Feb 2018 14:42:11 +0100 Subject: [PATCH 2/3] dry-run is not necessary since we expect the build to fail --- .../internal/detekt/DetektIntegrationTest.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index b990f0a..ff3f179 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -34,7 +34,7 @@ class DetektIntegrationTest { def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_WARNINGS) .withToolsConfig(emptyConfiguration) - .buildAndFail('check', '--dry-run') + .buildAndFail('check') assertThat(result.logs).contains(OUTPUT_NOT_DEFINED) } @@ -43,7 +43,7 @@ class DetektIntegrationTest { void shouldFailBuildOnConfigurationWhenDetektConfiguredButNotApplied() { def result = projectRule.newProject() .withToolsConfig(detektConfiguration(Fixtures.Detekt.SOURCES_WITH_ERRORS)) - .buildAndFail('check', '--dry-run') + .buildAndFail('check') assertThat(result.logs).contains(DETEKT_NOT_APPLIED) } From 6bd15d08f101cd0be21d3254288374298abb05dd Mon Sep 17 00:00:00 2001 From: Antonio Bertucci Date: Thu, 15 Feb 2018 15:57:17 +0000 Subject: [PATCH 3/3] Update test name to match project convention --- .../staticanalysis/internal/detekt/DetektIntegrationTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy index ff3f179..12379c2 100644 --- a/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy +++ b/plugin/src/test/groovy/com/novoda/staticanalysis/internal/detekt/DetektIntegrationTest.groovy @@ -29,7 +29,7 @@ class DetektIntegrationTest { } @Test - void givenOutputNotDefinedShouldFailBuildOnConfiguration() { + void shouldFailBuildOnConfigurationWhenNoOutputNotDefined() { def emptyConfiguration = detektWith("") def result = createProjectWithZeroThreshold(Fixtures.Detekt.SOURCES_WITH_WARNINGS)