Skip to content

MagicNumber DslGradleRunner test fix#9245

Merged
schalkms merged 2 commits into
detekt:mainfrom
FletchMcKee:magicnumber-dslgradlerunner-fix
Apr 14, 2026
Merged

MagicNumber DslGradleRunner test fix#9245
schalkms merged 2 commits into
detekt:mainfrom
FletchMcKee:magicnumber-dslgradlerunner-fix

Conversation

@FletchMcKee
Copy link
Copy Markdown
Contributor

@FletchMcKee FletchMcKee commented Apr 13, 2026

This was easy to miss with changing the default of ignorePropertyDeclaration from false to true, but some of these Gradle tests rely on expecting this smellyConstant to continue being a code smell which isn't the case anymore by default. Changed it to a method so that it should report when withFinding is true.

Edit: Changing it to a method caused other issues, so the easiest fix I can think of for now is converting back to a constant and setting ignorePropertyDeclaration to false since this is what these test cases expected. Updated those test cases to make sure it uses the configFileContent by passing a file name through withConfigFile.

This was easy to miss with changing the default of `ignorePropertyDeclaration` from false to true, but some of these Gradle tests rely on expecting this `smellyConstant` to continue being a code smell which isn't the case anymore by default. Changed it to a method so that it should report when withFinding is true.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.84%. Comparing base (e6574d0) to head (508e17a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #9245   +/-   ##
=========================================
  Coverage     84.84%   84.84%           
  Complexity     4429     4429           
=========================================
  Files           569      569           
  Lines         12299    12299           
  Branches       2707     2707           
=========================================
  Hits          10435    10435           
  Misses          688      688           
  Partials       1176     1176           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Changing it to a method caused other issues, so the easiest fix I can think of for now is converting back to a constant and setting `ignorePropertyDeclaration` to false since this is what these test cases expected. Updated those test cases to make sure it uses the configFileContent by passing a file name through `withConfigFile`.
@FletchMcKee
Copy link
Copy Markdown
Contributor Author

This should also allow #9244 to pass if merged.

@schalkms schalkms merged commit 2f21f5e into detekt:main Apr 14, 2026
20 of 21 checks passed
@detekt-ci
Copy link
Copy Markdown
Collaborator

Warnings
⚠️ This PR is approved with no milestone set. If merged, it won't appear in the detekt release notes.

Generated by 🚫 dangerJS against 508e17a

@schalkms
Copy link
Copy Markdown
Member

I triggered a rebase including test runs for the linked PR. Thank you for the hint 😊

@schalkms schalkms added this to the 2.0.0 milestone Apr 14, 2026
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.

3 participants