887 Update sharding limit (#915)

Flank · Aug 3, 2020 · 052cd87 · 052cd87
1 parent 16b1fed
commit 052cd87
Show file tree

Hide file tree

Showing 26 changed files with 241 additions and 74 deletions.
diff --git a/release_notes.md b/release_notes.md
@@ -13,6 +13,7 @@
 - [#913](https://github.com/Flank/flank/pull/913) Add Gradle Enterprise API example. ([pawelpasterz](https://github.com/pawelpasterz))
 - [#916](https://github.com/Flank/flank/pull/916) Test artifacts monorepo. ([jan-gogo](https://github.com/jan-gogo))
 - [#910](https://github.com/Flank/flank/pull/910) Migrate Bitrise release workflow into GitHub actions.  ([piotradamczyk5](https://github.com/piotradamczyk5))
+- [#915](https://github.com/Flank/flank/pull/915) Update virtual devices sharding limit. ([adamfilipow92](https://github.com/adamfilipow92))
 -
 -
 

diff --git a/test_runner/src/main/kotlin/ftl/android/AndroidCatalog.kt b/test_runner/src/main/kotlin/ftl/android/AndroidCatalog.kt
@@ -51,8 +51,9 @@ object AndroidCatalog {
         return SupportedDeviceConfig
     }
 
-    fun isVirtualDevice(device: AndroidDevice?, projectId: String): Boolean {
-        val modelId = device?.androidModelId ?: return false
+    fun isVirtualDevice(device: AndroidDevice?, projectId: String): Boolean = device?.androidModelId?.let { isVirtualDevice(it, projectId) } ?: false
+
+    fun isVirtualDevice(modelId: String, projectId: String): Boolean {
         val form = deviceCatalog(projectId).models.find { it.id == modelId }?.form ?: "PHYSICAL"
         return form == "VIRTUAL"
     }

diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
@@ -10,7 +10,7 @@ import com.google.cloud.storage.BucketInfo
 import com.google.cloud.storage.Storage
 import com.google.cloud.storage.StorageClass
 import com.google.cloud.storage.StorageOptions
-import ftl.args.IArgs.Companion.AVAILABLE_SHARD_COUNT_RANGE
+import ftl.args.IArgs.Companion.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE
 import ftl.args.yml.YamlObjectMapper
 import ftl.config.FtlConstants
 import ftl.config.FtlConstants.GCS_PREFIX
@@ -53,8 +53,8 @@ object ArgsHelper {
                     " See https://github.com/GoogleCloudPlatform/google-cloud-java#specifying-a-project-id"
         )
 
-        if (args.maxTestShards !in AVAILABLE_SHARD_COUNT_RANGE && args.maxTestShards != -1)
-            throw FlankFatalError("max-test-shards must be >= ${AVAILABLE_SHARD_COUNT_RANGE.first} and <= ${AVAILABLE_SHARD_COUNT_RANGE.last}. But current is ${args.maxTestShards}")
+        if (args.maxTestShards !in AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE && args.maxTestShards != -1)
+            throw FlankFatalError("max-test-shards must be >= ${AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} and <= ${AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}. But current is ${args.maxTestShards}")
 
         if (args.shardTime <= 0 && args.shardTime != -1) throw FlankFatalError("shard-time must be >= 1 or -1")
         if (args.repeatTests < 1) throw FlankFatalError("num-test-runs must be >= 1")

diff --git a/test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt
@@ -9,10 +9,9 @@ fun createAndroidArgs(
     config: AndroidConfig? = null,
     gcloud: AndroidGcloudConfig = config!!.platform.gcloud,
     flank: AndroidFlankConfig = config!!.platform.flank,
-    commonArgs: CommonArgs = config!!.common.createCommonArgs(config.data)
+    commonArgs: CommonArgs = config!!.prepareAndroidCommonConfig()
 ) = AndroidArgs(
     commonArgs = commonArgs,
-
     // gcloud
     appApk = gcloud.app?.processFilePath("from app"),
     testApk = gcloud.test?.processFilePath("from test"),

diff --git a/test_runner/src/main/kotlin/ftl/args/CreateCommonArgs.kt b/test_runner/src/main/kotlin/ftl/args/CreateCommonArgs.kt
@@ -26,7 +26,7 @@ fun CommonConfig.createCommonArgs(
     clientDetails = gcloud.clientDetails,
 
     // flank
-    maxTestShards = convertToShardCount(flank.maxTestShards!!),
+    maxTestShards = flank.maxTestShards!!,
     shardTime = flank.shardTime!!,
     repeatTests = flank.repeatTests!!,
     smartFlankGcsPath = flank.smartFlankGcsPath!!,
@@ -59,8 +59,3 @@ private val CommonConfig.defaultOutputStyle
 private val CommonConfig.hasMultipleExecutions
     get() = gcloud.flakyTestAttempts!! > 0 ||
             (!flank.disableSharding!! && flank.maxTestShards!! > 0)
-
-private fun convertToShardCount(inputValue: Int): Int =
-    if (inputValue != -1)
-        inputValue else
-        IArgs.AVAILABLE_SHARD_COUNT_RANGE.last
diff --git a/test_runner/src/main/kotlin/ftl/args/CreateIosArgs.kt b/test_runner/src/main/kotlin/ftl/args/CreateIosArgs.kt
@@ -17,9 +17,14 @@ private fun createIosArgs(
     flank: IosFlankConfig,
     commonArgs: CommonArgs
 ) = IosArgs(
-    commonArgs = commonArgs,
+    commonArgs = commonArgs.copy(maxTestShards = convertToShardCount(commonArgs.maxTestShards)),
     xctestrunZip = gcloud.test!!.processFilePath("from test"),
     xctestrunFile = gcloud.xctestrunFile!!.processFilePath("from xctestrun-file"),
     xcodeVersion = gcloud.xcodeVersion,
     testTargets = flank.testTargets!!.filterNotNull()
 )
+
+private fun convertToShardCount(inputValue: Int) =
+    if (inputValue != -1)
+        inputValue else
+        IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last
diff --git a/test_runner/src/main/kotlin/ftl/args/IArgs.kt b/test_runner/src/main/kotlin/ftl/args/IArgs.kt
@@ -53,11 +53,19 @@ interface IArgs {
 
     val disableResultsUpload: Boolean get() = false
 
+    val inPhysicalRange: Boolean
+        get() = maxTestShards in AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE
+
+    val inVirtualRange: Boolean
+        get() = maxTestShards in AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE
+
     fun useLocalResultDir() = localResultDir != defaultLocalResultsDir
 
     companion object {
         // num_shards must be >= 1, and <= 50
-        val AVAILABLE_SHARD_COUNT_RANGE = 1..50
+        val AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE = 1..50
+
+        val AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE = 1..250
     }
 
     interface ICompanion {

diff --git a/test_runner/src/main/kotlin/ftl/args/PrepareAndroidCommonConfig.kt b/test_runner/src/main/kotlin/ftl/args/PrepareAndroidCommonConfig.kt
@@ -0,0 +1,33 @@
+package ftl.args
+
+import ftl.android.AndroidCatalog
+import ftl.config.AndroidConfig
+import ftl.config.Device
+import ftl.config.containsPhysicalDevices
+import ftl.config.containsVirtualDevices
+
+fun AndroidConfig.prepareAndroidCommonConfig() = common.createCommonArgs(data).let { commonArgs ->
+    commonArgs.devices.resolveDeviceType(commonArgs.project).updateMaxTestShards(commonArgs)
+}
+
+private fun List<Device>.resolveDeviceType(projectId: String) = map { device ->
+    device.copy(isVirtual = AndroidCatalog.isVirtualDevice(device.model, projectId))
+}
+
+private fun List<Device>.updateMaxTestShards(commonArgs: CommonArgs) =
+    commonArgs.copy(devices = this, maxTestShards = this.calculateMaxTestShards(commonArgs.maxTestShards))
+
+private fun List<Device>.calculateMaxTestShards(maxTestShards: Int) =
+    if (maxTestShards == -1) getMaxShardsByDevice()
+    else scaleMaxShardsByDevice(maxTestShards)
+
+private fun List<Device>.getMaxShardsByDevice() =
+    if (containsPhysicalDevices()) IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last
+    else IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.last
+
+private fun List<Device>.scaleMaxShardsByDevice(maxTestShards: Int) =
+    if (containsPhysicalDevices() && containsVirtualDevices()) maxTestShards.scaleToPhysicalShardsCount()
+    else maxTestShards
+
+private fun Int.scaleToPhysicalShardsCount() = if (this !in IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE)
+    IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last else this
diff --git a/test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt
@@ -5,6 +5,8 @@ import ftl.android.IncompatibleModelVersion
 import ftl.android.SupportedDeviceConfig
 import ftl.android.UnsupportedModelId
 import ftl.android.UnsupportedVersionId
+import ftl.config.containsPhysicalDevices
+import ftl.config.containsVirtualDevices
 import ftl.util.FlankFatalError
 import java.io.File
 
@@ -15,6 +17,7 @@ fun AndroidArgs.validate() {
     assertTestTypes()
     assertRoboTest()
     assertDirectoriesToPull()
+    assertMaxTestShardsByDeviceType()
 }
 
 private fun AndroidArgs.assertAdditionalAppTestApks() {
@@ -68,8 +71,30 @@ private fun AndroidArgs.assertDirectoriesToPull() {
         ?.also {
             throw FlankFatalError(
                 "Invalid value for [directories-to-pull]: Invalid path $it.\n" +
-                "Path must be absolute paths under /sdcard or /data/local/tmp (for example, --directories-to-pull /sdcard/tempDir1,/data/local/tmp/tempDir2).\n" +
-                "Path names are restricted to the characters [a-zA-Z0-9_-./+]. "
+                        "Path must be absolute paths under /sdcard or /data/local/tmp (for example, --directories-to-pull /sdcard/tempDir1,/data/local/tmp/tempDir2).\n" +
+                        "Path names are restricted to the characters [a-zA-Z0-9_-./+]. "
             )
         }
 }
+
+private fun AndroidArgs.assertMaxTestShardsByDeviceType() =
+    when {
+        devices.containsPhysicalDevices() && devices.containsVirtualDevices() -> assertDevicesShards()
+        devices.containsPhysicalDevices() && !devices.containsVirtualDevices() && !inPhysicalRange -> throwMaxTestShardsLimitExceeded()
+        else -> assertVirtualDevicesShards()
+    }
+
+private fun AndroidArgs.assertDevicesShards() {
+    if (inVirtualRange && !inPhysicalRange) println("Physical devices configured, but max-test-shards limit set to $maxTestShards, for physical devices range is ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} to ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}, you additionally have configured virtual devices. In this case, the physical limit will be decreased to: ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}")
+    else if (!inVirtualRange && !inPhysicalRange) throwMaxTestShardsLimitExceeded()
+}
+
+private fun AndroidArgs.assertVirtualDevicesShards() {
+    if (!inVirtualRange) throwMaxTestShardsLimitExceeded()
+}
+
+private fun AndroidArgs.throwMaxTestShardsLimitExceeded(): Nothing {
+    throw FlankFatalError(
+        "max-test-shards must be >= ${IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.last} for virtual devices, for physical devices max-test-shards must be >= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}, or -1. But current is $maxTestShards"
+    )
+}
diff --git a/test_runner/src/main/kotlin/ftl/args/ValidateCommonArgs.kt b/test_runner/src/main/kotlin/ftl/args/ValidateCommonArgs.kt
@@ -5,7 +5,6 @@ import ftl.util.FlankFatalError
 
 fun CommonArgs.validate() {
     assertProjectId()
-    assertMaxTestShards()
     assertShardTime()
     assertRepeatTests()
     assertSmartFlankGcsPath()
@@ -19,15 +18,6 @@ private fun CommonArgs.assertProjectId() {
     )
 }
 
-private fun CommonArgs.assertMaxTestShards() {
-    if (
-        maxTestShards !in IArgs.AVAILABLE_SHARD_COUNT_RANGE &&
-        maxTestShards != -1
-    ) throw FlankFatalError(
-        "max-test-shards must be >= ${IArgs.AVAILABLE_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_SHARD_COUNT_RANGE.last}, or -1. But current is $maxTestShards"
-    )
-}
-
 private fun CommonArgs.assertShardTime() {
     if (shardTime <= 0 && shardTime != -1) throw FlankFatalError(
         "shard-time must be >= 1 or -1"

diff --git a/test_runner/src/main/kotlin/ftl/args/ValidateIosArgs.kt b/test_runner/src/main/kotlin/ftl/args/ValidateIosArgs.kt
@@ -6,8 +6,16 @@ import ftl.util.FlankFatalError
 fun IosArgs.validate() {
     assertXcodeSupported()
     assertDevicesSupported()
+    assertMaxTestShards()
+}
+private fun IosArgs.assertMaxTestShards() { this.maxTestShards
+    if (
+        maxTestShards !in IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE &&
+        maxTestShards != -1
+    ) throw FlankFatalError(
+        "max-test-shards must be >= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}, or -1. But current is $maxTestShards"
+    )
 }
-
 private fun IosArgs.assertXcodeSupported() = when {
     xcodeVersion == null -> Unit
     IosCatalog.supportedXcode(xcodeVersion, project) -> Unit

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
@@ -44,11 +44,9 @@ class AndroidRunCommand : CommonRunCommand(), Runnable {
         }
 
         val config = AndroidArgs.load(Paths.get(configPath), cli = this)
-
         runBlocking {
-            if (dumpShards)
-                dumpShards(args = config, obfuscatedOutput = obfuscate) else
-                newTestRun(config)
+            if (dumpShards) dumpShards(args = config, obfuscatedOutput = obfuscate)
+            else newTestRun(config)
         }
     }
 

diff --git a/test_runner/src/main/kotlin/ftl/config/Device.kt b/test_runner/src/main/kotlin/ftl/config/Device.kt
@@ -12,9 +12,9 @@ data class Device(
     val model: String,
     val version: String,
     val locale: String = defaultLocale,
-    val orientation: String = defaultOrientation
+    val orientation: String = defaultOrientation,
+    val isVirtual: Boolean = false
 ) {
-
     override fun toString(): String {
         return """
         - model: $model
@@ -39,3 +39,7 @@ fun Map<String, String>.asDevice(android: Boolean) =
             orientation = getOrDefault("orientation", version)
         )
     }
+
+fun List<Device>.containsVirtualDevices() = any { it.isVirtual }
+
+fun List<Device>.containsPhysicalDevices() = any { !it.isVirtual }
diff --git a/test_runner/src/main/kotlin/ftl/run/NewTestRun.kt b/test_runner/src/main/kotlin/ftl/run/NewTestRun.kt
@@ -15,17 +15,15 @@ import ftl.util.FlankTimeoutError
 import kotlinx.coroutines.TimeoutCancellationException
 import kotlinx.coroutines.withTimeoutOrNull
 
-suspend fun newTestRun(args: IArgs) {
-    withTimeoutOrNull(args.parsedTimeout) {
-        println(args)
-        val (matrixMap, testShardChunks, ignoredTests) = cancelTestsOnTimeout(args.project) { runTests(args) }
+suspend fun newTestRun(args: IArgs) = withTimeoutOrNull(args.parsedTimeout) {
+    println(args)
+    val (matrixMap, testShardChunks, ignoredTests) = cancelTestsOnTimeout(args.project) { runTests(args) }
 
-        if (!args.async) {
-            cancelTestsOnTimeout(args.project, matrixMap.map) { pollMatrices(matrixMap.map.keys, args).update(matrixMap) }
-            cancelTestsOnTimeout(args.project, matrixMap.map) { fetchArtifacts(matrixMap, args) }
+    if (!args.async) {
+        cancelTestsOnTimeout(args.project, matrixMap.map) { pollMatrices(matrixMap.map.keys, args).update(matrixMap) }
+        cancelTestsOnTimeout(args.project, matrixMap.map) { fetchArtifacts(matrixMap, args) }
 
-            ReportManager.generate(matrixMap, args, testShardChunks, ignoredTests)
-        }
+        ReportManager.generate(matrixMap, args, testShardChunks, ignoredTests)
     }
 }
 

diff --git a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt
@@ -29,8 +29,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
 
     // GcAndroidMatrix => GcAndroidTestMatrix
     // GcAndroidTestMatrix.execute() 3x retry => matrix id (string)
-    val androidDeviceList = GcAndroidDevice.build(args.devices)
-
+    val devices = GcAndroidDevice.build(args.devices)
     val testMatrices = mutableListOf<Deferred<TestMatrix>>()
     val allTestShardChunks = mutableListOf<List<String>>()
     val ignoredTestsShardChunks = mutableListOf<List<String>>()
@@ -52,7 +51,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
                     androidTestConfig = androidTestConfig,
                     runGcsPath = "$runGcsPath/matrix_$index/",
                     additionalApkGcsPaths = additionalApks,
-                    androidDeviceList = androidDeviceList,
+                    androidDeviceList = devices,
                     args = args,
                     otherFiles = otherGcsFiles,
                     toolResultsHistory = history

diff --git a/test_runner/src/main/kotlin/ftl/shard/ShardCount.kt b/test_runner/src/main/kotlin/ftl/shard/ShardCount.kt
@@ -1,7 +1,7 @@
 package ftl.shard
 
 import ftl.args.IArgs
-import ftl.args.IArgs.Companion.AVAILABLE_SHARD_COUNT_RANGE
+import ftl.args.IArgs.Companion.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE
 import ftl.reports.xml.model.JUnitTestResult
 import ftl.util.FlankFatalError
 import ftl.util.FlankTestMethod
@@ -41,7 +41,7 @@ private fun calculateShardCount(
     testsToRunCount: Int
 ): Int = when {
     testsTotalTime <= args.shardTime -> SINGLE_SHARD
-    args.maxTestShards == NO_LIMIT -> min(AVAILABLE_SHARD_COUNT_RANGE.last, testsToRunCount)
+    args.maxTestShards == NO_LIMIT -> min(AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last, testsToRunCount)
     else -> shardCount(testsTotalTime, args).also { count ->
         if (count <= 0) throw FlankFatalError("Invalid shard count $count")
     }

diff --git a/test_runner/src/test/kotlin/Debug.kt b/test_runner/src/test/kotlin/Debug.kt
@@ -17,8 +17,8 @@ fun main() {
     withGlobalExceptionHandling {
         CommandLine(Main()).execute(
 //            "--debug",
-            "firebase", "test", "ios",
-            "test-environment",
+            "firebase", "test", "android",
+            "run",
 //            "--dry",
 //            "--dump-shards",
 //            "--output-style=single",

diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt
@@ -77,9 +77,9 @@ class AndroidArgsFileTest {
             assert(testTargets, listOf(testName))
             assert(
                 devices, listOf(
-                    Device("NexusLowRes", "23", "en", "portrait"),
-                    Device("NexusLowRes", "23", "en", "landscape"),
-                    Device("shamu", "22", "zh_CN", "default")
+                    Device("NexusLowRes", "23", "en", "portrait", isVirtual = true),
+                    Device("NexusLowRes", "23", "en", "landscape", isVirtual = true),
+                    Device("shamu", "22", "zh_CN", "default", isVirtual = false)
                 )
             )
             assert(maxTestShards, 1)