fix: Uploading performance metrics for multiple matrices

test_runner/src/main/kotlin/ftl/gc/GcStorage.kt

@@ -18,8 +18,8 @@ import ftl.reports.xml.model.JUnitTestResult
 import ftl.reports.xml.parseAllSuitesXml
 import ftl.reports.xml.xmlToString
 import ftl.run.exception.FlankGeneralError
-import ftl.util.ProgressBar
 import ftl.util.join
+import ftl.util.runWithProgress
 import java.io.File
 import java.io.FileOutputStream
 import java.net.URI
@@ -73,16 +73,11 @@ object GcStorage {
         val name = rawPath.substringAfter('/')
 
         val fileBlob = BlobInfo.newBuilder(bucket, name).build()
-
-        val progress = ProgressBar()
-        try {
-            progress.start("Uploading smart flank XML")
-            storage.create(fileBlob, testResult.xmlToString().toByteArray())
-        } catch (e: Exception) {
-            throw FlankGeneralError(e)
-        } finally {
-            progress.stop()
-        }
+        runWithProgress(
+            startMessage = "Uploading smart flank XML",
+            action = { storage.create(fileBlob, testResult.xmlToString().toByteArray()) },
+            onError = { throw FlankGeneralError(it) }
+        )
     }
 
     fun uploadPerformanceMetrics(perfMetricsSummary: PerfMetricsSummary, resultsBucket: String, resultDir: String) =
@@ -95,8 +90,6 @@ object GcStorage {
             )
         }.onFailure {
             println("Cannot upload performance metrics ${it.message}")
-        }.onSuccess {
-            println("Performance metrics uploaded to https://console.developers.google.com/storage/browser/$resultsBucket/$resultDir")
         }.getOrNull()
 
     fun uploadReportResult(testResult: String, args: IArgs, fileName: String) {
@@ -141,10 +134,8 @@ object GcStorage {
         storage: Storage = GcStorage.storage
     ): String {
         val file = File(filePath)
-        val absolutePath = file.absolutePath
-        val fileName = file.name
-        return uploadCache[absolutePath] ?: uploadCache.computeIfAbsent(absolutePath) {
-            val gcsPath = join(runGcsPath, fileName)
+        return uploadCache.computeIfAbsent("$runGcsPath-${file.absolutePath}") {
+            val gcsPath = join(runGcsPath, file.name)
             val index = duplicatedGcsPathCounter.merge(gcsPath, 0) { old, _ -> old + 1 }
             val validGcsPath = when {
                 index == 0 -> gcsPath
@@ -154,16 +145,11 @@ object GcStorage {
 
             // 404 Not Found error when rootGcsBucket does not exist
             val fileBlob = BlobInfo.newBuilder(rootGcsBucket, validGcsPath).build()
-
-            val progress = ProgressBar()
-            try {
-                progress.start("Uploading $fileName")
-                storage.create(fileBlob, fileBytes)
-            } catch (e: Exception) {
-                throw FlankGeneralError("Error on uploading $fileName\nCause: $e")
-            } finally {
-                progress.stop()
-            }
+            runWithProgress(
+                startMessage = "Uploading ${file.name}",
+                action = { storage.create(fileBlob, fileBytes) },
+                onError = { throw FlankGeneralError("Error on uploading ${file.name}\nCause: $it") }
+            )
             GCS_PREFIX + join(rootGcsBucket, validGcsPath)
         }
     }

test_runner/src/main/kotlin/ftl/reports/api/PerformanceMetrics.kt

@@ -21,7 +21,9 @@ internal fun List<Pair<TestExecution, String>>.getAndUploadPerformanceMetrics(
             async(Dispatchers.IO) {
                 val performanceMetrics = testExecution.getPerformanceMetric()
                 performanceMetrics.save(gcsStoragePath, args)
-                performanceMetrics.upload(resultBucket = args.resultsBucket, resultDir = gcsStoragePath)
+                if (args.disableResultsUpload.not()) {
+                    performanceMetrics.upload(resultBucket = args.resultsBucket, resultDir = gcsStoragePath)
+                }
             }
         }
         .awaitAll()

test_runner/src/main/kotlin/ftl/util/ProgressBar.kt

@@ -24,3 +24,19 @@ private class ProgressBarTask : TimerTask() {
         print(".")
     }
 }
+
+fun runWithProgress(
+    startMessage: String,
+    action: () -> Unit,
+    onError: (Exception) -> Unit
+) {
+    val progress = ProgressBar()
+    try {
+        progress.start(startMessage)
+        action()
+    } catch (e: Exception) {
+        onError(e)
+    } finally {
+        progress.stop()
+    }
+}

test_runner/src/test/kotlin/ftl/reports/api/PerformanceMetricsTest.kt

@@ -37,7 +37,7 @@ class PerformanceMetricsTest {
     }
 
     @Test
-    fun `should get and upload performance metrics for physical devices`() {
+    fun `should get and upload performance metrics for physical devices if results upload enabled`() {
         val expectedBucket = "bucket"
         val expectedPath = "path"
 
@@ -52,6 +52,7 @@ class PerformanceMetricsTest {
                     every { resultsBucket } returns expectedBucket
                     every { useLocalResultDir() } returns false
                     every { localResultDir } returns "local"
+                    every { disableResultsUpload } returns false
                 }
                 testExecutions.map { it to expectedPath }.getAndUploadPerformanceMetrics(args)
                 performanceMetrics.forEach {
@@ -61,6 +62,32 @@ class PerformanceMetricsTest {
         }
     }
 
+    @Test
+    fun `should get and not upload performance metrics for physical devices if results upload disabled`() {
+        val expectedBucket = "bucket"
+        val expectedPath = "path"
+
+        mockkObject(AndroidCatalog) {
+            every { AndroidCatalog.isVirtualDevice(any<AndroidDevice>(), any()) } returns false
+            val performanceMetrics = testExecutions.map {
+                GcToolResults.getPerformanceMetric(it.toolResultsStep)
+            }
+
+            mockkObject(GcStorage) {
+                val args = mockk<IArgs> {
+                    every { resultsBucket } returns expectedBucket
+                    every { useLocalResultDir() } returns false
+                    every { localResultDir } returns "local"
+                    every { disableResultsUpload } returns true
+                }
+                testExecutions.map { it to expectedPath }.getAndUploadPerformanceMetrics(args)
+                performanceMetrics.forEach {
+                    verify(exactly = 0) { GcStorage.uploadPerformanceMetrics(it, expectedBucket, expectedPath) }
+                }
+            }
+        }
+    }
+
     private val testExecutions = (1..5).map {
         mockk<TestExecution> {
             every { projectId } returns "test"

test_runner/src/test/kotlin/ftl/util/ProgressBarTest.kt

@@ -2,6 +2,9 @@ package ftl.util
 
 import com.google.common.truth.Truth.assertThat
 import ftl.test.util.TestHelper.normalizeLineEnding
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.verify
 import org.junit.Rule
 import org.junit.Test
 import org.junit.contrib.java.lang.system.SystemOutRule
@@ -23,4 +26,53 @@ class ProgressBarTest {
         progress.stop()
         assertThat(systemOutRule.log.normalizeLineEnding()).isEqualTo("  hi .\n")
     }
+
+    @Test
+    fun `should properly wrap progress bar execution using helper function for success`() {
+        // given
+        val action: Runnable = mockk {
+            every { run() } returns Unit
+        }
+        val onError: Runnable = mockk {
+            every { run() } returns Unit
+        }
+
+        // when
+        runWithProgress(
+            startMessage = "TEST",
+            action = { action.run() },
+            onError = { onError.run() }
+        )
+
+        // then
+        assertThat(systemOutRule.log).contains("  TEST ")
+        verify { action.run() }
+        verify(exactly = 0) { onError.run() }
+    }
+
+    @Test
+    fun `should properly wrap progress bar execution using helper function for failure`() {
+        // given
+        val action: Runnable = mockk {
+            every { run() } returns Unit
+        }
+        val onError: Runnable = mockk {
+            every { run() } returns Unit
+        }
+
+        // when
+        runWithProgress(
+            startMessage = "TEST",
+            action = {
+                action.run()
+                throw Exception()
+            },
+            onError = { onError.run() }
+        )
+
+        // then
+        assertThat(systemOutRule.log).contains("  TEST ")
+        verify { action.run() }
+        verify { onError.run() }
+    }
 }