Skip to content
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.

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

Better error classification in Gradle Enterprise. #4586

Merged
merged 4 commits into from
Feb 19, 2022

Conversation

runningcode
Copy link
Contributor

Use Lint failed instead of Build failed in exception

Fixes #4585

Correctly classified error

Incorrectly classified error

Use `Lint failed` instead of `Build failed` in exception

Fixes detekt#4585
@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #4586 (cdc5511) into main (f15ce50) will increase coverage by 84.49%.
The diff coverage is 100.00%.

❗ Current head cdc5511 differs from pull request most recent head c29ddea. Consider uploading reports for the commit c29ddea to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #4586       +/-   ##
===========================================
+ Coverage        0   84.49%   +84.49%     
- Complexity      0     3335     +3335     
===========================================
  Files           0      481      +481     
  Lines           0    11147    +11147     
  Branches        0     2040     +2040     
===========================================
+ Hits            0     9419     +9419     
- Misses          0      697      +697     
- Partials        0     1031     +1031     
Impacted Files Coverage Δ
...lab/arturbosch/detekt/core/config/MaxIssueCheck.kt 100.00% <100.00%> (ø)
...b/arturbosch/detekt/rules/empty/EmptyWhileBlock.kt 75.00% <0.00%> (ø)
...arturbosch/detekt/core/baseline/BaselineHandler.kt 95.00% <0.00%> (ø)
...gitlab/arturbosch/detekt/rules/style/MayBeConst.kt 81.42% <0.00%> (ø)
...rturbosch/detekt/rules/style/NewLineAtEndOfFile.kt 100.00% <0.00%> (ø)
...arturbosch/detekt/rules/empty/EmptyDoWhileBlock.kt 75.00% <0.00%> (ø)
...or/collection/exception/InvalidIssueDeclaration.kt 100.00% <0.00%> (ø)
...in/kotlin/io/gitlab/arturbosch/detekt/api/Issue.kt 88.88% <0.00%> (ø)
...lin/io/github/detekt/report/xml/XmlOutputReport.kt 100.00% <0.00%> (ø)
...in/io/gitlab/arturbosch/detekt/rules/Traversing.kt 42.85% <0.00%> (ø)
... and 472 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f15ce50...c29ddea. Read the comment docs.

@cortinico
Copy link
Member

I'd change to "Analysis failed" as "Lint" is an overloaded term in the Android space

@runningcode
Copy link
Contributor Author

@cortinico I've changed it. It seems that some of the tests that are failing are not picking up the new detekt artifact. Do you know why?

@cortinico
Copy link
Member

It seems that some of the tests that are failing are not picking up the new detekt artifact. Do you know why?

@3flex can tell you more here. The issue is that our build setup is configured to don't use composite builds (I never recall why). So essentially you're testing against the latest released version of the Gradle plugin (as the failing tests are only those on :detekt-gradle-plugin:functionalTest).

Unless we switch to use the included build here:

include("detekt-gradle-plugin")

We should probably keep the test update as a draft PR and merge it once we're about to release.

@@ -66,7 +66,7 @@ internal class DefaultCliInvoker(
}
}

private fun isBuildFailure(msg: String) = "Analysis failed with" in msg && "issues" in msg
private fun isBuildFailure(msg: String) = "Build failed with" in msg && "issues" in msg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cortinico I had to revert this change to the Gradle plugin since the failure detection would break. Is the Gradle plugin published together with the detekt-core artifact?

Copy link
Member

Choose a reason for hiding this comment

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

Yes they are 👍

@runningcode
Copy link
Contributor Author

Here is the follow up PR: #4588

@cortinico cortinico added this to the 1.20.0 milestone Feb 19, 2022
@chao2zhang chao2zhang merged commit cf4b7bd into detekt:main Feb 19, 2022
@runningcode runningcode deleted the no/lint-error branch February 20, 2022 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prefix exception message with Lint failed or Analysis failed
3 participants