Add support for num-uniform-shards option

release_notes.md

@@ -1,4 +1,5 @@
 ## next (unreleased)
+- [#714](https://github.com/Flank/flank/pull/714) Add support for num-uniform-shards option. ([jan-gogo](https://github.com/jan-gogo))
 - [#712](https://github.com/Flank/flank/pull/712) Add keep file path for ios. ([pawelpasterz](https://github.com/pawelpasterz))
 - [#711](https://github.com/Flank/flank/pull/711) Remove hardcoded height. ([pawelpasterz](https://github.com/pawelpasterz))
 - [#708](https://github.com/Flank/flank/pull/708) Add ignore failed tests option to Flank. ([pawelpasterz](https://github.com/pawelpasterz))

test_runner/flank.yml

@@ -89,6 +89,16 @@ gcloud:
   ## Enabled by default, use --no-performance-metrics to disable.
   # performance-metrics: true
 
+  ## Specifies the number of shards into which you want to evenly distribute test cases.
+  ## The shards are run in parallel on separate devices. For example,
+  ## if your test execution contains 20 test cases and you specify four shards, each shard executes five test cases.
+  ## The number of shards should be less than the total number of test cases.
+  ## The number of shards specified must be >= 1 and <= 50.
+  ## This option cannot be used along max-test-shards and is not compatible with smart sharding.
+  ## If you want to take benefits of smart sharding use max-test-shards instead.
+  ## default: null
+  # num-uniform-shards: 50
+
   ## The fully-qualified Java class name of the instrumentation test runner
   ## (default: the last name extracted from the APK manifest).
   # test-runner-class: com.foo.TestRunner

test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt

@@ -63,6 +63,7 @@ class AndroidArgs(
         devicePath to filePath.processFilePath("from otherFiles")
     }.toMap()
     val performanceMetrics = cli?.performanceMetrics ?: cli?.noPerformanceMetrics?.not() ?: androidGcloud.performanceMetrics
+    val numUniformShards = cli?.numUniformShards ?: androidGcloud.numUniformShards
     val testRunnerClass = cli?.testRunnerClass ?: androidGcloud.testRunnerClass
     val testTargets = cli?.testTargets ?: androidGcloud.testTargets.filterNotNull()
     val devices = cli?.device ?: androidGcloud.device
@@ -99,6 +100,10 @@ class AndroidArgs(
 
         devices.forEach { device -> assertDeviceSupported(device) }
 
+        if (numUniformShards != null && maxTestShards > 1) throw FlankFatalError(
+            "Option num-uniform-shards cannot be specified along with max-test-shards. Use only one of them"
+        )
+
         assertCommonProps(this)
     }
 
@@ -134,6 +139,7 @@ AndroidArgs
       directories-to-pull:${listToString(directoriesToPull)}
       other-files:${mapToString(otherFiles)}
       performance-metrics: $performanceMetrics
+      num-uniform-shards: $numUniformShards
       test-runner-class: $testRunnerClass
       test-targets:${listToString(testTargets)}
       device:${devicesToString(devices)}

test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt

@@ -14,13 +14,15 @@ object AndroidTestShard {
     // computed properties not specified in yaml
     fun getTestShardChunks(args: AndroidArgs, testApk: String): ShardChunks {
         // Download test APK if necessary so it can be used to validate test methods
-        var testLocalApk = testApk
-        if (testApk.startsWith(FtlConstants.GCS_PREFIX)) {
-            testLocalApk = GcStorage.download(testApk)
-        }
+        val testLocalApk = if (testApk.startsWith(FtlConstants.GCS_PREFIX))
+            GcStorage.download(testApk) else
+            testApk
 
         val filteredTests = getTestMethods(args, testLocalApk)
-        return ArgsHelper.calculateShards(filteredTests, args)
+
+        return if (args.numUniformShards == null)
+            ArgsHelper.calculateShards(filteredTests, args) else
+            listOf(filteredTests.map(FlankTestMethod::testName))
     }
 
     private fun getTestMethods(args: AndroidArgs, testLocalApk: String): List<FlankTestMethod> {

test_runner/src/main/kotlin/ftl/args/yml/AndroidGcloudYml.kt

@@ -38,6 +38,9 @@ class AndroidGcloudYmlParams(
     @field:JsonProperty("performance-metrics")
     val performanceMetrics: Boolean = FlankDefaults.DISABLE_PERFORMANCE_METRICS,
 
+    @field:JsonProperty("num-uniform-shards")
+    val numUniformShards: Int? = null,
+
     @field:JsonProperty("test-runner-class")
     val testRunnerClass: String? = null,
 
@@ -57,6 +60,7 @@ class AndroidGcloudYmlParams(
             "directories-to-pull",
             "other-files",
             "performance-metrics",
+            "num-uniform-shards",
             "test-runner-class",
             "test-targets",
             "device"

test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidRunCommand.kt

@@ -175,6 +175,20 @@ class AndroidRunCommand : CommonRunCommand(), Runnable {
     )
     var noPerformanceMetrics: Boolean? = null
 
+    @Option(
+        names = ["--num-uniform-shards"],
+        description = [
+            "Specifies the number of shards into which you want to evenly distribute test cases.",
+            "The shards are run in parallel on separate devices. For example,",
+            "if your test execution contains 20 test cases and you specify four shards, each shard executes five test cases.",
+            "The number of shards should be less than the total number of test cases.",
+            "The number of shards specified must be >= 1 and <= 50.",
+            "This option cannot be used along max-test-shards and is not compatible with smart sharding.",
+            "If you want to take benefits of smart sharding use max-test-shards."
+        ]
+    )
+    var numUniformShards: Int? = null
+
     @Option(
         names = ["--test-runner-class"],
         description = ["The fully-qualified Java class name of the instrumentation test runner (default: the last name extracted " +

test_runner/src/main/kotlin/ftl/gc/GcAndroidTestMatrix.kt

@@ -12,14 +12,11 @@ import com.google.api.services.testing.model.EnvironmentVariable
 import com.google.api.services.testing.model.FileReference
 import com.google.api.services.testing.model.GoogleAuto
 import com.google.api.services.testing.model.GoogleCloudStorage
-import com.google.api.services.testing.model.ManualSharding
 import com.google.api.services.testing.model.RegularFile
 import com.google.api.services.testing.model.ResultStorage
-import com.google.api.services.testing.model.ShardingOption
 import com.google.api.services.testing.model.TestMatrix
 import com.google.api.services.testing.model.TestSetup
 import com.google.api.services.testing.model.TestSpecification
-import com.google.api.services.testing.model.TestTargetsForShard
 import com.google.api.services.testing.model.ToolResultsHistory
 import ftl.args.AndroidArgs
 import ftl.args.ShardChunks
@@ -52,17 +49,10 @@ object GcAndroidTestMatrix {
 
         val matrixGcsPath = join(args.resultsBucket, runGcsPath)
 
-        // ShardingOption().setUniformSharding(UniformSharding().setNumShards())
-        val testTargetsForShard: List<TestTargetsForShard> = testShards.map {
-            TestTargetsForShard().setTestTargets(it)
-        }
-        val manualSharding = ManualSharding().setTestTargetsForShard(testTargetsForShard)
-        val shardingOption = ShardingOption().setManualSharding(manualSharding)
-
         val androidInstrumentation = AndroidInstrumentationTest()
             .setAppApk(FileReference().setGcsPath(appApkGcsPath))
             .setTestApk(FileReference().setGcsPath(testApkGcsPath))
-            .setShardingOption(shardingOption)
+            .setupTestTargets(args, testShards)
 
         if (args.testRunnerClass != null) {
             androidInstrumentation.testRunnerClass = args.testRunnerClass

test_runner/src/main/kotlin/ftl/gc/Utils.kt

@@ -1,9 +1,40 @@
 package ftl.gc
 
+import com.google.api.services.testing.model.AndroidInstrumentationTest
 import com.google.api.services.testing.model.ClientInfoDetail
+import com.google.api.services.testing.model.ManualSharding
+import com.google.api.services.testing.model.ShardingOption
+import com.google.api.services.testing.model.TestTargetsForShard
+import com.google.api.services.testing.model.UniformSharding
+import ftl.args.AndroidArgs
+import ftl.args.ShardChunks
 
 internal fun Map<String, String>.toClientInfoDetailList() = map { (key, value) ->
     ClientInfoDetail()
         .setKey(key)
         .setValue(value)
 }
+
+internal fun AndroidInstrumentationTest.setupTestTargets(args: AndroidArgs, testShards: ShardChunks) = apply {
+    if (args.disableSharding) {
+        testTargets = testShards.flatten()
+    } else {
+        shardingOption = ShardingOption().apply {
+            if (args.numUniformShards != null) {
+                testTargets = testShards.flatten()
+                val numUniformShards =
+                    if (testTargets.size > args.numUniformShards) {
+                        args.numUniformShards
+                    } else {
+                        println("WARNING: num-uniform-shards (${args.numUniformShards}) is higher than number of test cases (${testTargets.size}) from ${testApk.gcsPath}")
+                        testTargets.size
+                    }
+                uniformSharding = UniformSharding().setNumShards(numUniformShards)
+            } else {
+                manualSharding = ManualSharding().setTestTargetsForShard(testShards.map {
+                    TestTargetsForShard().setTestTargets(it)
+                })
+            }
+        }
+    }
+}

test_runner/src/main/kotlin/ftl/reports/api/CreateTestExecutionData.kt

@@ -4,6 +4,7 @@ import com.google.api.services.testing.model.TestExecution
 import com.google.api.services.testing.model.ToolResultsStep
 import com.google.api.services.toolresults.model.ListTestCasesResponse
 import com.google.api.services.toolresults.model.Step
+import com.google.api.services.toolresults.model.TestCase
 import com.google.api.services.toolresults.model.Timestamp
 import ftl.gc.GcToolResults
 import ftl.reports.api.data.TestExecutionData
@@ -29,11 +30,13 @@ private suspend fun TestExecution.createTestExecutionData(): TestExecutionData {
         step: Step
     ) = getAsync(toolResultsStep)
 
+    val testCases = response.testCases ?: emptyList()
+
     return TestExecutionData(
         testExecution = this@createTestExecutionData,
-        testCases = response.testCases,
+        testCases = testCases,
         step = step,
-        timestamp = response.getStartTimestamp()
+        timestamp = testCases.getStartTimestamp()
     )
 }
 
@@ -45,6 +48,6 @@ private suspend fun getAsync(toolResultsStep: ToolResultsStep) = coroutineScope
 
 // Unfortunately is not possible to obtain from api exact the same timestamp as is in autogenerated test_result_1.xml from google cloud.
 // This one is a little bit lower but close as possible. The difference is around ~3 seconds.
-private fun ListTestCasesResponse.getStartTimestamp(): Timestamp = testCases.minBy { testCase ->
+private fun List<TestCase>.getStartTimestamp(): Timestamp = minBy { testCase ->
     testCase.startTime.asUnixTimestamp()
-}!!.startTime
+}?.startTime ?: Timestamp()

test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt

@@ -34,7 +34,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
     val runCount = args.repeatTests
     val history = GcToolResults.createToolResultsHistory(args)
     val resolvedTestApks = args.getResolvedTestApks()
-    val otherGcsFiles = args.otherFiles.uploadOtherFiles(args.resultsBucket, runGcsPath)
+    val otherGcsFiles = args.uploadOtherFiles(runGcsPath)
 
     val allTestShardChunks: ShardChunks = resolvedTestApks.map { apks: ResolvedTestApks ->
         // Ensure we only shard tests that are part of the test apk. Use the resolved test apk path to make sure
@@ -128,11 +128,10 @@ private suspend fun uploadTestApks(
     )
 }
 
-private suspend fun Map<String, String>.uploadOtherFiles(
-    gcsBucket: String,
+private suspend fun AndroidArgs.uploadOtherFiles(
     runGcsPath: String
 ): Map<String, String> = coroutineScope {
-    map { (devicePath: String, filePath: String) ->
-        async(Dispatchers.IO) { devicePath to GcStorage.upload(filePath, gcsBucket, runGcsPath) }
+    otherFiles.map { (devicePath: String, filePath: String) ->
+        async(Dispatchers.IO) { devicePath to GcStorage.upload(filePath, resultsBucket, runGcsPath) }
     }.awaitAll().toMap()
 }

test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt

@@ -67,6 +67,7 @@ class AndroidArgsTest {
             /sdcard/dir1/file1.txt: $appApk
             /sdcard/dir2/file2.jpg: $testApk
           performance-metrics: false
+          num-uniform-shards: null
           test-runner-class: com.foo.TestRunner
           test-targets:
           - class com.example.app.ExampleUiTest#testPasses
@@ -268,6 +269,7 @@ AndroidArgs
         /sdcard/dir1/file1.txt: $appApkAbsolutePath
         /sdcard/dir2/file2.jpg: $testApkAbsolutePath
       performance-metrics: false
+      num-uniform-shards: null
       test-runner-class: com.foo.TestRunner
       test-targets:
         - class com.example.app.ExampleUiTest#testPasses
@@ -334,6 +336,7 @@ AndroidArgs
       directories-to-pull:
       other-files:
       performance-metrics: false
+      num-uniform-shards: null
       test-runner-class: null
       test-targets:
       device:
@@ -627,6 +630,36 @@ AndroidArgs
         assertThat(androidArgs.performanceMetrics).isFalse()
     }
 
+    @Test
+    fun `cli numUniformShards`() {
+        val expected = 50
+        val cli = AndroidRunCommand()
+        CommandLine(cli).parseArgs("--num-uniform-shards=$expected")
+
+        val yaml = """
+        gcloud:
+          app: $appApk
+          test: $testApk
+      """
+        assertThat(AndroidArgs.load(yaml).numUniformShards).isNull()
+
+        val androidArgs = AndroidArgs.load(yaml, cli)
+        assertThat(androidArgs.numUniformShards).isEqualTo(expected)
+    }
+
+    @Test(expected = FlankFatalError::class)
+    fun `should throw if numUniformShards is specified along with maxTestShards`() {
+        val yaml = """
+        gcloud:
+          app: $appApk
+          test: $testApk
+          num-uniform-shards: 50
+        flank:
+          max-test-shards: 50
+      """
+        AndroidArgs.load(yaml)
+    }
+
     @Test
     fun `cli testRunnerClass`() {
         val cli = AndroidRunCommand()

test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt

@@ -68,6 +68,7 @@ class AndroidRunCommandTest {
         assertThat(cmd.noUseOrchestrator).isNull()
         assertThat(cmd.performanceMetrics).isNull()
         assertThat(cmd.noPerformanceMetrics).isNull()
+        assertThat(cmd.numUniformShards).isNull()
         assertThat(cmd.testRunnerClass).isNull()
         assertThat(cmd.environmentVariables).isNull()
         assertThat(cmd.directoriesToPull).isNull()
@@ -180,6 +181,15 @@ class AndroidRunCommandTest {
         assertThat(cmd.noPerformanceMetrics).isTrue()
     }
 
+    @Test
+    fun `numUniformShards parse`() {
+        val expected = 50
+        val cmd = AndroidRunCommand()
+        CommandLine(cmd).parseArgs("--num-uniform-shards=$expected")
+
+        assertThat(cmd.numUniformShards).isEqualTo(expected)
+    }
+
     @Test
     fun `testRunnerClass parse`() {
         val cmd = AndroidRunCommand()

test_runner/src/test/kotlin/ftl/gc/UtilsKtTest.kt

@@ -0,0 +1,69 @@
+package ftl.gc
+
+import com.google.api.services.testing.model.AndroidInstrumentationTest
+import com.google.api.services.testing.model.FileReference
+import ftl.args.AndroidArgs
+import ftl.args.ShardChunks
+import io.mockk.every
+import io.mockk.mockk
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNotNull
+import org.junit.Test
+
+class UtilsKtTest {
+
+    @Test
+    fun `setupTestTargets should setup testTargets`() {
+        // given
+        val args = mockk<AndroidArgs> {
+            every { disableSharding } returns true
+        }
+        val testShards: ShardChunks = emptyList()
+
+        // when
+        val actual = AndroidInstrumentationTest().setupTestTargets(args, testShards)
+            .testTargets
+
+        // then
+        assertNotNull(actual)
+    }
+
+    @Test
+    fun `setupTestTargets should setup uniformSharding`() {
+        // given
+        val expectedTestTargets = emptyList<String>()
+        val args = mockk<AndroidArgs> {
+            every { disableSharding } returns false
+            every { numUniformShards } returns 50
+        }
+        val testShards: ShardChunks = listOf(expectedTestTargets)
+
+        // when
+        val actual = AndroidInstrumentationTest()
+            .setTestApk(FileReference().setGcsPath("testApk"))
+            .setupTestTargets(args, testShards)
+
+        // then
+        assertEquals(0, actual.shardingOption.uniformSharding.numShards)
+        assertEquals(expectedTestTargets, actual.testTargets)
+    }
+
+    @Test
+    fun `setupTestTargets should setup manualSharding`() {
+        // given
+        val shardChunks: ShardChunks = listOf(emptyList(), emptyList())
+        val args = mockk<AndroidArgs> {
+            every { disableSharding } returns false
+            every { numUniformShards } returns null
+        }
+
+        // when
+        val actual = AndroidInstrumentationTest().setupTestTargets(args, shardChunks)
+            .shardingOption
+            .manualSharding
+            .testTargetsForShard
+
+        // then
+        assertEquals(shardChunks.size, actual.size)
+    }
+}