Don't re-use bad test matrices (#60)

* Don't re-use bad test matrices

This CL changes the TestMatrix re-use logic to skip test matrices
which failed for an infra or bad configuration error.

Both of these error types have the possibility of being intermediate
so it is best not to re-use them and instead try running them again.

Test: TestMatrixStoreTest

* rename test

Author: yigit

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

@@ -84,8 +84,16 @@ internal class TestMatrixStore(
         }
 
         getExistingTestMatrix(testRunId)?.let {
-            logger.info("found existing test matrix: ${it.testMatrixId}")
-            return it
+            logger.info("found existing test matrix: ${it.testMatrixId} with state: ${it.state}")
+            val state = it.state
+            // these states are ordered so anything above ERROR is not worth re-using
+            if (state != null && state >= TestMatrix.State.ERROR) {
+                logger.warn {
+                    "Skipping cache for ${it.testMatrixId} because its state is $state"
+                }
+            } else {
+                return it
+            }
         }
         logger.trace {
             "No test run history for $testRunId, creating a new one."

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

@@ -28,6 +28,7 @@ import dev.androidx.ci.generated.ftl.ClientInfoDetail
 import dev.androidx.ci.generated.ftl.EnvironmentMatrix
 import dev.androidx.ci.generated.ftl.FileReference
 import dev.androidx.ci.generated.ftl.ShardingOption
+import dev.androidx.ci.generated.ftl.TestMatrix
 import dev.androidx.ci.generated.ftl.UniformSharding
 import dev.androidx.ci.testRunner.vo.ApkInfo
 import dev.androidx.ci.testRunner.vo.DeviceSetup
@@ -220,6 +221,45 @@ internal class TestMatrixStoreTest {
         assertThat(reUploadedAfterDeletion.testMatrixId).isNotEqualTo(testMatrix.testMatrixId)
     }
 
+    @Test
+    fun dontReuseTestMatricesWithInfraFailures() = runBlocking<Unit> {
+        val appApk = createFakeApk("app.pak")
+        val testApk = createFakeApk("test.apk")
+        val extraApk = createFakeApk("extra.apk")
+        val testMatrix = store.getOrCreateTestMatrix(
+            appApk = appApk,
+            testApk = testApk,
+            environmentMatrix = envMatrix1,
+            clientInfo = null,
+            deviceSetup = null,
+            sharding = null
+        )
+        val reload = store.getOrCreateTestMatrix(
+            appApk = appApk,
+            testApk = testApk,
+            environmentMatrix = envMatrix1,
+            clientInfo = null,
+            deviceSetup = null,
+            sharding = null
+        )
+        assertThat(testMatrix.testMatrixId).isEqualTo(reload.testMatrixId)
+        // set it to failed.
+        firebaseTestLabApi.setTestMatrix(
+            testMatrix.copy(
+                state = TestMatrix.State.ERROR
+            )
+        )
+        val reloadAfterError = store.getOrCreateTestMatrix(
+            appApk = appApk,
+            testApk = testApk,
+            environmentMatrix = envMatrix1,
+            clientInfo = null,
+            deviceSetup = null,
+            sharding = null
+        )
+        assertThat(reloadAfterError.testMatrixId).isNotEqualTo(testMatrix.testMatrixId)
+    }
+
     @Test
     fun pullScreenshots() = runBlocking<Unit> {
         val appApk = createFakeApk("app.pak")