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

evaluateViolations doesn't stop w/ errors? #221

Closed
kenyee opened this issue Jan 2, 2020 · 8 comments
Closed

evaluateViolations doesn't stop w/ errors? #221

kenyee opened this issue Jan 2, 2020 · 8 comments

Comments

@kenyee
Copy link

kenyee commented Jan 2, 2020

Not sure how to debug this so I'm hoping someone has seen this.
This is using the latest 1.2 version of the plugin.

When I run ./gradlew evaluateViolations, it claims the build is successful. When I do a --dry-run, it seems to run the correct tasks:
:app:ktlintDebugSourceSetCheck SKIPPED
:app:collectKtlintDebugViolations SKIPPED
:app:ktlintMainSourceSetCheck SKIPPED
:app:collectKtlintMainViolations SKIPPED
:app:ktlintStandardDebugSourceSetCheck SKIPPED
:app:collectKtlintStandardDebugViolations SKIPPED
:app:ktlintStandardSourceSetCheck SKIPPED
:app:collectKtlintStandardViolations SKIPPED
:app:collectKtlintStandardDebugVariantViolations SKIPPED

However, it's not failing properly. E.g., when I run detekt manually via ./gradlew detekt, there are a lot of errors. The config is pretty standard with the failure config like this:

staticAnalysis {
    penalty {
        maxErrors = 0
        maxWarnings = 0
    }
@tasomaniac
Copy link
Contributor

I just had a look at our integration tests for Detekt. We don't have it for recent 1.x versions. All I can think of is that if Detekt changed their XML output format and we are failing to parse errors.

@tasomaniac
Copy link
Contributor

Just extended the test suite to include Detekt 1.3.1. Let's see what happens
#222

Is this the version we use at Wayfair?

@tasomaniac
Copy link
Contributor

Wait, now I had a closer look at your dry run output. It only includes ktlint tasks. Not detekt!

@kenyee
Copy link
Author

kenyee commented Jan 2, 2020

yep, Wayfair. You're right..detekt isn't in the dry run list :-(
The static-analysis.gradle file looks like it's configuring it:

    detekt {
        config = files(configFile('quality/detekt/detekt-config.yml'))
        baseline = configFile('quality/detekt/detekt-baseline.xml')
    }

the docs here claim it needs another profile lambda:
https://github.com/novoda/gradle-static-analysis-plugin/blob/master/docs/advanced-usage.md
but adding that causes gradle to fail with:
Could not find method profile() for arguments [main, static_analysis_azg1haq43bq1kq7hni2bf37ok$_run_closure1$_closure7$_closure12@1946e804] on object of type io.gitlab.arturbosch.detekt.extensions.DetektExtension.

@tasomaniac
Copy link
Contributor

We updated detekt docs but missed this one. Profile is very very old function in detekt Gradle plugin. You basically should configure it by following Detekt tutorials. The important thing is that it should be inside staticAnalysis { }

@kenyee
Copy link
Author

kenyee commented Jan 2, 2020

it's definitely inside staticAnalysis {}

So next question is: how do I debug why the detekt task is not added?

@tasomaniac
Copy link
Contributor

For some reason the opened PR didn't trigger the CI.

The best would be to try if it can be reproduced in the sample project in this repository (using the same Detekt version).

You can debug by adding static analysis plugin as a composite build. Sample uses that approach. By doing that Gradle will replace the dependency with the sources.

@tasomaniac
Copy link
Contributor

Putting the configuration inside afterEvaluate fixed this issue. Especially if the tools like Detekt or Ktlint start to use afterEvaluate themselves, we may need to use it internally too.

Just putting afterEvaluate inside this plugin would fix it. But I would like to investigate the root cause of this and try to find a better solution if possible.

Leaving this open for now. It would be good to find a way to reproduce this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants