-
Notifications
You must be signed in to change notification settings - Fork 427
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
Increase test processes to start in parallel #2953
Conversation
@@ -22,6 +22,12 @@ java { | |||
tasks.withType<Test>().configureEach { | |||
useJUnitPlatform() | |||
|
|||
maxParallelForks = if (System.getenv("CI") != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or we can read dokka_integration_test_parallelism
like
dokka/build-logic/src/main/kotlin/org/jetbrains/conventions/dokka-integration-test.gradle.kts
Lines 54 to 56 in 3ea4142
project.properties["dokka_integration_test_parallelism"]?.toString()?.toIntOrNull()?.let { parallelism -> | |
maxParallelForks = parallelism | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are more cores in TeamCity runners, maybe we can use
maxParallelForks = if (System.getenv("GITHUB_ACTIONS") != null) {
Runtime.getRuntime().availableProcessors()
} else {
(Runtime.getRuntime().availableProcessors() / 2).takeIf { it > 0 } ?: 1
}
for both tests and integrationTests, get rid of dokka_integration_test_parallelism
property.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally set out some time to test it. I think it makes perfect sense to do this for unit tests, absolutely no questions here. But I wasn't sure what sort of an impact it would have on our integration tests. I have a totally overspec'd workstation (16C, 32T, 128GB RAM) to draw any conclusions, but running the
I actually wonder what would happen if I didn't have enough memory - would the integration tests fail, or would the time increase drastically because of GC problems? I'm trying to trigger integration tests on teamcity for this PR, but teamcity is not picking up the branch changes like it did before... Our admins might've introduced some security policies, so I'll have to resolve that first, which might take time... I'd be happy to merge this change for unit tests only though, if you open a separate PR or change this one |
c3159f0
to
c9f4aed
Compare
(looks like TeamCity was simply lagging yesterday, integration tests work now) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
I'm still interested in having (or at least testing) this change for integration tests, so @Goooler if you want and have the time, feel free to open a separate PR :)
More context see detekt/detekt#5944.
https://docs.gradle.org/7.6/userguide/performance.html#execute_tests_in_parallel