-
Notifications
You must be signed in to change notification settings - Fork 315
Create configure_tests convention plugin
#9859
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
2194059
96f586b
7ff614b
be41722
d85b522
afa8ed9
f654867
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,147 @@ | ||||||||||||||||||
| import org.gradle.api.tasks.testing.Test | ||||||||||||||||||
| import org.gradle.api.tasks.testing.junitplatform.JUnitPlatformOptions | ||||||||||||||||||
| import org.gradle.api.services.BuildService | ||||||||||||||||||
| import org.gradle.api.services.BuildServiceParameters | ||||||||||||||||||
| import org.gradle.testing.base.TestingExtension | ||||||||||||||||||
| import org.gradle.api.plugins.jvm.JvmTestSuite | ||||||||||||||||||
| import java.time.Duration | ||||||||||||||||||
| import java.time.temporal.ChronoUnit | ||||||||||||||||||
|
|
||||||||||||||||||
| fun isTestingInstrumentation(project: Project): Boolean { | ||||||||||||||||||
| return listOf( | ||||||||||||||||||
| "junit-4.10", | ||||||||||||||||||
| "cucumber", | ||||||||||||||||||
| "cucumber-junit-4", | ||||||||||||||||||
| "junit-4.13", | ||||||||||||||||||
| "munit-junit-4", | ||||||||||||||||||
| "junit-5.3", | ||||||||||||||||||
| "junit-5.8", | ||||||||||||||||||
| "cucumber-junit-5", | ||||||||||||||||||
| "spock-junit-5", | ||||||||||||||||||
| "testng-6", | ||||||||||||||||||
| "testng-7", | ||||||||||||||||||
| "karate", | ||||||||||||||||||
| "scalatest", | ||||||||||||||||||
| "selenium", | ||||||||||||||||||
| "weaver" | ||||||||||||||||||
| ).contains(project.name) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Need concrete implementation of BuildService in Kotlin | ||||||||||||||||||
| abstract class ForkedTestLimit : BuildService<BuildServiceParameters.None> | ||||||||||||||||||
|
|
||||||||||||||||||
| val forkedTestLimit = gradle.sharedServices.registerIfAbsent("forkedTestLimit", ForkedTestLimit::class.java) { | ||||||||||||||||||
| maxParallelUsages.set(3) | ||||||||||||||||||
| } | ||||||||||||||||||
AlexeyKuznetsov-DD marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
|
|
||||||||||||||||||
| extensions.findByType(TestingExtension::class.java)?.apply { | ||||||||||||||||||
| suites.withType(JvmTestSuite::class.java).configureEach { | ||||||||||||||||||
| // Use JUnit 5 to run tests | ||||||||||||||||||
| useJUnitJupiter() | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Use lazy providers to avoid evaluating the property until it is needed | ||||||||||||||||||
| val skipTestsProvider = rootProject.providers.gradleProperty("skipTests") | ||||||||||||||||||
| val skipForkedTestsProvider = rootProject.providers.gradleProperty("skipForkedTests") | ||||||||||||||||||
| val skipFlakyTestsProvider = rootProject.providers.gradleProperty("skipFlakyTests") | ||||||||||||||||||
| val runFlakyTestsProvider = rootProject.providers.gradleProperty("runFlakyTests") | ||||||||||||||||||
| val activePartitionProvider = providers.provider { | ||||||||||||||||||
| project.extra.properties["activePartition"] as? Boolean ?: true | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+47
to
+53
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. note: Maybe, these could end-up in the extension I mentioned earlier. I'm kind if undecided there. |
||||||||||||||||||
|
|
||||||||||||||||||
| // Go through the Test tasks and configure them | ||||||||||||||||||
| tasks.withType(Test::class.java).configureEach { | ||||||||||||||||||
|
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. nitpick:
Suggested change
|
||||||||||||||||||
| enabled = activePartitionProvider.get() | ||||||||||||||||||
|
Comment on lines
+51
to
+57
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. thought: I wonder if this "trait" or "feature" should be moved over the ci convention plugin ? |
||||||||||||||||||
|
|
||||||||||||||||||
| // Disable all tests if skipTests property was specified | ||||||||||||||||||
| onlyIf { !skipTestsProvider.isPresent } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Enable force rerun of tests with -Prerun.tests.${project.name} | ||||||||||||||||||
| outputs.upToDateWhen { | ||||||||||||||||||
| !rootProject.providers.gradleProperty("rerun.tests.${project.name}").isPresent | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Avoid executing classes used to test testing frameworks instrumentation | ||||||||||||||||||
| if (isTestingInstrumentation(project)) { | ||||||||||||||||||
| exclude("**/TestSucceed*") | ||||||||||||||||||
| exclude("**/TestFailed*") | ||||||||||||||||||
| exclude("**/TestFailedWithSuccessPercentage*") | ||||||||||||||||||
| exclude("**/TestError*") | ||||||||||||||||||
| exclude("**/TestSkipped*") | ||||||||||||||||||
| exclude("**/TestSkippedClass*") | ||||||||||||||||||
| exclude("**/TestInheritance*", "**/BaseTestInheritance*") | ||||||||||||||||||
| exclude("**/TestFactory*") | ||||||||||||||||||
| exclude("**/TestParameterized*") | ||||||||||||||||||
| exclude("**/TestRepeated*") | ||||||||||||||||||
| exclude("**/TestTemplate*") | ||||||||||||||||||
| exclude("**/TestDisableTestTrace*") | ||||||||||||||||||
| exclude("**/TestAssumption*", "**/TestSuiteSetUpAssumption*") | ||||||||||||||||||
| exclude("**/TestUnskippable*") | ||||||||||||||||||
| exclude("**/TestWithSetup*") | ||||||||||||||||||
sarahchen6 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Split up tests that want to run forked in their own separate JVM for generated tasks | ||||||||||||||||||
| if (name.startsWith("forkedTest") || name.endsWith("ForkedTest")) { | ||||||||||||||||||
| setExcludes(emptyList()) | ||||||||||||||||||
| setIncludes(listOf("**/*ForkedTest*")) | ||||||||||||||||||
| forkEvery = 1 | ||||||||||||||||||
| // Limit the number of concurrent forked tests | ||||||||||||||||||
| usesService(forkedTestLimit) | ||||||||||||||||||
| onlyIf { !skipForkedTestsProvider.isPresent } | ||||||||||||||||||
| } else { | ||||||||||||||||||
| exclude("**/*ForkedTest*") | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Set test timeout for 20 minutes. Default job timeout is 1h (configured on CI level). | ||||||||||||||||||
| timeout.set(Duration.of(20, ChronoUnit.MINUTES)) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| tasks.register("allTests") { | ||||||||||||||||||
| dependsOn(providers.provider { | ||||||||||||||||||
| tasks.withType<Test>().filter { testTask -> | ||||||||||||||||||
| !testTask.name.contains("latest", ignoreCase = true) && testTask.name != "traceAgentTest" | ||||||||||||||||||
| } | ||||||||||||||||||
| }) | ||||||||||||||||||
|
Comment on lines
+105
to
+109
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: I think the provider is not needed there, but
Suggested change
|
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| tasks.register("allLatestDepTests") { | ||||||||||||||||||
| dependsOn(providers.provider { | ||||||||||||||||||
|
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: same as above. |
||||||||||||||||||
| tasks.withType<Test>().filter { testTask -> | ||||||||||||||||||
| testTask.name.contains("latest", ignoreCase = true) | ||||||||||||||||||
| } | ||||||||||||||||||
| }) | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| tasks.named("check") { | ||||||||||||||||||
| dependsOn(tasks.withType<Test>()) | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
104
to
126
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. Would be cool to comment each 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. Like this: afa8ed9? |
||||||||||||||||||
|
|
||||||||||||||||||
| tasks.withType(Test::class.java).configureEach { | ||||||||||||||||||
|
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. nitpick:
Suggested change
|
||||||||||||||||||
| // Flaky tests management for JUnit 5 | ||||||||||||||||||
| if (testFramework is JUnitPlatformOptions) { | ||||||||||||||||||
| val junitPlatform = testFramework as JUnitPlatformOptions | ||||||||||||||||||
| if (skipFlakyTestsProvider.isPresent) { | ||||||||||||||||||
| junitPlatform.excludeTags("flaky") | ||||||||||||||||||
| } else if (runFlakyTestsProvider.isPresent) { | ||||||||||||||||||
| junitPlatform.includeTags("flaky") | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // Flaky tests management for Spock | ||||||||||||||||||
| if (skipFlakyTestsProvider.isPresent) { | ||||||||||||||||||
| jvmArgs("-Drun.flaky.tests=false") | ||||||||||||||||||
| } else if (runFlakyTestsProvider.isPresent) { | ||||||||||||||||||
| jvmArgs("-Drun.flaky.tests=true") | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // tasks.withType(Test).configureEach { | ||||||||||||||||||
| // // https://docs.gradle.com/develocity/flaky-test-detection/ | ||||||||||||||||||
| // // https://docs.gradle.com/develocity/gradle-plugin/current/#test_retry | ||||||||||||||||||
| // develocity.testRetry { | ||||||||||||||||||
| // if (providers.environmentVariable("CI").isPresent()) { | ||||||||||||||||||
| // maxRetries = 3 | ||||||||||||||||||
| // } | ||||||||||||||||||
| // } | ||||||||||||||||||
| // } | ||||||||||||||||||
|
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. I'm not sure how to convert this part to Kotlin... Any help or recommendations would be appreciated! Since CI is passing, I wonder if we can remove this section, but perhaps these CI runs did not need to leverage test retries whereas other runs will need to. 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. This should be kept, the issue is likely to be that you need to acces the develocity types. Usually this involves referring to the plugin without applying it. But since this is develocity, I'm not sure. |
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,8 +40,6 @@ protobuf { | |
| addTestSuiteForDir('latestDepTest', 'test') | ||
| addTestSuiteExtendingForDir('latestDepForkedTest', 'latestDepTest', 'test') | ||
|
|
||
| apply from: "$rootDir/gradle/configure_tests.gradle" | ||
|
|
||
| tasks.named("latestDepTest", Test) { | ||
|
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. These files should already be applying this plugin via 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. Thanks for the cleanup ! |
||
| finalizedBy 'latestDepForkedTest' | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.