From af798ebe3b1675a636647277a6e2ec1ce95085de Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Fri, 9 Jul 2021 11:25:48 +0200 Subject: [PATCH 1/2] Boiler plate code for multiple sharding --- docs/index.md | 10 +++++++--- test_runner/flank.yml | 6 +++++- .../src/main/kotlin/ftl/args/ValidateAndroidArgs.kt | 11 ++++++++++- .../run/platform/android/CreateAndroidTestContext.kt | 12 +++++++++--- 4 files changed, 31 insertions(+), 8 deletions(-) diff --git a/docs/index.md b/docs/index.md index d69341f3a4..5fe59bafb8 100644 --- a/docs/index.md +++ b/docs/index.md @@ -588,12 +588,16 @@ gcloud: # - class com.package2.for.shard2.Class ### parameterized-tests - ## Specifies how to handle tests which contain Parameterization. - ## 3 options are available + ## Specifies how to handle tests which contain the parameterization annotation. + ## 4 options are available ## default: treat Parameterized tests as normal and shard accordingly ## ignore-all: Parameterized tests are ignored and not sharded ## shard-into-single: Parameterized tests are collected and put into a single shard - ## Note: if left blank default is used. Default usage may result in significant increase/difference of shard times observed + ## shard-into-multiple: Parameterized tests are collected and sharded into different shards based upon matching names. (Experimental) + ## Note: If left blank default is used. Default usage may result in significant increase/difference of shard times observed + ## Note: If shard-into-single is used, a single additional shard is created that will run the Parameterized tests separately. + ## Note: If shard-into-multiple is used, each parameterized test will be matched by its corresponding name and sharded into a separate shard. + ## This may dramatically increase the amount of expected shards depending upon how many parameterized tests are discovered. # parameterized-tests: default flank: diff --git a/test_runner/flank.yml b/test_runner/flank.yml index 805cfef8a7..671677546c 100644 --- a/test_runner/flank.yml +++ b/test_runner/flank.yml @@ -225,7 +225,11 @@ gcloud: ## default: treat Parameterized tests as normal and shard accordingly ## ignore-all: Parameterized tests are ignored and not sharded ## shard-into-single: Parameterized tests are collected and put into a single shard - ## Note: if left blank default is used. Default usage may result in significant increase/difference of shard times observed + ## shard-into-multiple: Parameterized tests are collected and sharded into different shards based upon matching names. (Experimental) + ## Note: If left blank default is used. Default usage may result in significant increase/difference of shard times observed + ## Note: If shard-into-single is used, a single additional shard is created that will run the Parameterized tests separately. + ## Note: If shard-into-multiple is used, each parameterized test will be matched by its corresponding name and sharded into a separate shard. + ## This may dramatically increase the amount of expected shards depending upon how many parameterized tests are discovered. # parameterized-tests: default flank: diff --git a/test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt index 4fe81f33f1..bc4afa2390 100644 --- a/test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt +++ b/test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt @@ -276,7 +276,16 @@ private fun AndroidArgs.assertParameterizedTests() { if (parameterizedTests.isNullOrEmpty() || parameterizedTests !in listOf( "ignore-all", "default", - "shard-into-single" + "shard-into-single", + "shard-into-multiple" ) ) throw FlankConfigurationError("Parameterized test flag must be one of the following: `default`, `ignore-all`, `shard-into-single`, leaving it blank will result in `default` sharding.") + else checkParameterizedTestFeaturesWarning() +} + +private fun AndroidArgs.checkParameterizedTestFeaturesWarning() { + when (parameterizedTests) { + "shard-into-single" -> logLn("WARNING: All parameterized tests will be collected and sharded separately into a single shard. This may result in a long shard execution times if many parameterized tests are found.") + "shard-into-multiple" -> logLn("WARNING: All parameterized tests will be collected and sharded into different shards. This will result in additional shards created. max-shards is not adhered to for this selection.") + } } diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt index 58653cae81..67350a5b69 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt @@ -96,15 +96,20 @@ private fun InstrumentationTestContext.calculateShardsNormally( private fun InstrumentationTestContext.calculateShards( args: AndroidArgs, testFilter: TestFilter = TestFilters.fromTestTargets(args.testTargets, args.testTargetsForShard) -): InstrumentationTestContext = - if (args.parameterizedTests.shouldShardIntoSingle()) { +): InstrumentationTestContext = when { + args.parameterizedTests.shouldShardIntoSingle() -> { var flankTestMethods = getFlankTestMethods(testFilter, args.parameterizedTests) val parameterizedTests = flankTestMethods.filter { it.isParameterizedClass } flankTestMethods = flankTestMethods.filterNot { it.isParameterizedClass } val shards = calculateParameterizedShards(flankTestMethods, args) val parameterizedShard = calculateParameterizedShards(parameterizedTests, args, 1) shards.copy(shards = shards.shards + parameterizedShard.shards) - } else calculateShardsNormally(args, testFilter) + } + args.parameterizedTests.shouldShardMultiple() -> { + calculateShardsNormally(args, testFilter) + } + else -> calculateShardsNormally(args, testFilter) +} private fun InstrumentationTestContext.calculateParameterizedShards( filteredTests: List, @@ -122,6 +127,7 @@ private fun InstrumentationTestContext.calculateParameterizedShards( } private fun String.shouldShardIntoSingle() = (this == "shard-into-single") +private fun String.shouldShardMultiple() = (this == "shard-into-multiple") private fun InstrumentationTestContext.calculateDummyShards( args: AndroidArgs, From 0b808fc84417eda8585d1e05f79f11945fd93353 Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Fri, 9 Jul 2021 11:57:51 +0200 Subject: [PATCH 2/2] multiple sharding --- .../ftl/config/android/AndroidGcloudConfig.kt | 8 +++++- .../android/CreateAndroidTestContext.kt | 28 ++++++++----------- .../test/kotlin/ftl/args/AndroidArgsTest.kt | 14 ++++++++++ 3 files changed, 32 insertions(+), 18 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/config/android/AndroidGcloudConfig.kt b/test_runner/src/main/kotlin/ftl/config/android/AndroidGcloudConfig.kt index b91557e2cf..db6dfa43d3 100644 --- a/test_runner/src/main/kotlin/ftl/config/android/AndroidGcloudConfig.kt +++ b/test_runner/src/main/kotlin/ftl/config/android/AndroidGcloudConfig.kt @@ -250,7 +250,13 @@ data class AndroidGcloudConfig @JsonIgnore constructor( @set:CommandLine.Option( names = ["--parameterized-tests"], - description = ["Specifies how to handle tests which contain the parameterization annotation. Possible values: `default`, `ignore-all`, `shard-into-single`, leaving it blank will result in `default` sharding"] + description = [ + "Specifies how to handle tests which contain the parameterization annotation. Possible values: `default`, `ignore-all`, `shard-into-single`, `shard-into-multiple`.\n" + + "leaving it blank will result in `default` sharding.\n" + + "Note: Making use of shard-into-single` or `shard-into-multiple will result in additional shards being created even if a max number of shards has been specified.\n" + + "Note: If shard-into-single is used, a single additional shard is created that will run the Parameterized tests separately.\n" + + "Note: If shard-into-multiple is used, each parameterized test will be matched by its corresponding name and sharded into a separate shard. This may dramatically increase the amount of expected shards depending upon how many parameterized tests are discovered." + ] ) @set:JsonProperty("parameterized-tests") var parameterizedTests: String? by data diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt index 67350a5b69..d2f40ba70a 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt @@ -96,20 +96,15 @@ private fun InstrumentationTestContext.calculateShardsNormally( private fun InstrumentationTestContext.calculateShards( args: AndroidArgs, testFilter: TestFilter = TestFilters.fromTestTargets(args.testTargets, args.testTargetsForShard) -): InstrumentationTestContext = when { - args.parameterizedTests.shouldShardIntoSingle() -> { - var flankTestMethods = getFlankTestMethods(testFilter, args.parameterizedTests) - val parameterizedTests = flankTestMethods.filter { it.isParameterizedClass } - flankTestMethods = flankTestMethods.filterNot { it.isParameterizedClass } - val shards = calculateParameterizedShards(flankTestMethods, args) - val parameterizedShard = calculateParameterizedShards(parameterizedTests, args, 1) - shards.copy(shards = shards.shards + parameterizedShard.shards) - } - args.parameterizedTests.shouldShardMultiple() -> { - calculateShardsNormally(args, testFilter) - } - else -> calculateShardsNormally(args, testFilter) -} +): InstrumentationTestContext = if (args.parameterizedTests.shouldShardSmartly()) { + var flankTestMethods = getFlankTestMethods(testFilter, args.parameterizedTests) + val parameterizedTests = flankTestMethods.filter { it.isParameterizedClass } + flankTestMethods = flankTestMethods.filterNot { it.isParameterizedClass } + val shards = calculateParameterizedShards(flankTestMethods, args) + val shardCount = if (args.parameterizedTests.isSingleParameterizedShard()) 1 else parameterizedTests.size + val parameterizedShard = calculateParameterizedShards(parameterizedTests, args, shardCount) + shards.copy(shards = shards.shards + parameterizedShard.shards) +} else calculateShardsNormally(args, testFilter) private fun InstrumentationTestContext.calculateParameterizedShards( filteredTests: List, @@ -126,9 +121,6 @@ private fun InstrumentationTestContext.calculateParameterizedShards( ) } -private fun String.shouldShardIntoSingle() = (this == "shard-into-single") -private fun String.shouldShardMultiple() = (this == "shard-into-multiple") - private fun InstrumentationTestContext.calculateDummyShards( args: AndroidArgs, testFilter: TestFilter = TestFilters.fromTestTargets(args.testTargets, args.testTargetsForShard) @@ -183,6 +175,8 @@ private val ignoredAnnotations = listOf( "android.support.test.filters.Suppress" ) +private fun String.shouldShardSmartly() = (this == "shard-into-single" || this == "shard-into-multiple") +private fun String.isSingleParameterizedShard() = (this == "shard-into-single") private fun String.shouldIgnore(): Boolean = (this == "ignore-all") @VisibleForTesting diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt index 5edeeee2cc..1bda31327e 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt @@ -2832,6 +2832,20 @@ AndroidArgs val chunks = runBlocking { parsedYml.runAndroidTests() }.shardChunks assertTrue(chunks.size == 1) } + + @Test + fun `should shard tests into multiple new shards with shard-into-multiple`() { + val yaml = """ + gcloud: + app: $appApk + test: $testExtremeParameterizedOtherApk + parameterized-tests: shard-into-multiple + """.trimIndent() + + val parsedYml = AndroidArgs.load(yaml).validate() + val chunks = runBlocking { parsedYml.runAndroidTests() }.shardChunks + assertTrue(chunks.size == 5) + } } private fun AndroidArgs.Companion.load(