Fix build issues #6424

3flex · 2023-08-23T11:05:33Z

Fix build issues with the Gradle plugin by:

Ensuring artifacts are published to Maven Local before running functional tests
Moving mavenLocal to the first position in the repositories block in build files in the functional tests
Fixing issues that weren't caught when CI was run on recent PRs due to the faulty setup

codecov · 2023-08-23T11:17:25Z

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e6303e3) 85.07% compared to head (c683661) 85.07%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##               main    #6424   +/-   ##
=========================================
  Coverage     85.07%   85.07%           
- Complexity     4026     4029    +3     
=========================================
  Files           566      566           
  Lines         13162    13167    +5     
  Branches       2363     2364    +1     
=========================================
+ Hits          11197    11202    +5     
  Misses          794      794           
  Partials       1171     1171

Files Changed	Coverage Δ
...n/io/github/detekt/metrics/CyclomaticComplexity.kt	`95.52% <100.00%> (+0.20%)`	⬆️
...detekt/rules/complexity/CyclomaticComplexMethod.kt	`96.00% <100.00%> (+0.16%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

detekt-ci · 2023-08-23T11:45:13Z

	Warnings
⚠️	It looks like this PR contains functional changes without a corresponding test.
⚠️	This PR is approved with no milestone set. If merged, it won't appear in the detekt release notes.

Generated by 🚫 dangerJS against c683661

marschwar · 2023-08-24T19:54:26Z

I was going to ask this in my next PR but I guess it fits here as well. How should I handle changes that will affect the functional tests (such as the removal of the weights and maxIssues). As far as I understood the functional tests use the latest version of detekt. I did change the tests in the PR but then the build failed.

3flex · 2023-08-29T08:28:32Z

As far as I understood the functional tests use the latest version of detekt

They should, but they're not in all cases for some reason. I have a POC fix in #6415 which is ugly but works and might be the way forward. We clearly need a reliable solution.

See detekt#6408

See detekt#6323

See detekt#5870

Updates were missed in detekt#6389 due to lack of properly configured integration tests.

3flex · 2023-08-29T22:28:42Z

@detekt/maintainers please review. CI should be green going forward, and this should also catch missed updates to Gradle plugin functional test updates in future PRs.

chao2zhang · 2023-08-29T23:03:08Z

detekt-gradle-plugin/src/main/kotlin/io/gitlab/arturbosch/detekt/DetektPlugin.kt

@@ -21,6 +21,7 @@ class DetektPlugin : Plugin<Project> {
            )

        extension.reportsDir = project.extensions.getByType(ReportingExtension::class.java).file("detekt")
+        extension.basePath = project.layout.projectDirectory.asFile.absolutePath


This is an actual fix for recent PRs, right?

It's a fix, but only for tests on Windows.

When TestKit executes tests it starts the daemon from the Gradle plugin project directory but the build under test is in a different directory. That's ok building locally in most cases but on GitHub runner one of those directories is on C: but the other is on D:

There's no way to get a relative file path from one drive to another, which causes build failures. The base path is determined based on a compiled class's resource path which is on one drive, but the absolute path to the file checked by detekt is on another drive.

The CLI defaults base path to user.dir so adding this line makes it more correct when running from Gradle. Technically it should be rootDir to match current behaviour but still fix the build... I've made that change. It's more correct to use projectDir I think but that's technically a behaviour change so can do that in another PR.

Will the absolute path break cache relocation in Gradle when executed on CI vs Local builds on user's setups?

Hmm yes it will due to the usage of @Input on that property, I did check the annotation on the task but didn't think of the consequences. I'll submit a PR to fix.

Here's a potentially interesting way to validate this (never tried it myself, just read it somewhere):
https://github.com/gradle/gradle-enterprise-build-validation-scripts/blob/main/Gradle.md#structure
could run on a fully standalone integration test (like this) after the snapshot is published to maven local similar to what you did in this PR?

BraisGabin · 2023-08-29T23:36:40Z

.github/workflows/pre-merge.yaml

+        uses: gradle/gradle-build-action@243af859f8ca30903d9d7f7936897ca0358ba691 # v2
+        with:
+          gradle-home-cache-cleanup: true
+          arguments: publishToMavenLocal


Is this better than make the functionalTest depend on publishToMavebLocal?

You're right, but I couldn't find a way to do that while the plugin is an included build, so this seemed the next best option.

Skipping this step when running locally may lead to confusion (though that's no different to the current situation) but CI will at least be ok.

Could a similar switcheroo be done here? cashapp/paparazzi#924

I didn't know there was prior art but there's this draft PR open already to do the same: #6415

I might look at what they did in cashapp org since they often end up setting unofficial standard ways of doing things.

3flex · 2023-08-30T07:51:28Z

I'll go ahead and merge to unblock other PRs.

Missed in detekt#6424

Missed in #6424

* Remove maxIssues from config in Gradle functional tests See detekt#6408 * Set basepath to project dir by default See detekt#6323 * Fix AbstractClassCanBeInterface violation See detekt#5870 * Fix expected output Updates were missed in detekt#6389 due to lack of properly configured integration tests. * Prioritise Maven Local in repo list for functional tests * Publish artifacts to Maven Local before building detekt * Set root project directory as default base path in DGP

Missed in detekt#6424

vercel bot deployed to Preview August 23, 2023 11:15 View deployment

detekt-ci added the gradle-plugin label Aug 23, 2023

3flex marked this pull request as draft August 23, 2023 11:45

vercel bot deployed to Preview August 23, 2023 11:49 View deployment

3flex force-pushed the fix-build branch from bb6d4e7 to 1b2e3ea Compare August 23, 2023 12:40

vercel bot deployed to Preview August 23, 2023 12:45 View deployment

3flex force-pushed the fix-build branch from 1b2e3ea to c0a5836 Compare August 29, 2023 08:26

vercel bot deployed to Preview August 29, 2023 08:31 View deployment

detekt-ci added the build label Aug 29, 2023

vercel bot deployed to Preview August 29, 2023 08:56 View deployment

detekt-ci added the ci label Aug 29, 2023

vercel bot deployed to Preview August 29, 2023 11:15 View deployment

3flex force-pushed the fix-build branch from 56a887d to 3f46d52 Compare August 29, 2023 11:28

vercel bot deployed to Preview August 29, 2023 11:33 View deployment

3flex marked this pull request as ready for review August 29, 2023 21:52

3flex added 6 commits August 30, 2023 07:52

Remove maxIssues from config in Gradle functional tests

bc9b24b

See detekt#6408

Set basepath to project dir by default

a2de419

See detekt#6323

Fix AbstractClassCanBeInterface violation

db8a371

See detekt#5870

Fix expected output

a1410ec

Updates were missed in detekt#6389 due to lack of properly configured integration tests.

Prioritise Maven Local in repo list for functional tests

072b0b5

Publish artifacts to Maven Local before building detekt

190fc2f

3flex force-pushed the fix-build branch from 3f46d52 to 190fc2f Compare August 29, 2023 21:52

vercel bot deployed to Preview August 29, 2023 21:57 View deployment

chao2zhang reviewed Aug 29, 2023

View reviewed changes

chao2zhang approved these changes Aug 29, 2023

View reviewed changes

BraisGabin reviewed Aug 29, 2023

View reviewed changes

Set root project directory as default base path in DGP

c683661

vercel bot deployed to Preview August 30, 2023 00:52 View deployment

3flex merged commit 57285b4 into detekt:main Aug 30, 2023
23 checks passed

3flex added a commit to 3flex/detekt that referenced this pull request Aug 30, 2023

Publish to Maven Local before building detekt

32cf393

Missed in detekt#6424

3flex mentioned this pull request Aug 30, 2023

Publish to Maven Local before building detekt #6441

Merged

3flex added a commit that referenced this pull request Aug 30, 2023

Publish to Maven Local before building detekt (#6441)

1616f6d

Missed in #6424

3flex deleted the fix-build branch August 30, 2023 08:48

3flex mentioned this pull request Oct 29, 2023

Remove dependency on mavenLocal() for functionalTests #6060

Open

jprinet mentioned this pull request Nov 3, 2023

Ignore flakiness in :detekt-gradle-plugin:functionalTest gradle/develocity-oss-projects#81

Merged

mgroth0 pushed a commit to mgroth0/detekt that referenced this pull request Feb 11, 2024

Publish to Maven Local before building detekt (detekt#6441)

01f8fbe

Missed in detekt#6424

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix build issues #6424

Fix build issues #6424

3flex commented Aug 23, 2023 •

edited

codecov bot commented Aug 23, 2023 •

edited

detekt-ci commented Aug 23, 2023 •

edited

marschwar commented Aug 24, 2023

3flex commented Aug 29, 2023

3flex commented Aug 29, 2023

chao2zhang Aug 29, 2023

3flex Aug 30, 2023 •

edited

TWiStErRob Aug 30, 2023

3flex Aug 30, 2023

TWiStErRob Aug 30, 2023

BraisGabin Aug 29, 2023

3flex Aug 30, 2023 •

edited

TWiStErRob Aug 30, 2023

3flex Aug 30, 2023

3flex commented Aug 30, 2023

Fix build issues #6424

Fix build issues #6424

Conversation

3flex commented Aug 23, 2023 • edited

codecov bot commented Aug 23, 2023 • edited

Codecov Report

detekt-ci commented Aug 23, 2023 • edited

marschwar commented Aug 24, 2023

3flex commented Aug 29, 2023

3flex commented Aug 29, 2023

chao2zhang Aug 29, 2023

Choose a reason for hiding this comment

3flex Aug 30, 2023 • edited

Choose a reason for hiding this comment

TWiStErRob Aug 30, 2023

Choose a reason for hiding this comment

3flex Aug 30, 2023

Choose a reason for hiding this comment

TWiStErRob Aug 30, 2023

Choose a reason for hiding this comment

BraisGabin Aug 29, 2023

Choose a reason for hiding this comment

3flex Aug 30, 2023 • edited

Choose a reason for hiding this comment

TWiStErRob Aug 30, 2023

Choose a reason for hiding this comment

3flex Aug 30, 2023

Choose a reason for hiding this comment

3flex commented Aug 30, 2023

3flex commented Aug 23, 2023 •

edited

codecov bot commented Aug 23, 2023 •

edited

detekt-ci commented Aug 23, 2023 •

edited

3flex Aug 30, 2023 •

edited

3flex Aug 30, 2023 •

edited