feat: added max-test-shards and client-details to additional-app-test…

…-apks (#1947)

Fixes #

**This PR is a rework of #1898. A PR was merged recently which refactors some of the code I was using and introduced merge conflicts in #1898. This PR implements the same functionality with the refactored code.**

This PR adds the ability to specify `max-test-shards` and `client-details` for tests under additional-app-test-apks like so:
```
additional-app-test-apks:
    - test: lib1-test.apk
      max-test-shards: 3
    - test: lib2-test.apk
      max-test-shards: 1
      client-details:
        modulename: lib2
        key2: val2
```

These configurations really improves support for multi-module projects, usually the thing I want to change the most is number of shards used to test a module. It also lets me differentiate between modules by setting key/values under `client-details`.





## Test Plan
> How do we know the code works?
Added unit tests
.

## Checklist

- [ ] Documented
- [x] Unit tested
- [ ] Integration tests updated

test_runner/src/main/kotlin/ftl/api/TestAndroidMatrix.kt

@@ -47,7 +47,8 @@ object TestMatrixAndroid {
             val numUniformShards: Int?,
             val keepTestTargetsEmpty: Boolean,
             val environmentVariables: Map<String, String> = emptyMap(),
-            val testTargetsForShard: ShardChunks
+            val testTargetsForShard: ShardChunks,
+            val clientDetails: Map<String, String> = emptyMap(),
         ) : Type()
 
         data class Robo(

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

@@ -32,11 +32,19 @@ fun createAndroidArgs(
     roboScript = gcloud.roboScript?.normalizeFilePath(),
 
     // flank
-    additionalAppTestApks = flank.additionalAppTestApks?.map { (app, test, env) ->
+    additionalAppTestApks = flank.additionalAppTestApks?.map {
+        // if additional-pair did not provide certain values, set as top level ones
+        val mergedClientDetails = mutableMapOf<String, String>().apply {
+            // merge additionalAppTestApk's client-details with top-level client-details
+            putAll(commonArgs.clientDetails ?: emptyMap())
+            putAll(it.clientDetails)
+        }
         AppTestPair(
-            app = app?.normalizeFilePath(),
-            test = test.normalizeFilePath(),
-            environmentVariables = env
+            app = it.app?.normalizeFilePath(),
+            test = it.test.normalizeFilePath(),
+            environmentVariables = it.environmentVariables,
+            maxTestShards = it.maxTestShards ?: commonArgs.maxTestShards,
+            clientDetails = mergedClientDetails
         )
     } ?: emptyList(),
     useLegacyJUnitResult = flank::useLegacyJUnitResult.require(),

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

@@ -8,5 +8,9 @@ data class AppTestPair(
     val app: String?,
     val test: String,
     @JsonProperty("environment-variables")
-    val environmentVariables: Map<String, String> = emptyMap()
+    val environmentVariables: Map<String, String> = emptyMap(),
+    @JsonProperty("max-test-shards")
+    val maxTestShards: Int? = null,
+    @JsonProperty("client-details")
+    var clientDetails: Map<String, String> = emptyMap()
 )

test_runner/src/main/kotlin/ftl/client/google/run/android/GcAndroidTestMatrix.kt

@@ -58,8 +58,10 @@ private fun createAndroidTestMatrix(
     runIndex: Int
 ): Testing.Projects.TestMatrices.Create {
 
+    val clientDetails = config.clientInfo(testMatrixType)
+
     val testMatrix = TestMatrix()
-        .setClientInfo(config.clientInfo)
+        .setClientInfo(clientDetails)
         .setTestSpecification(getTestSpecification(testMatrixType, config))
         .setResultStorage(config.resultsStorage(contextIndex, runIndex))
         .setEnvironmentMatrix(config.environmentMatrix)
@@ -71,11 +73,18 @@ private fun createAndroidTestMatrix(
     }.getOrElse { e -> throw FlankGeneralError(e) }
 }
 
-// https://github.com/bootstraponline/studio-google-cloud-testing/blob/203ed2890c27a8078cd1b8f7ae12cf77527f426b/firebase-testing/src/com/google/gct/testing/launcher/CloudTestsLauncher.java#L120
-private val TestMatrixAndroid.Config.clientInfo
-    get() = ClientInfo()
-        .setName("Flank")
-        .setClientInfoDetails(clientDetails?.toClientInfoDetailList())
+fun TestMatrixAndroid.Config.clientInfo(matrix: TestMatrixAndroid.Type): ClientInfo {
+    return if (matrix is TestMatrixAndroid.Type.Instrumentation && matrix.clientDetails.isNotEmpty()) {
+        ClientInfo()
+            .setName("Flank")
+            .setClientInfoDetails(matrix.clientDetails.toClientInfoDetailList())
+    } else {
+        // https://github.com/bootstraponline/studio-google-cloud-testing/blob/203ed2890c27a8078cd1b8f7ae12cf77527f426b/firebase-testing/src/com/google/gct/testing/launcher/CloudTestsLauncher.java#L120
+        ClientInfo()
+            .setName("Flank")
+            .setClientInfoDetails(clientDetails?.toClientInfoDetailList())
+    }
+}
 
 private val TestMatrixAndroid.Config.environmentMatrix
     get() = EnvironmentMatrix()

test_runner/src/main/kotlin/ftl/config/android/AndroidFlankConfig.kt

@@ -35,7 +35,8 @@ data class AndroidFlankConfig @JsonIgnore constructor(
             additionalAppTestApks?.add(
                 AppTestPair(
                     app = appApk,
-                    test = testApk
+                    test = testApk,
+                    maxTestShards = map["max-test-shards"]?.toInt()
                 )
             )
         }

test_runner/src/main/kotlin/ftl/mock/MockServer.kt

@@ -1,5 +1,7 @@
 package ftl.mock
 
+import com.fasterxml.jackson.databind.ObjectMapper
+import com.fasterxml.jackson.module.kotlin.readValue
 import com.google.api.services.toolresults.model.AppStartTime
 import com.google.api.services.toolresults.model.CPUInfo
 import com.google.api.services.toolresults.model.Duration
@@ -27,6 +29,7 @@ import com.google.gson.GsonBuilder
 import com.google.gson.LongSerializationPolicy
 import com.google.testing.model.AndroidDevice
 import com.google.testing.model.AndroidDeviceCatalog
+import com.google.testing.model.ClientInfo
 import com.google.testing.model.Environment
 import com.google.testing.model.GoogleCloudStorage
 import com.google.testing.model.IosDeviceCatalog
@@ -37,6 +40,8 @@ import com.google.testing.model.TestExecution
 import com.google.testing.model.TestMatrix
 import com.google.testing.model.ToolResultsExecution
 import com.google.testing.model.ToolResultsStep
+import ftl.analytics.objectToMap
+import ftl.client.google.run.toClientInfoDetailList
 import ftl.config.FtlConstants
 import ftl.config.FtlConstants.JSON_FACTORY
 import ftl.log.LogbackLogger
@@ -52,17 +57,22 @@ import io.ktor.application.install
 import io.ktor.features.ContentNegotiation
 import io.ktor.gson.GsonConverter
 import io.ktor.http.ContentType
+import io.ktor.request.receive
 import io.ktor.request.uri
 import io.ktor.response.respond
 import io.ktor.routing.get
 import io.ktor.routing.post
 import io.ktor.routing.routing
 import io.ktor.server.engine.embeddedServer
 import io.ktor.server.netty.Netty
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.withContext
 import java.net.BindException
+import java.nio.charset.StandardCharsets.UTF_8
 import java.nio.file.Files
 import java.nio.file.Paths
 import java.util.concurrent.atomic.AtomicInteger
+import java.util.zip.GZIPInputStream
 
 object MockServer {
 
@@ -180,6 +190,12 @@ object MockServer {
                     val projectId = call.parameters["project"]
 
                     val matrixId = matrixIdCounter.incrementAndGet().toString()
+                    val requestBody = withContext(Dispatchers.IO) {
+                        GZIPInputStream(call.receive<ByteArray>().inputStream()).bufferedReader(UTF_8).use {
+                            ObjectMapper().readValue<Map<String, Any>>(it.readText())
+                        }
+                    }
+                    val clientInfo = parseClientDetails(requestBody)
 
                     val resultStorage = ResultStorage().apply {
                         toolResultsExecution = ToolResultsExecution()
@@ -207,6 +223,7 @@ object MockServer {
                         .setEnvironment(environment)
 
                     val matrix = TestMatrix()
+                        .setClientInfo(clientInfo)
                         .setProjectId(projectId)
                         .setTestMatrixId("matrix-$matrixId")
                         .setState("FINISHED")
@@ -366,6 +383,25 @@ object MockServer {
         }
     }
 
+    private fun parseClientDetails(requestBody: Map<String, Any>): ClientInfo {
+        val clientName = requestBody["clientInfo"]?.objectToMap()?.get("name") as String
+        val allClientDetails = mutableMapOf<String, String>()
+        requestBody["clientInfo"]
+            ?.objectToMap()
+            ?.get("clientInfoDetails")
+            .run { this as? List<*> }
+            ?.let { list ->
+                list.filterIsInstance<Map<String, String>>()
+                    .filter { it.containsKey("key") && it.containsKey("value") }
+                    .map { it.getValue("key") to it.getValue("value") }
+                    .forEach { (key, value) -> allClientDetails[key] = value }
+            }
+
+        return ClientInfo()
+            .setName(clientName)
+            .setClientInfoDetails(allClientDetails.toClientInfoDetailList())
+    }
+
     fun start() {
         if (isStarted) return
         val loggingEnabled = LogbackLogger.Root.isEnabled

test_runner/src/main/kotlin/ftl/run/model/AndroidTestContext.kt

@@ -13,7 +13,9 @@ data class InstrumentationTestContext(
     val shards: List<Chunk> = emptyList(),
     val ignoredTestCases: IgnoredTestCases = emptyList(),
     val environmentVariables: Map<String, String> = emptyMap(),
-    val testTargetsForShard: ShardChunks = emptyList()
+    val testTargetsForShard: ShardChunks = emptyList(),
+    val maxTestShards: Int? = null,
+    val clientDetails: Map<String, String> = emptyMap(),
 ) : AndroidTestContext()
 
 data class RoboTestContext(

test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt

@@ -39,17 +39,21 @@ private suspend fun List<AndroidTestContext>.setupShards(
 ): List<AndroidTestContext> = coroutineScope {
     map { testContext ->
         async {
+            val newArgs = args.prepareArgsForSharding(testContext)
             when {
                 testContext !is InstrumentationTestContext -> testContext
-                args.useCustomSharding -> testContext.userShards(args.customSharding)
-                args.useTestTargetsForShard ->
-                    testContext.downloadApks()
-                        .calculateDummyShards(args, testFilter)
-                else -> testContext.downloadApks().calculateShards(args, testFilter)
+                newArgs.useCustomSharding -> testContext.userShards(newArgs.customSharding)
+                newArgs.useTestTargetsForShard -> testContext.downloadApks().calculateDummyShards(newArgs, testFilter)
+                else -> testContext.downloadApks().calculateShards(newArgs, testFilter)
             }
         }
     }.awaitAll().dropEmptyInstrumentationTest()
 }
+private fun AndroidArgs.prepareArgsForSharding(context: AndroidTestContext): AndroidArgs {
+    return if (context is InstrumentationTestContext && context.maxTestShards != null) {
+        copy(commonArgs = commonArgs.copy(maxTestShards = context.maxTestShards))
+    } else this
+}
 
 private fun InstrumentationTestContext.userShards(customShardingMap: Map<String, AndroidTestShards>) = customShardingMap
     .values

test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestMatrixType.kt

@@ -39,7 +39,8 @@ internal fun AndroidArgs.createInstrumentationConfig(
     testShards = testApk.shards.testCases,
     keepTestTargetsEmpty = disableSharding && testTargets.isEmpty(),
     environmentVariables = testApk.environmentVariables,
-    testTargetsForShard = testTargetsForShard
+    testTargetsForShard = testTargetsForShard,
+    clientDetails = testApk.clientDetails
 )
 
 internal fun AndroidArgs.createRoboConfig(

test_runner/src/main/kotlin/ftl/run/platform/android/ResolveApks.kt

@@ -40,6 +40,8 @@ private fun AndroidArgs.additionalApksContexts() = additionalAppTestApks.map {
         app = appApk.asFileReference(),
         test = it.test.asFileReference(),
         environmentVariables = it.environmentVariables,
-        testTargetsForShard = testTargetsForShard
+        testTargetsForShard = testTargetsForShard,
+        maxTestShards = it.maxTestShards,
+        clientDetails = it.clientDetails,
     )
 }.toTypedArray()

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

@@ -132,6 +132,41 @@ class AndroidArgsFileTest {
             assertEquals(52, get("matrix-1")!!.shards["shard-2"]!!.size)
         }
     }
+    @Test
+    fun `calculateShards additionalAppTestApks with override`() {
+        val test1 = "src/test/kotlin/ftl/fixtures/tmp/apk/app-debug-androidTest_1.apk"
+        val test155 = "src/test/kotlin/ftl/fixtures/tmp/apk/app-debug-androidTest_155.apk"
+        val config = createAndroidArgs(
+            defaultAndroidConfig().apply {
+                platform.apply {
+                    gcloud.apply {
+                        app = appApkLocal
+                        test = getString(test1)
+                    }
+                    flank.apply {
+                        additionalAppTestApks = mutableListOf(
+                            AppTestPair(
+                                app = appApkLocal,
+                                test = getString(test155),
+                                maxTestShards = 4
+                            )
+                        )
+                    }
+                }
+                common.flank.maxTestShards = 3
+            }
+        )
+        with(runBlocking { config.getAndroidMatrixShards() }) {
+            assertEquals(1, get("matrix-0")!!.shards.size)
+            assertEquals(4, get("matrix-1")!!.shards.size)
+            assertEquals(1, get("matrix-0")!!.shards["shard-0"]!!.size)
+            // 155/4 = ~39
+            assertEquals(38, get("matrix-1")!!.shards["shard-0"]!!.size)
+            assertEquals(39, get("matrix-1")!!.shards["shard-1"]!!.size)
+            assertEquals(39, get("matrix-1")!!.shards["shard-2"]!!.size)
+            assertEquals(39, get("matrix-1")!!.shards["shard-3"]!!.size)
+        }
+    }
 
     @Test
     fun `calculateShards 0`() = runBlocking {