Repository navigation
Split Akka HTTP and Spring WebFlux tests across CI shards #12848
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -183,3 +183,32 @@ tasks.named("compileVersion102Scala213TestGroovy", GroovyCompile) { | |||||||||||||||||
| tasks.named("compileLatestDepTestGroovy", GroovyCompile) { | ||||||||||||||||||
| classpath += files(tasks.named('compileLatestDepTestScala').map { it.destinationDirectory }) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Split the large Akka HTTP project into independently scheduled CI units. Existing test tasks | ||||||||||||||||||
| // remain unchanged for local builds; only sharded CI delegates them through virtual projects. | ||||||||||||||||||
| if (rootProject.providers.gradleProperty("slot").isPresent()) { | ||||||||||||||||||
| tasks.named("allTests") { | ||||||||||||||||||
| setDependsOn([]) | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+189
to
+192
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: This kinda break the model of declaring aggregating tasks. I.e. clearing these dependencies replaces automatic suite discovery with a fixed list. A newly added suite can therefore run locally but be omitted from sharded CI until this list is updated. I understand this happens only when there's a The WebFlux split has the same issue. |
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| def ciAkkaTestShards = [ | ||||||||||||||||||
| 'akka-http-base-tests': [tasks.named('baseTest'), tasks.named('lagomTest')], | ||||||||||||||||||
| 'akka-http-base-forked-tests': [tasks.named('baseTestForkedTest'), tasks.named('iastTest')], | ||||||||||||||||||
| 'akka-http-version-101-tests': [tasks.named('version101Test'), tasks.named('version101IastTest')], | ||||||||||||||||||
| 'akka-http-version-101-forked-tests': [tasks.named('version101ForkedTest'), tasks.named('version102IastTest')], | ||||||||||||||||||
| 'akka-http-version-102-tests': [tasks.named('version102Scala213Test')], | ||||||||||||||||||
| ] | ||||||||||||||||||
|
|
||||||||||||||||||
| def ciShardProjectPaths = rootProject.extensions.extraProperties.get('ciShardProjectPaths') as Map | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: While Gradle support storing objects there, using extra properties is most likely the wrong approach, which is presently the case. Possibly I would rather go toward a build service. In this case there is two concrete problems
|
||||||||||||||||||
| ciAkkaTestShards.each { childName, testTasks -> | ||||||||||||||||||
| def childPath = ":dd-java-agent:instrumentation:akka:akka-http:akka-http-10.0:$childName" | ||||||||||||||||||
| testTasks.each { testTask -> | ||||||||||||||||||
| ciShardProjectPaths[testTask.get().path] = childPath | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: This reealizes a test task early breaking the Gradle's task configuration best practices. This loop runs during every build configuration, including unrelated invocations like
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
| project(childPath) { | ||||||||||||||||||
| tasks.register("allTests") { | ||||||||||||||||||
| dependsOn(testTasks) | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+210
to
+212
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (non-blocking): Could we give the new child aggregates a verification group and a description? The 32 child
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -137,3 +137,76 @@ tasks.named("latestDepTest", Test) { | |||||||||||||||||||||||||||||||||||||||
| dependsOn "latestBoot24Test" | ||||||||||||||||||||||||||||||||||||||||
| dependsOn "latestBoot2LatestTest" | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| def ciWebfluxBaseGroups = [ | ||||||||||||||||||||||||||||||||||||||||
| webfluxClientATest: [ | ||||||||||||||||||||||||||||||||||||||||
| '**/SpringWebfluxHttpClientBasicTest.class', | ||||||||||||||||||||||||||||||||||||||||
| '**/SpringWebfluxHttpClientBuilderReuseTest.class', | ||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||
| webfluxClientBTest: [ | ||||||||||||||||||||||||||||||||||||||||
| '**/SpringWebfluxHttpClientDoAfterTerminateTest.class', | ||||||||||||||||||||||||||||||||||||||||
| '**/SpringWebfluxHttpClientDoOnSuccessTest.class', | ||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||
| webfluxClientCTest: [ | ||||||||||||||||||||||||||||||||||||||||
| '**/SpringWebfluxHttpClientDoOnTerminateTest.class', | ||||||||||||||||||||||||||||||||||||||||
| '**/SpringWebfluxHttpClientFilterBuilderReuseTest.class', | ||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||
| webfluxClientDTest: [ | ||||||||||||||||||||||||||||||||||||||||
| '**/SpringWebfluxHttpClientFilterTest.class', | ||||||||||||||||||||||||||||||||||||||||
| '**/SpringWebfluxHttpClientFiltersRemovedTest.class', | ||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| ciWebfluxBaseGroups.each { taskName, includes -> | ||||||||||||||||||||||||||||||||||||||||
| tasks.register(taskName, Test) { | ||||||||||||||||||||||||||||||||||||||||
| group = LifecycleBasePlugin.VERIFICATION_GROUP | ||||||||||||||||||||||||||||||||||||||||
| useJUnitPlatform() | ||||||||||||||||||||||||||||||||||||||||
| testClassesDirs = sourceSets.test.output.classesDirs | ||||||||||||||||||||||||||||||||||||||||
| classpath = sourceSets.test.runtimeClasspath | ||||||||||||||||||||||||||||||||||||||||
| setIncludes(includes) | ||||||||||||||||||||||||||||||||||||||||
| onlyIf("CI sharding is enabled") { rootProject.providers.gradleProperty("slot").isPresent() } | ||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Could we capture the sharding flag locally during task configuration and read that value or provider in This predicate reads This extends an existing configuration-cache limitation rather than introducing a new failure in the current CI pipeline, but it would be useful to keep the new wiring compatible as that work progresses. |
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| tasks.register('webfluxClientOtherTest', Test) { | ||||||||||||||||||||||||||||||||||||||||
| group = LifecycleBasePlugin.VERIFICATION_GROUP | ||||||||||||||||||||||||||||||||||||||||
| useJUnitPlatform() | ||||||||||||||||||||||||||||||||||||||||
| testClassesDirs = sourceSets.test.output.classesDirs | ||||||||||||||||||||||||||||||||||||||||
| classpath = sourceSets.test.runtimeClasspath | ||||||||||||||||||||||||||||||||||||||||
| ciWebfluxBaseGroups.values().flatten().each { pattern -> exclude pattern } | ||||||||||||||||||||||||||||||||||||||||
| failOnNoDiscoveredTests = false | ||||||||||||||||||||||||||||||||||||||||
| onlyIf("CI sharding is enabled") { rootProject.providers.gradleProperty("slot").isPresent() } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| // In sharded CI, replace the original base task with disjoint class groups and expose every group | ||||||||||||||||||||||||||||||||||||||||
| // through a virtual child project. Local allTests and check retain their normal behavior. | ||||||||||||||||||||||||||||||||||||||||
| if (rootProject.providers.gradleProperty("slot").isPresent()) { | ||||||||||||||||||||||||||||||||||||||||
| tasks.named("allTests") { | ||||||||||||||||||||||||||||||||||||||||
| setDependsOn([]) | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| tasks.named("test", Test) { | ||||||||||||||||||||||||||||||||||||||||
| enabled = false | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| def ciWebfluxTestShards = [ | ||||||||||||||||||||||||||||||||||||||||
| 'webflux-client-a-test': [tasks.named('webfluxClientATest')], | ||||||||||||||||||||||||||||||||||||||||
| 'webflux-client-b-test': [tasks.named('webfluxClientBTest')], | ||||||||||||||||||||||||||||||||||||||||
| 'webflux-client-c-test': [tasks.named('webfluxClientCTest')], | ||||||||||||||||||||||||||||||||||||||||
| 'webflux-client-d-test': [tasks.named('webfluxClientDTest')], | ||||||||||||||||||||||||||||||||||||||||
| 'webflux-client-other-test': [tasks.named('webfluxClientOtherTest')], | ||||||||||||||||||||||||||||||||||||||||
| 'webflux-forked-iast-tests': [tasks.named('forkedTest'), tasks.named('iastTest')], | ||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| def ciShardProjectPaths = rootProject.extensions.extraProperties.get('ciShardProjectPaths') as Map | ||||||||||||||||||||||||||||||||||||||||
| ciWebfluxTestShards.each { childName, testTasks -> | ||||||||||||||||||||||||||||||||||||||||
| def childPath = ":dd-java-agent:instrumentation:spring:spring-webflux:spring-webflux-5.0:$childName" | ||||||||||||||||||||||||||||||||||||||||
| testTasks.each { testTask -> | ||||||||||||||||||||||||||||||||||||||||
| ciShardProjectPaths[testTask.get().path] = childPath | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| project(childPath) { | ||||||||||||||||||||||||||||||||||||||||
| tasks.register("allTests") { | ||||||||||||||||||||||||||||||||||||||||
| dependsOn(testTasks) | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Should
For a direct The relevant root selection is here: dd-trace-java/buildSrc/src/main/kotlin/datadog/gradle/plugin/ci/CIJobsExtensions.kt Lines 110 to 128 in 6bf07f7
|
||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| // CI-only virtual project; its allTests task is registered by the parent project. |
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.
suggestion (non-blocking): I'm really not keep on having virtual projects, but if we go this way, could we use
build.gradle.ktsfor the new child build files if these scheduling projects remain?Also for all the other 32 comment-only build files, so there is no Groovy logic to migrate in them. This follows Gradle's recommendation for new build scripts; the existing module scripts can remain as they are.
Gradle's Kotlin DSL guidance.