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

Convert DetektJvmSpec to use ProjectBuilder #4075

Merged
merged 2 commits into from
Oct 3, 2021

Conversation

3flex
Copy link
Member

@3flex 3flex commented Aug 27, 2021

See #3778. This reduced execution of the DetektJvmSpec test by about 25% on my machine.

@codecov
Copy link

codecov bot commented Aug 27, 2021

Codecov Report

Merging #4075 (ea1274e) into main (cf1d830) will increase coverage by 0.06%.
The diff coverage is n/a.

❗ Current head ea1274e differs from pull request most recent head 3a306c2. Consider uploading reports for the commit 3a306c2 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##               main    #4075      +/-   ##
============================================
+ Coverage     83.45%   83.51%   +0.06%     
- Complexity     3185     3186       +1     
============================================
  Files           463      461       -2     
  Lines          9095     9095              
  Branches       1768     1768              
============================================
+ Hits           7590     7596       +6     
+ Misses          571      570       -1     
+ Partials        934      929       -5     
Impacted Files Coverage Δ
...detekt/rules/style/UnderscoresInNumericLiterals.kt 81.08% <0.00%> (-0.50%) ⬇️
...etekt/generator/printer/defaultconfig/Exclusion.kt 96.66% <0.00%> (-0.11%) ⬇️
...gitlab/arturbosch/detekt/core/tooling/Lifecycle.kt 100.00% <0.00%> (ø)
...itlab/arturbosch/detekt/core/extensions/Loading.kt 20.00% <0.00%> (ø)
...tlab/arturbosch/detekt/core/reporting/Reporting.kt 94.11% <0.00%> (ø)
...ch/detekt/core/reporting/console/FindingsReport.kt 100.00% <0.00%> (ø)
.../core/reporting/console/FileBasedFindingsReport.kt 100.00% <0.00%> (ø)
...t/generator/printer/defaultconfig/ConfigPrinter.kt 92.10% <0.00%> (ø)
...etekt/core/reporting/console/LiteFindingsReport.kt
...t/core/reporting/console/AbstractFindingsReport.kt
... and 7 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 cf1d830...3a306c2. Read the comment docs.

@chao2zhang
Copy link
Member

Pre-merge check currently fails. Is it fair to assume that the improvement comes from changing from groovy interpreter to Kotlin compiler?

@3flex
Copy link
Member Author

3flex commented Aug 28, 2021

Is it fair to assume that the improvement comes from changing from groovy interpreter to Kotlin compiler?

It's from using ProjectBuilder instead of Gradle's TestKit.

TestKit runs a full Gradle build and spins up its own daemon. ProjectBuilder is much more limited, and can't be used for functional tests, because you can't execute tasks. But for checking things like whether task input & output values are set correctly it's suitable, and faster.

@3flex 3flex added the housekeeping Marker for housekeeping tasks and refactorings label Aug 28, 2021
Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Great work 👍

Just one side note: now we're moving from actually running Gradle tasks to just setting up a project and asserting that the tasks are configured properly.

Not a big deal but if the task blows up during execution, the former task would have found it. This one not. Something to keep in mind 👍

@3flex 3flex force-pushed the projectbuilder branch 3 times, most recently from 2525cd2 to 30a68ad Compare September 18, 2021 03:32
@cortinico cortinico added this to the 1.19.0 milestone Sep 18, 2021
@chao2zhang chao2zhang merged commit b35f38e into detekt:main Oct 3, 2021
@3flex 3flex deleted the projectbuilder branch October 3, 2021 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Marker for housekeeping tasks and refactorings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants