Add APIs to decide whether client wants to re-use test matrices or not (#65)

* WIP provide APIs to skip cached test matrices

* use suspend function for cached test matrix filter

* fix ktlint

* add filter to the pair method for consistency

Author: yigit

AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/CachedTestMatrixFilter.kt

@@ -0,0 +1,10 @@
+package dev.androidx.ci.testRunner
+
+import dev.androidx.ci.generated.ftl.TestMatrix
+
+/**
+ * Filter interface that is used to decide whether a TestMatrix result can be re-used.
+ *
+ * Return `true` if the cached TestMatrix can be re-used, false otherwise
+ */
+typealias CachedTestMatrixFilter = suspend (TestMatrix) -> Boolean

AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/FirebaseTestLabController.kt

@@ -108,7 +108,8 @@ internal class FirebaseTestLabController(
         sharding: ShardingOption?,
         deviceSetup: DeviceSetup?,
         devicePicker: DevicePicker? = null,
-        pullScreenshots: Boolean = false
+        pullScreenshots: Boolean = false,
+        cachedTestMatrixFilter: CachedTestMatrixFilter,
     ): List<TestMatrix> {
         val devices = (devicePicker ?: defaultDevicePicker).pickDevices()
         logger.info {
@@ -123,7 +124,8 @@ internal class FirebaseTestLabController(
                 clientInfo = clientInfo,
                 sharding = sharding,
                 deviceSetup = deviceSetup,
-                pullScreenshots = pullScreenshots
+                pullScreenshots = pullScreenshots,
+                cachedTestMatrixFilter = cachedTestMatrixFilter
             )
         }
     }
@@ -208,7 +210,8 @@ internal class FirebaseTestLabController(
     suspend fun pairAndStartTests(
         apks: List<UploadedApk>,
         placeholderApk: UploadedApk,
-        devicePicker: DevicePicker? = null
+        devicePicker: DevicePicker? = null,
+        cachedTestMatrixFilter: CachedTestMatrixFilter,
     ): List<TestMatrix> {
         val pairs = apks.mapNotNull { uploadedApk ->
             val isTestApkWithLegacySuffix = uploadedApk.apkInfo.filePath.endsWith(TEST_APK_SUFFIX_LEGACY)
@@ -239,7 +242,8 @@ internal class FirebaseTestLabController(
                 testApk = it.second,
                 sharding = null,
                 deviceSetup = null,
-                devicePicker = devicePicker
+                devicePicker = devicePicker,
+                cachedTestMatrixFilter = cachedTestMatrixFilter,
             )
         }
     }

AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestMatrixStore.kt

@@ -67,7 +67,8 @@ internal class TestMatrixStore(
         clientInfo: ClientInfo?,
         sharding: ShardingOption?,
         deviceSetup: DeviceSetup?,
-        pullScreenshots: Boolean = false
+        pullScreenshots: Boolean = false,
+        cachedTestMatrixFilter: CachedTestMatrixFilter = { true },
     ): TestMatrix {
 
         val testRunId = TestRun.createId(
@@ -91,12 +92,16 @@ internal class TestMatrixStore(
                 logger.warn {
                     "Skipping cache for ${it.testMatrixId} because its state is $state"
                 }
+            } else if (!cachedTestMatrixFilter(it)) {
+                logger.info {
+                    "Not re-using cached matrix due to filter"
+                }
             } else {
                 return it
             }
         }
         logger.trace {
-            "No test run history for $testRunId, creating a new one."
+            "No test run history for $testRunId or cached TestMatrix is rejected, creating a new one."
         }
         val newTestMatrix = firebaseTestLabApi.createTestMatrix(
             projectId = firebaseProjectId,

AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestRunnerService.kt

@@ -78,7 +78,8 @@ interface TestRunnerService {
         sharding: ShardingOption?,
         deviceSetup: DeviceSetup?,
         devicePicker: (TestEnvironmentCatalog) -> List<AndroidDevice>,
-        pullScreenshots: Boolean = false
+        pullScreenshots: Boolean = false,
+        cachedTestMatrixFilter: CachedTestMatrixFilter = { true },
     ): ScheduleTestsResponse
 
     /**

AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestRunnerServiceImpl.kt

@@ -98,7 +98,8 @@ internal class TestRunnerServiceImpl internal constructor(
         sharding: ShardingOption?,
         deviceSetup: DeviceSetup?,
         devicePicker: (TestEnvironmentCatalog) -> List<AndroidDevice>,
-        pullScreenshots: Boolean
+        pullScreenshots: Boolean,
+        cachedTestMatrixFilter: CachedTestMatrixFilter,
     ): TestRunnerService.ScheduleTestsResponse {
         val testMatrices = testLabController.submitTests(
             appApk = appApk ?: apkStore.getPlaceholderApk(),
@@ -107,7 +108,8 @@ internal class TestRunnerServiceImpl internal constructor(
             sharding = sharding,
             deviceSetup = deviceSetup,
             devicePicker = devicePicker,
-            pullScreenshots = pullScreenshots
+            pullScreenshots = pullScreenshots,
+            cachedTestMatrixFilter = cachedTestMatrixFilter
         )
         return TestRunnerService.ScheduleTestsResponse.create(
             testMatrices

AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestScheduler.kt

@@ -51,7 +51,7 @@ internal sealed interface TestScheduler {
         private val githubApi: GithubApi,
         private val firebaseTestLabController: FirebaseTestLabController,
         private val apkStore: ApkStore,
-        private val devicePicker: DevicePicker?
+        private val devicePicker: DevicePicker?,
     ) : TestScheduler {
         override suspend fun enqueueTests(
             artifact: ArtifactsResponse.Artifact
@@ -61,7 +61,8 @@ internal sealed interface TestScheduler {
             return firebaseTestLabController.pairAndStartTests(
                 apks = apks,
                 placeholderApk = apkStore.getPlaceholderApk(),
-                devicePicker = devicePicker
+                devicePicker = devicePicker,
+                cachedTestMatrixFilter = { true }
             )
         }
 
@@ -98,13 +99,13 @@ internal sealed interface TestScheduler {
                 githubApi: GithubApi,
                 firebaseTestLabController: FirebaseTestLabController,
                 apkStore: ApkStore,
-                devicePicker: DevicePicker?
+                devicePicker: DevicePicker?,
             ): TestScheduler {
                 return PairAndRunAllApks(
                     githubApi = githubApi,
                     firebaseTestLabController = firebaseTestLabController,
                     apkStore = apkStore,
-                    devicePicker = devicePicker
+                    devicePicker = devicePicker,
                 )
             }
         }
@@ -223,7 +224,8 @@ internal sealed interface TestScheduler {
                         instrumentationArguments = instrumentationArguments
                     ),
                     devicePicker = devicePicker,
-                    pullScreenshots = false
+                    pullScreenshots = false,
+                    cachedTestMatrixFilter = { true }
                 )
             }
 
@@ -324,7 +326,7 @@ internal sealed interface TestScheduler {
             githubApi: GithubApi,
             firebaseTestLabController: FirebaseTestLabController,
             apkStore: ApkStore,
-            devicePicker: DevicePicker?
+            devicePicker: DevicePicker?,
         ): TestScheduler
     }
     companion object {

AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/FirebaseTestLabControllerTest.kt

@@ -64,7 +64,8 @@ internal class FirebaseTestLabControllerTest {
         )
         val testMatrices = firebaseTestLabApi.pairAndStartTests(
             apks = apks,
-            placeholderApk = placeholderApk
+            placeholderApk = placeholderApk,
+            cachedTestMatrixFilter = { true }
         )
         assertThat(testMatrices).hasSize(4)
         val appApkPaths = testMatrices.mapNotNull {

AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestMatrixStoreTest.kt

@@ -105,7 +105,8 @@ internal class TestMatrixStoreTest {
             environmentMatrix = envMatrix1,
             clientInfo = clientInfo,
             deviceSetup = deviceSetup,
-            sharding = sharding
+            sharding = sharding,
+            cachedTestMatrixFilter = { true },
         )
 
         assertThat(firebaseTestLabApi.getTestMatrices()).hasSize(1)
@@ -219,6 +220,17 @@ internal class TestMatrixStoreTest {
             sharding = sharding
         )
         assertThat(reUploadedAfterDeletion.testMatrixId).isNotEqualTo(testMatrix.testMatrixId)
+        val dontReuse = store.getOrCreateTestMatrix(
+            appApk = appApk,
+            testApk = testApk,
+            environmentMatrix = envMatrix1,
+            clientInfo = clientInfo,
+            deviceSetup = deviceSetup,
+            sharding = sharding,
+            cachedTestMatrixFilter = { false }
+        )
+        assertThat(dontReuse.testMatrixId)
+            .isNotIn(listOf(testMatrix.testMatrixId, reUploadedAfterDeletion.testMatrixId))
     }
 
     @Test

AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServiceImplTest.kt

@@ -140,6 +140,20 @@ class TestRunnerServiceImplTest {
         assertThat(
             sameRequest.testMatrices
         ).containsExactlyElementsIn(result.testMatrices)
+        // reject apk, should be a new one
+        val noReuse = subject.scheduleTests(
+            testApk = upload1,
+            appApk = null,
+            clientInfo = null,
+            sharding = null,
+            deviceSetup = null,
+            devicePicker = devicePicker,
+            cachedTestMatrixFilter = { false }
+        )
+        assertThat(
+            noReuse.testMatrices
+        ).containsNoneIn(result.testMatrices)
+
         // change client info, it should result in new test matrices
         val clientInfo = ClientInfo(
             name = "test",