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

The tests are slow #2902

Closed
BraisGabin opened this issue Jul 27, 2020 · 9 comments · Fixed by #2921
Closed

The tests are slow #2902

BraisGabin opened this issue Jul 27, 2020 · 9 comments · Fixed by #2921
Labels
bug build housekeeping Marker for housekeeping tasks and refactorings
Milestone

Comments

@BraisGabin
Copy link
Member

The tests take around 25 seconds to start running an example:

Grabación de pantalla 2020-07-27 a las 22 24 51

It's not a complete record but you get the idea. Why does it take more than 20 seconds to test :detekt-report-xml?

@cortinico
Copy link
Member

The tests take around 25 seconds to start running an example:

Same. I thought it was a misconfiguration of mine, but apparently not.
@arturbosch was doing profiling so maybe has more insights on this

@schalkms
Copy link
Member

The same holds true on my machine.

@arturbosch
Copy link
Member

arturbosch commented Jul 28, 2020

Even running :detekt-report-xml:test without any detekt-test dependencies/accidentally creating KtCompiler the test discovery is slow.

 g/r/detekt/detekt-report-xml{} $ gradle cleanTest test -x :detekt-gradle-plugin:test -Dorg.gradle.caching=false -Dorg.gradle.unsafe.watch-fs=false -Dorg.gradle.parallel=false       4s 776ms

I've used this branch if someone is interested: https://github.com/detekt/detekt/pull/new/debug-test-performance

Reference: spekframework/spek#782

It's actually hard to profile this as the Gradle Daemon creates a Gradle Main Process which creates a Gradle Worker Main Process ...

Edit: I understand that testcase discovery in Spek for modules creating KotlinEnvironments in tests can be high (init lambda in constructor, however now we only have a dependency on detekt-api which pulls the whole kotlin-compiler into the classpath.
Maybe classpath scanning of Junit is the problem here ... ?

@arturbosch
Copy link
Member

arturbosch commented Jul 30, 2020

I've migrated the report-xml and report-txt module tests to junit jupiter test engine and the test discovery got reduced by 2 seconds (https://github.com/detekt/detekt/pull/new/debug-tests):

With Spek:

https://scans.gradle.com/s/mdxkbujmuzqw4/tests

2020-07-30-112828_992x416_scrot

With Junit Jupiter:

https://scans.gradle.com/s/udwjw43v5ravw/tests

2020-07-30-112936_848x399_scrot

Edit:

With spekframework/spek#905

https://scans.gradle.com/s/dvvdyqdjwq77q/tests

2020-07-30-125723_1019x449_scrot

@arturbosch
Copy link
Member

#2921 introduces many improvements.

The big penalty for us still is and will be: kotlin environment first inits (2s) + mockk first inits (2s) in many modules (multiplier) building in parallel (slowing down each other).

@arturbosch arturbosch added this to the 1.10.1 milestone Aug 2, 2020
@arturbosch arturbosch added the housekeeping Marker for housekeeping tasks and refactorings label Aug 2, 2020
@BraisGabin
Copy link
Member Author

#2921 introduces many improvements.

The big penalty for us still is and will be: kotlin environment first inits (2s) + mockk first inits (2s) in many modules (multiplier) building in parallel (slowing down each other).

At least the really fast tests that don't need does things will run fast. This will help to keep more threads free or don't the real work.

2s for mockk? We don't need nearly any mock. I vote to remove them and write fakes.

@schalkms
Copy link
Member

schalkms commented Aug 2, 2020

Maybe we could use another mocking library.
This are the results from a quick search.

➜  detekt git:(master) grep -R -I "mockk" .
./detekt-report-html/src/test/kotlin/io/github/detekt/report/html/HtmlOutputReportSpec.kt:import io.mockk.every
./detekt-report-html/src/test/kotlin/io/github/detekt/report/html/HtmlOutputReportSpec.kt:import io.mockk.mockk
./detekt-report-html/src/test/kotlin/io/github/detekt/report/html/HtmlOutputReportSpec.kt:    val ktElementMock = mockk<KtElement>()
./detekt-report-html/src/test/kotlin/io/github/detekt/report/html/HtmlOutputReportSpec.kt:    val psiFileMock = mockk<PsiFile>()
./detekt-bom/build.gradle.kts:        api("io.mockk:mockk:1.10.0")
./docs/pages/changelog 1.x.x.md:- Introduces mocking library 'mockk' - [#2055](https://github.com/detekt/detekt/pull/2055)
./buildSrc/src/main/kotlin/commons.gradle.kts:        testImplementation("io.mockk:mockk")
./detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResultSpec.kt:import io.mockk.every
./detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResultSpec.kt:import io.mockk.mockk
./detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/baseline/BaselineFilteredResultSpec.kt:        val finding = mockk<Finding>()
./detekt-metrics/src/test/kotlin/io/github/detekt/metrics/ComplexityReportGeneratorSpec.kt:import io.mockk.every
./detekt-metrics/src/test/kotlin/io/github/detekt/metrics/ComplexityReportGeneratorSpec.kt:import io.mockk.mockk
./detekt-metrics/src/test/kotlin/io/github/detekt/metrics/ComplexityReportGeneratorSpec.kt:            val finding = mockk<Finding>()

@cortinico
Copy link
Member

Maybe we could use another mocking library.

Adding this as it's relevant for the discussion: mockk/mockk#13
I'm using Mockk in another project and so far I'm not slowed down by perfs. But in our case test spin up time seems to be a significant bottleneck so we should probably investigate using mockito or something else.

@arturbosch
Copy link
Member

arturbosch commented Aug 3, 2020

Imo we can safely remove mockk in our three uses due to the discovery of Gradle test fixtures :D

I think we only use it to mock Finding or Entity.

Note only the first usage of mocking is slow, after it's loaded it is okay.
If we safe a lot of work with mocking, mocking is the way. If it slows down starting a single test in debugging -> let's remove mocking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug build housekeeping Marker for housekeeping tasks and refactorings
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants