fix: Adds check for .aab format before querying apkDetails (#2189)

* Adds check for .aab format before querying apkDetails * PR fixes * Update test
Flank · Apr 7, 2022 · 4a6e2b7 · 4a6e2b7
1 parent cf7d734
commit 4a6e2b7
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 24 deletions.
diff --git a/test_projects/android/bundle/app-debug.aab b/test_projects/android/bundle/app-debug.aab
diff --git a/test_runner/src/main/kotlin/ftl/client/google/AppDetails.kt b/test_runner/src/main/kotlin/ftl/client/google/AppDetails.kt
@@ -3,10 +3,14 @@ package ftl.client.google
 import com.google.testing.model.FileReference
 import ftl.http.executeWithRetry
 
-fun getAndroidAppDetails(gcsAppPath: String): String =
-    GcTesting.get
+fun getAndroidAppDetails(gcsAppPath: String): String {
+    // getApkDetails errors when sent non-apk files such as aab
+    if (gcsAppPath.trim().lowercase().endsWith(".apk").not()) return ""
+
+    return GcTesting.get
         .ApplicationDetailService()
         .getApkDetails(FileReference().apply { gcsPath = gcsAppPath })
         .apply { requestHeaders.set("X-Server-Timeout", 1800) } // 30 min
         .executeWithRetry()
         ?.apkDetail?.apkManifest?.packageName?.toString().orEmpty()
+}
diff --git a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt
@@ -50,6 +50,7 @@ internal suspend fun AndroidArgs.runAndroidTests(): TestResult = coroutineScope
                 ignoredTestsShardChunks += context.ignoredTestCases
                 allTestShardChunks += context.shards
             }
+
             context.reportPackageName()
         }
         .map { createTestSetup(it) }

diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt
@@ -2,6 +2,7 @@ package ftl.args
 
 import com.google.common.truth.Truth.assertThat
 import com.google.testing.model.TestSpecification
+import flank.tool.analytics.mixpanel.Mixpanel
 import ftl.args.IArgs.Companion.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE
 import ftl.args.IArgs.Companion.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE
 import ftl.args.yml.AppTestPair
@@ -31,10 +32,13 @@ import ftl.test.util.TestHelper.getPath
 import ftl.test.util.TestHelper.getThrowable
 import ftl.test.util.assertThrowsWithMessage
 import ftl.util.asFileReference
+import ftl.util.getMockedTestMatrix
+import ftl.util.mockTestMatrices
 import io.mockk.every
 import io.mockk.mockkObject
 import io.mockk.mockkStatic
 import io.mockk.unmockkAll
+import io.mockk.verify
 import kotlinx.coroutines.runBlocking
 import org.junit.After
 import org.junit.Assert.assertEquals
@@ -62,6 +66,7 @@ class AndroidArgsTest {
 
     private val empty = emptyList<String>()
     private val appApk = "../test_projects/android/apks/app-debug.apk"
+    private val appAab = "../test_projects/android/bundle/app-debug.aab"
     private val invalidApk = "../test_projects/android/apks/invalid.apk"
     private val nonExistingApk = "../test_projects/android/apks/app-debug_non_existing.apk"
     private val testApk = "../test_projects/android/apks/app-debug-androidTest.apk"
@@ -1717,6 +1722,29 @@ AndroidArgs
             }
     }
 
+    @Test
+    fun `should send no package name to mixpanel for aab format`() {
+        val yaml = """
+        gcloud:
+          app: $appAab
+          test: $testApk
+        """.trimIndent()
+
+        val parsedYml = AndroidArgs.load(yaml).validate()
+
+        mockTestMatrices(
+            getMockedTestMatrix().apply { state = "RUNNING" },
+            getMockedTestMatrix().apply { state = "RUNNING" },
+            getMockedTestMatrix()
+        )
+
+        mockkObject(Mixpanel)
+
+        runBlocking { parsedYml.runAndroidTests() }
+
+        verify { Mixpanel.add(Mixpanel.APP_ID, "") }
+    }
+
     @Test
     fun `results-dir (cloud directory) should not throw if it doesn't exist locally`() {
         val resultsDir = UUID.randomUUID().toString()

diff --git a/test_runner/src/test/kotlin/ftl/run/TestRunnerTest.kt b/test_runner/src/test/kotlin/ftl/run/TestRunnerTest.kt
@@ -2,10 +2,6 @@ package ftl.run
 
 import com.google.common.truth.Truth.assertThat
 import com.google.testing.Testing
-import com.google.testing.model.GoogleCloudStorage
-import com.google.testing.model.ResultStorage
-import com.google.testing.model.TestExecution
-import com.google.testing.model.TestMatrix
 import flank.common.isWindows
 import ftl.adapter.google.getFilePathToDownload
 import ftl.api.Artifacts.DownloadPath
@@ -15,6 +11,8 @@ import ftl.client.google.getAndroidAppDetails
 import ftl.http.executeWithRetry
 import ftl.test.util.FlankTestRunner
 import ftl.test.util.LocalGcs
+import ftl.util.getMockedTestMatrix
+import ftl.util.mockTestMatrices
 import io.mockk.every
 import io.mockk.mockkStatic
 import io.mockk.unmockkAll
@@ -155,10 +153,7 @@ class TestRunnerTest {
     @Test
     fun `flank should stop updating web link if matrix has invalid state`() {
         val localConfig = AndroidArgs.load(Paths.get("src/test/kotlin/ftl/fixtures/flank.local.yml"))
-        mockkStatic("ftl.http.ExecuteWithRetryKt")
-        every {
-            any<Testing.Projects.TestMatrices.Get>().executeWithRetry()
-        } returnsMany listOf(
+        mockTestMatrices(
             getMockedTestMatrix().apply { state = "RUNNING" },
             getMockedTestMatrix().apply { state = "RUNNING" },
             getMockedTestMatrix()
@@ -180,18 +175,4 @@ class TestRunnerTest {
         assertFalse(output.contains(matrixLink))
         verify(exactly = 3) { any<Testing.Projects.TestMatrices.Get>().executeWithRetry() }
     }
-
-    private fun getMockedTestMatrix() = TestMatrix().apply {
-        state = "INVALID"
-        testMatrixId = "matrix-12345"
-        testExecutions = listOf(
-            TestExecution().apply {
-                resultStorage = ResultStorage().apply {
-                    googleCloudStorage = GoogleCloudStorage().apply {
-                        gcsPath = "any/Path"
-                    }
-                }
-            }
-        )
-    }
 }
diff --git a/test_runner/src/test/kotlin/ftl/util/MatricesMocks.kt b/test_runner/src/test/kotlin/ftl/util/MatricesMocks.kt
@@ -0,0 +1,33 @@
+package ftl.util
+
+import com.google.testing.Testing
+import com.google.testing.model.GoogleCloudStorage
+import com.google.testing.model.ResultStorage
+import com.google.testing.model.TestExecution
+import com.google.testing.model.TestMatrix
+import ftl.http.executeWithRetry
+import io.mockk.every
+import io.mockk.mockkStatic
+
+fun mockTestMatrices(
+    vararg mocks: TestMatrix
+) {
+    mockkStatic("ftl.http.ExecuteWithRetryKt")
+    every {
+        any<Testing.Projects.TestMatrices.Get>().executeWithRetry()
+    } returnsMany listOf(*mocks)
+}
+
+fun getMockedTestMatrix() = TestMatrix().apply {
+    state = "INVALID"
+    testMatrixId = "matrix-12345"
+    testExecutions = listOf(
+        TestExecution().apply {
+            resultStorage = ResultStorage().apply {
+                googleCloudStorage = GoogleCloudStorage().apply {
+                    gcsPath = "any/Path"
+                }
+            }
+        }
+    )
+}