Pass test time estimate as stabilize smart flank without cached results

RainNapper · RainNapper · commit de2336db4c39 · 2020-02-03T18:37:49.000-08:00
diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
@@ -64,6 +64,7 @@ class AndroidArgs(
     private val flank = flankYml.flank
     override val maxTestShards = cli?.maxTestShards ?: flank.maxTestShards
     override val shardTime = cli?.shardTime ?: flank.shardTime
+    override val testTimeEstimate = cli?.testTimeEstimate ?: flank.testTimeEstimate
     override val repeatTests = cli?.repeatTests ?: flank.repeatTests
     override val smartFlankGcsPath = cli?.smartFlankGcsPath ?: flank.smartFlankGcsPath
     override val smartFlankDisableUpload = cli?.smartFlankDisableUpload ?: flank.smartFlankDisableUpload
@@ -139,6 +140,7 @@ ${devicesToString(devices)}
     flank:
       max-test-shards: $maxTestShards
       shard-time: $shardTime
+      test-time-estimate: $testTimeEstimate
       num-test-runs: $repeatTests
       smart-flank-gcs-path: $smartFlankGcsPath
       smart-flank-disable-upload: $smartFlankDisableUpload
diff --git a/test_runner/src/main/kotlin/ftl/args/IArgs.kt b/test_runner/src/main/kotlin/ftl/args/IArgs.kt
@@ -20,6 +20,7 @@ interface IArgs {
     // FlankYml
     val maxTestShards: Int
     val shardTime: Int
+    val testTimeEstimate: Int
     val repeatTests: Int
     val smartFlankGcsPath: String
     val smartFlankDisableUpload: Boolean
diff --git a/test_runner/src/main/kotlin/ftl/args/IosArgs.kt b/test_runner/src/main/kotlin/ftl/args/IosArgs.kt
@@ -52,6 +52,7 @@ class IosArgs(
     private val flank = flankYml.flank
     override val maxTestShards = cli?.maxTestShards ?: flank.maxTestShards
     override val shardTime = cli?.shardTime ?: flank.shardTime
+    override val testTimeEstimate = cli?.testTimeEstimate ?: flank.testTimeEstimate
     override val repeatTests = cli?.repeatTests ?: flank.repeatTests
     override val smartFlankGcsPath = cli?.smartFlankGcsPath ?: flank.smartFlankGcsPath
     override val smartFlankDisableUpload = cli?.smartFlankDisableUpload ?: flank.smartFlankDisableUpload
@@ -127,6 +128,7 @@ ${devicesToString(devices)}
     flank:
       max-test-shards: $maxTestShards
       shard-time: $shardTime
+      test-time-estimate: $testTimeEstimate
       num-test-runs: $repeatTests
       smart-flank-gcs-path: $smartFlankGcsPath
       smart-flank-disable-upload: $smartFlankDisableUpload
diff --git a/test_runner/src/main/kotlin/ftl/args/yml/FlankYml.kt b/test_runner/src/main/kotlin/ftl/args/yml/FlankYml.kt
@@ -13,6 +13,9 @@ class FlankYmlParams(
     @field:JsonProperty("shard-time")
     val shardTime: Int = -1,
 
+    @field:JsonProperty("test-time-estimate")
+    val testTimeEstimate: Int = 10,
+
     @field:JsonProperty("num-test-runs")
     val repeatTests: Int = 1,
 
@@ -38,8 +41,9 @@ class FlankYmlParams(
 ) {
     companion object : IYmlKeys {
         override val keys = listOf(
-            "max-test-shards", "shard-time", "num-test-runs", "smart-flank-gcs-path", "smart-flank-disable-upload",
-            "disable-sharding", "test-targets-always-run", "files-to-download", "project", "local-result-dir"
+            "max-test-shards", "shard-time", "test-time-estimate", "num-test-runs", "smart-flank-gcs-path",
+            "smart-flank-disable-upload", "disable-sharding", "test-targets-always-run", "files-to-download",
+            "project", "local-result-dir"
         )
 
         const val defaultLocalResultDir = "results"
diff --git a/test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidRunCommand.kt b/test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidRunCommand.kt
@@ -276,6 +276,12 @@ class AndroidRunCommand : Runnable {
     )
     var shardTime: Int? = null
 
+    @Option(
+        names = ["--test-time-estimate"],
+        description = ["The approximate time in seconds each test is assumed to run for. This is used to help group tests in shards with Smart Flank"]
+    )
+    var testTimeEstimate: Int? = null
+
     @Option(
         names = ["--num-test-runs"],
         description = ["The amount of times to run the test executions."]
diff --git a/test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosRunCommand.kt b/test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosRunCommand.kt
@@ -198,6 +198,12 @@ class IosRunCommand : Runnable {
     )
     var shardTime: Int? = null
 
+    @Option(
+        names = ["--test-time-estimate"],
+        description = ["The approximately time in seconds each test is assumed to run for. This is used to groups tests in shards with Smart Flank"]
+    )
+    var testTimeEstimate: Int? = null
+
     @Option(
         names = ["--num-test-runs"],
         description = ["The amount of times to run the test executions."]
diff --git a/test_runner/src/main/kotlin/ftl/shard/Shard.kt b/test_runner/src/main/kotlin/ftl/shard/Shard.kt
@@ -66,7 +66,7 @@ object Shard {
         if (args.shardTime < -1 || args.shardTime == 0) fatalError("Invalid shard time ${args.shardTime}")
 
         val junitMap = createJunitMap(oldTestResult, args)
-        val testsTotalTime = testsToRun.sumByDouble { junitMap[it] ?: 10.0 }
+        val testsTotalTime = testsToRun.sumByDouble { junitMap[it] ?: args.testTimeEstimate.toDouble() }
 
         val shardsByTime = ceil(testsTotalTime / args.shardTime).toInt()
 
@@ -102,9 +102,11 @@ object Shard {
         var cacheMiss = 0
         val testcases = mutableListOf<TestMethod>()
 
+        // Set this arg to a local variable to avoid referencing it from args for every test.
+        val testTimeEstimate = args.testTimeEstimate.toDouble()
         testsToRun.forEach { key ->
             val previousTime = junitMap[key]
-            val time = previousTime ?: 10.0
+            val time = previousTime ?: testTimeEstimate
 
             if (previousTime == null) {
                 cacheMiss += 1
diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt
@@ -64,6 +64,7 @@ class AndroidArgsTest {
         flank:
           max-test-shards: 7
           shard-time: 60
+          test-time-estimate: 50
           num-test-runs: 8
           files-to-download:
             - /sdcard/screenshots
@@ -185,6 +186,7 @@ class AndroidArgsTest {
             // FlankYml
             assert(maxTestShards, 7)
             assert(shardTime, 60)
+            assert(testTimeEstimate, 50)
             assert(repeatTests, 8)
             assert(filesToDownload, listOf("/sdcard/screenshots", "/sdcard/screenshots2"))
             assert(
@@ -237,6 +239,7 @@ AndroidArgs
     flank:
       max-test-shards: 7
       shard-time: 60
+      test-time-estimate: 50
       num-test-runs: 8
       smart-flank-gcs-path:${' '}
       smart-flank-disable-upload: false
@@ -291,6 +294,8 @@ AndroidArgs
 
             // FlankYml
             assert(maxTestShards, 1)
+            assert(shardTime, -1)
+            assert(testTimeEstimate, 10)
             assert(repeatTests, 1)
             assert(filesToDownload, empty)
             assert(testTargetsAlwaysRun, empty)
@@ -765,6 +770,23 @@ AndroidArgs
         assertThat(AndroidArgs.load(yaml, cli).shardTime).isEqualTo(3)
     }
 
+    @Test
+    fun `cli testTimeEstimate`() {
+        val cli = AndroidRunCommand()
+        CommandLine(cli).parseArgs("--test-time-estimate=30")
+
+        val yaml = """
+        gcloud:
+          app: $appApk
+          test: $testApk
+
+        flank:
+          test-time-estimate: 60
+      """
+        assertThat(AndroidArgs.load(yaml).shardTime).isEqualTo(60)
+        assertThat(AndroidArgs.load(yaml, cli).shardTime).isEqualTo(30)
+    }
+
     @Test
     fun `cli disableSharding`() {
         val cli = AndroidRunCommand()
diff --git a/test_runner/src/test/kotlin/ftl/args/FlankYmlTest.kt b/test_runner/src/test/kotlin/ftl/args/FlankYmlTest.kt
@@ -25,10 +25,11 @@ class FlankYmlTest {
     fun testValidArgs() {
         FlankYml()
         FlankYml(FlankYmlParams(maxTestShards = -1))
-        val yml = FlankYml(FlankYmlParams(maxTestShards = 1, repeatTests = 1, shardTime = 58))
+        val yml = FlankYml(FlankYmlParams(maxTestShards = 1, repeatTests = 1, shardTime = 58, testTimeEstimate = 60))
         assertThat(yml.flank.repeatTests).isEqualTo(1)
         assertThat(yml.flank.maxTestShards).isEqualTo(1)
         assertThat(yml.flank.shardTime).isEqualTo(58)
+        assertThat(yml.flank.testTimeEstimate).isEqualTo(60)
         assertThat(yml.flank.testTargetsAlwaysRun).isEqualTo(emptyList<String>())
         assertThat(FlankYml.map).isNotEmpty()
     }
diff --git a/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt
@@ -58,6 +58,7 @@ class IosArgsTest {
         flank:
           max-test-shards: 7
           shard-time: 60
+          test-time-estimate: 50
           num-test-runs: 8
           files-to-download:
             - /sdcard/screenshots
@@ -142,6 +143,7 @@ flank:
             // FlankYml
             assert(maxTestShards, 7)
             assert(shardTime, 60)
+            assert(testTimeEstimate, 50)
             assert(repeatTests, 8)
             assert(testTargetsAlwaysRun, listOf("a/testGrantPermissions", "a/testGrantPermissions2"))
 
@@ -184,6 +186,7 @@ IosArgs
     flank:
       max-test-shards: 7
       shard-time: 60
+      test-time-estimate: 50
       num-test-runs: 8
       smart-flank-gcs-path:${' '}
       smart-flank-disable-upload: false
@@ -230,6 +233,7 @@ IosArgs
             // FlankYml
             assert(maxTestShards, 1)
             assert(shardTime, -1)
+            assert(testTimeEstimate, 10)
             assert(repeatTests, 1)
             assert(testTargetsAlwaysRun, emptyList<String>())
             assert(filesToDownload, emptyList<String>())
@@ -445,6 +449,23 @@ IosArgs
         assertThat(IosArgs.load(yaml, cli).shardTime).isEqualTo(3)
     }
 
+    @Test
+    fun `cli testTimeEstimate`() {
+        val cli = IosRunCommand()
+        CommandLine(cli).parseArgs("--test-time-estimate=30")
+
+        val yaml = """
+        gcloud:
+          test: $testPath
+          xctestrun-file: $xctestrunFile
+
+        flank:
+          test-time-estimate: 60
+      """
+        assertThat(IosArgs.load(yaml).shardTime).isEqualTo(60)
+        assertThat(IosArgs.load(yaml, cli).shardTime).isEqualTo(30)
+    }
+
     @Test
     fun `cli disableSharding`() {
         val cli = IosRunCommand()
diff --git a/test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt b/test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt
@@ -85,6 +85,7 @@ class AndroidRunCommandTest {
         assertThat(cmd.resultsHistoryName).isNull()
         assertThat(cmd.maxTestShards).isNull()
         assertThat(cmd.shardTime).isNull()
+        assertThat(cmd.testTimeEstimate).isNull()
         assertThat(cmd.repeatTests).isNull()
         assertThat(cmd.testTargetsAlwaysRun).isNull()
         assertThat(cmd.filesToDownload).isNull()
@@ -321,6 +322,14 @@ class AndroidRunCommandTest {
         assertThat(cmd.shardTime).isEqualTo(99)
     }
 
+    @Test
+    fun `testTimeEstimate parse`() {
+        val cmd = AndroidRunCommand()
+        CommandLine(cmd).parseArgs("--test-time-estimate=99")
+
+        assertThat(cmd.testTimeEstimate).isEqualTo(99)
+    }
+
     @Test
     fun `disableSharding parse`() {
         val cmd = AndroidRunCommand()
diff --git a/test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosRunCommandTest.kt b/test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosRunCommandTest.kt
@@ -75,6 +75,7 @@ class IosRunCommandTest {
         assertThat(cmd.resultsHistoryName).isNull()
         assertThat(cmd.maxTestShards).isNull()
         assertThat(cmd.shardTime).isNull()
+        assertThat(cmd.testTimeEstimate).isNull()
         assertThat(cmd.repeatTests).isNull()
         assertThat(cmd.testTargetsAlwaysRun).isNull()
         assertThat(cmd.testTargets).isNull()
@@ -247,6 +248,14 @@ class IosRunCommandTest {
         assertThat(cmd.shardTime).isEqualTo(99)
     }
 
+    @Test
+    fun `testTimeEstimate parse`() {
+        val cmd = IosRunCommand()
+        CommandLine(cmd).parseArgs("--test-time-estimate=99")
+
+        assertThat(cmd.testTimeEstimate).isEqualTo(99)
+    }
+
     @Test
     fun `disableShard parse`() {
         val cmd = IosRunCommand()
diff --git a/test_runner/src/test/kotlin/ftl/shard/ShardTest.kt b/test_runner/src/test/kotlin/ftl/shard/ShardTest.kt
@@ -41,10 +41,11 @@ class ShardTest {
         return JUnitTestResult(mutableListOf(suite1, suite2))
     }
 
-    private fun mockArgs(maxTestShards: Int, shardTime: Int = 0): IArgs {
+    private fun mockArgs(maxTestShards: Int, shardTime: Int = 0, testTimeEstimate: Int = 10): IArgs {
         val mockArgs = mock(IosArgs::class.java)
         `when`(mockArgs.maxTestShards).thenReturn(maxTestShards)
         `when`(mockArgs.shardTime).thenReturn(shardTime)
+        `when`(mockArgs.testTimeEstimate).thenReturn(testTimeEstimate)
         return mockArgs
     }
 
@@ -153,6 +154,25 @@ class ShardTest {
         assertThat(result).isEqualTo(3)
     }
 
+    @Test
+    fun `createShardsByShardTime uncachedTestResultsUseProvidedEstimate`() {
+        val testsToRun = listOf("h/h", "i/i", "j/j")
+        val suite = sample()
+        val result = Shard.shardCountByTime(testsToRun, suite, mockArgs(maxTestShards = -1, shardTime = 60, testTimeEstimate = 60))
+
+        assertThat(result).isEqualTo(3)
+    }
+
+    @Test
+    fun `createShardsByShardTime mixedCachedAndUncachedTestResultsUseProvidedEstimate`() {
+        // Test "a/a" is hard-coded to have 1.0 second run time in test suite results.
+        val testsToRun = listOf("a/a", "i/i", "j/j")
+        val suite = sample()
+        val result = Shard.shardCountByTime(testsToRun, suite, mockArgs(maxTestShards = -1, shardTime = 60, testTimeEstimate = 59))
+
+        assertThat(result).isEqualTo(2)
+    }
+
     @Test(expected = RuntimeException::class)
     fun `createShardsByShardCount throws on forcedShardCount = 0`() {
         Shard.createShardsByShardCount(
