fix: Matrix path not found in json (#1067)

* fix: Matrix path not found in json * Remove fast fail on existing bucket * Update ValidateCommonArgs.kt * Delete ValidateResultsDirUniqueTests.kt * Update ReportManagerTest.kt * Update ReportManagerTest.kt * Remove unused method * detekt * Update test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt Co-authored-by: Jan Góral <[email protected]> * Print warning when results dir already exist * fix: Matrix path not found in json * Remove fast fail on existing bucket * Update ValidateCommonArgs.kt * Delete ValidateResultsDirUniqueTests.kt * Update ReportManagerTest.kt * Update ReportManagerTest.kt * Remove unused method * detekt * Update test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt Co-authored-by: Jan Góral <[email protected]> * Print warning when results dir already exist * Fix matching text for legacy Android and iOS Co-authored-by: Adam <[email protected]> Co-authored-by: adamfilipow92 <[email protected]> Co-authored-by: Jan Góral <[email protected]>
Flank · Sep 9, 2020 · a36407c · a36407c
1 parent d12f42f
commit a36407c
Show file tree

Hide file tree

Showing 11 changed files with 258 additions and 28 deletions.
diff --git a/test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt
@@ -21,6 +21,7 @@ fun AndroidArgs.validate() = apply {
     assertParametersConflict()
     assertTestFiles()
     assertOtherFiles()
+    checkResultsDirUnique()
 }
 
 private fun AndroidArgs.assertDevicesSupported() = devices

diff --git a/test_runner/src/main/kotlin/ftl/args/ValidateCommonArgs.kt b/test_runner/src/main/kotlin/ftl/args/ValidateCommonArgs.kt
@@ -2,6 +2,7 @@ package ftl.args
 
 import ftl.config.Device
 import ftl.config.FtlConstants
+import ftl.gc.GcStorage
 import ftl.reports.FullJUnitReport
 import ftl.reports.JUnitReport
 import ftl.run.exception.FlankConfigurationError
@@ -28,8 +29,8 @@ private fun List<Device>.throwIfAnyMisspelt() =
 private fun CommonArgs.assertProjectId() {
     if (project.isEmpty()) throw FlankConfigurationError(
         "The project is not set. Define GOOGLE_CLOUD_PROJECT, set project in flank.yml\n" +
-            "or save service account credential to ${FtlConstants.defaultCredentialPath}\n" +
-            " See https://github.com/GoogleCloudPlatform/google-cloud-java#specifying-a-project-id"
+                "or save service account credential to ${FtlConstants.defaultCredentialPath}\n" +
+                " See https://github.com/GoogleCloudPlatform/google-cloud-java#specifying-a-project-id"
     )
 }
 
@@ -66,3 +67,8 @@ private fun CommonArgs.assertSmartFlankGcsPath() = with(smartFlankGcsPath) {
         )
     }
 }
+
+fun IArgs.checkResultsDirUnique() {
+    if (useLegacyJUnitResult && GcStorage.exist(resultsBucket, resultsDir))
+        println("WARNING: Google cloud storage result directory should be unique, otherwise results from multiple test matrices will be overwritten or intermingled\n")
+}
diff --git a/test_runner/src/main/kotlin/ftl/args/ValidateIosArgs.kt b/test_runner/src/main/kotlin/ftl/args/ValidateIosArgs.kt
@@ -11,6 +11,7 @@ fun IosArgs.validate() = apply {
     assertTestTypes()
     assertMaxTestShards()
     assertTestFiles()
+    checkResultsDirUnique()
 }
 
 fun IosArgs.validateRefresh() = apply {

diff --git a/test_runner/src/main/kotlin/ftl/gc/GcStorage.kt b/test_runner/src/main/kotlin/ftl/gc/GcStorage.kt
@@ -3,6 +3,8 @@ package ftl.gc
 import com.google.api.client.http.GoogleApiLogger
 import com.google.cloud.storage.BlobInfo
 import com.google.cloud.storage.Storage
+import com.google.cloud.storage.Storage.BlobListOption.pageSize
+import com.google.cloud.storage.Storage.BlobListOption.prefix
 import com.google.cloud.storage.StorageOptions
 import com.google.cloud.storage.contrib.nio.testing.LocalStorageHelper
 import com.google.common.annotations.VisibleForTesting
@@ -172,4 +174,10 @@ object GcStorage {
             outputFile.path
         }
     }
+
+    fun exist(
+        rootGcsBucket: String,
+        runGcsPath: String,
+        storage: Storage = GcStorage.storage
+    ) = storage.list(rootGcsBucket, pageSize(1), prefix("$runGcsPath/")).values.count() > 0
 }
diff --git a/test_runner/src/main/kotlin/ftl/reports/util/MatrixPath.kt b/test_runner/src/main/kotlin/ftl/reports/util/MatrixPath.kt
@@ -3,9 +3,16 @@ package ftl.reports.util
 import java.io.File
 import java.nio.file.Paths
 
-fun File.getMatrixPath(objectName: String) = getShardName()?.asObjectPath(objectName)
+fun File.getMatrixPath(objectName: String) = getShardName(objectName)?.asObjectPath()
 
-private fun File.getShardName() = shardNameRegex.find(toString())?.value?.removePrefix("/")?.removeSuffix("/")
-private val shardNameRegex = "/.*(shard_|matrix_)\\d+(-rerun_\\d+)?/".toRegex()
+private fun File.getShardName(
+    objectName: String
+) = shardNameRegex(objectName)
+    .find(toString())
+    ?.value
+    ?.removePrefix("/")
+    ?.removeSuffix("/")
 
-private fun String.asObjectPath(objectName: String) = Paths.get(objectName, this).toString()
+private fun shardNameRegex(objectName: String) = "/($objectName)/(shard_|matrix_)\\d+(-rerun_\\d+)?/".toRegex()
+
+private fun String.asObjectPath() = Paths.get(this).toString()
diff --git a/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt b/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt
@@ -28,21 +28,19 @@ import kotlin.math.roundToInt
 
 object ReportManager {
 
-    private fun findXmlFiles(matrices: MatrixMap, args: IArgs): List<File> {
-        val xmlFiles = mutableListOf<File>()
-        val rootFolder = File(resolveLocalRunPath(matrices, args))
-
-        rootFolder.walk().forEach {
-            if (it.name.matches(Artifacts.testResultRgx)) {
-                xmlFiles.add(it)
-            }
+    private fun findXmlFiles(
+        matrices: MatrixMap,
+        args: IArgs
+    ) = File(resolveLocalRunPath(matrices, args)).walk()
+        .filter { it.name.matches(Artifacts.testResultRgx) }
+        .fold(listOf<File>()) { xmlFiles, file ->
+            xmlFiles + file
         }
 
-        return xmlFiles
-    }
-
-    private fun getWebLink(matrices: MatrixMap, xmlFile: File): String = xmlFile.getMatrixPath(matrices.runPath)
-        ?.findMatrixPath(matrices) ?: "".also { println("WARNING: Matrix path not found in JSON.") }
+    private fun getWebLink(matrices: MatrixMap, xmlFile: File): String =
+        xmlFile.getMatrixPath(matrices.runPath)
+            ?.findMatrixPath(matrices)
+            ?: "".also { println("WARNING: Matrix path not found in JSON.") }
 
     private val deviceStringRgx = Regex("([^-]+-[^-]+-[^-]+-[^-]+).*")
 
@@ -201,6 +199,10 @@ object ReportManager {
     }
 }
 
-private fun String.findMatrixPath(matrices: MatrixMap) =
-    matrices.map.values.firstOrNull { savedMatrix -> savedMatrix.gcsPath.endsWith(this) }?.webLink
-        ?: "".also { println("WARNING: Matrix path not found in JSON. $this") }
+private fun String.findMatrixPath(matrices: MatrixMap) = matrices.map.values
+    .firstOrNull { savedMatrix -> savedMatrix.gcsPath.endsWithTextWithOptionalSlashAtTheEnd(this) }
+    ?.webLink
+    ?: "".also { println("WARNING: Matrix path not found in JSON. $this") }
+
+@VisibleForTesting
+internal fun String.endsWithTextWithOptionalSlashAtTheEnd(text: String) = "($text)/*$".toRegex().containsMatchIn(this)
diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt
@@ -9,6 +9,8 @@ import ftl.cli.firebase.test.android.AndroidRunCommand
 import ftl.config.Device
 import ftl.config.FtlConstants.defaultAndroidModel
 import ftl.config.FtlConstants.defaultAndroidVersion
+import ftl.gc.GcStorage
+import ftl.gc.GcStorage.exist
 import ftl.gc.android.setupAndroidTest
 import ftl.run.model.InstrumentationTestContext
 import ftl.run.platform.android.createAndroidTestConfig
@@ -25,6 +27,7 @@ import ftl.run.exception.FlankConfigurationError
 import ftl.run.exception.IncompatibleTestDimensionError
 import ftl.util.asFileReference
 import io.mockk.every
+import io.mockk.mockkObject
 import io.mockk.mockkStatic
 import io.mockk.unmockkAll
 import kotlinx.coroutines.runBlocking
@@ -33,7 +36,9 @@ import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Assert.fail
+import org.junit.Rule
 import org.junit.Test
+import org.junit.contrib.java.lang.system.SystemOutRule
 import org.junit.runner.RunWith
 import picocli.CommandLine
 import java.io.StringReader
@@ -44,6 +49,11 @@ import java.util.UUID
 @Suppress("TooManyFunctions")
 @RunWith(FlankTestRunner::class)
 class AndroidArgsTest {
+
+    @Rule
+    @JvmField
+    val systemOutRule: SystemOutRule = SystemOutRule().enableLog()
+
     private val empty = emptyList<String>()
     private val appApk = "../test_projects/android/apks/app-debug.apk"
     private val invalidApk = "../test_projects/android/apks/invalid.apk"
@@ -1596,6 +1606,7 @@ AndroidArgs
         """.trimIndent()
         AndroidArgs.load(yaml).validate()
     }
+
     @Test
     fun `should not throw when --legacy-junit-result set and --full-junit-result not set`() {
         val yaml = """
@@ -1768,6 +1779,69 @@ AndroidArgs
         """.trimIndent()
         AndroidArgs.load(yaml).validate()
     }
+
+    @Test
+    fun `should show warning message when bucket exist and legacy junit result used`() {
+        val yaml = """
+        gcloud:
+          app: $appApk
+          test: $testApk
+          results-dir: test
+        flank:
+          max-test-shards: -1
+          legacy-junit-result: true
+        """.trimIndent()
+        mockkObject(GcStorage) {
+            every { exist(any(), any(), any()) } returns true
+
+            // when
+            AndroidArgs.load(yaml).validate()
+
+            // then
+            assertTrue(systemOutRule.log.contains("WARNING: Google cloud storage result directory should be unique, otherwise results from multiple test matrices will be overwritten or intermingled"))
+        }
+    }
+
+    @Test
+    fun `should not print Google cloud storage warning when legacy junit results is false`() {
+        val yaml = """
+        gcloud:
+          app: $appApk
+          test: $testApk
+          results-dir: test
+        flank:
+          max-test-shards: -1
+          legacy-junit-result: false
+        """.trimIndent()
+
+        // when
+        AndroidArgs.load(yaml).validate()
+
+        // then
+        assertFalse(systemOutRule.log.contains("WARNING: Google cloud storage result directory should be unique, otherwise results from multiple test matrices will be overwritten or intermingled"))
+    }
+
+    @Test
+    fun `should not print result-dir warning when same directory does not exist`() {
+        val yaml = """
+        gcloud:
+          app: $appApk
+          test: $testApk
+          results-dir: test
+        flank:
+          max-test-shards: -1
+          legacy-junit-result: true
+        """.trimIndent()
+        mockkObject(GcStorage) {
+            every { exist(any(), any(), any()) } returns false
+
+            // when
+            AndroidArgs.load(yaml).validate()
+
+            // then
+            assertFalse(systemOutRule.log.contains("WARNING: Google cloud storage result directory should be unique, otherwise results from multiple test matrices will be overwritten or intermingled"))
+        }
+    }
 }
 
 private fun AndroidArgs.Companion.load(yamlData: String, cli: AndroidRunCommand? = null): AndroidArgs =

diff --git a/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt
@@ -7,20 +7,24 @@ import ftl.config.FtlConstants
 import ftl.config.FtlConstants.defaultIosModel
 import ftl.config.FtlConstants.defaultIosVersion
 import ftl.config.defaultIosConfig
+import ftl.gc.GcStorage
+import ftl.run.exception.FlankConfigurationError
 import ftl.run.status.OutputStyle
 import ftl.test.util.FlankTestRunner
 import ftl.test.util.TestHelper.absolutePath
 import ftl.test.util.TestHelper.assert
 import ftl.test.util.TestHelper.getPath
 import ftl.test.util.assertThrowsWithMessage
-import ftl.run.exception.FlankConfigurationError
-import org.junit.Assert
+import io.mockk.every
+import io.mockk.mockkObject
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
 import org.junit.Assume
 import org.junit.Rule
 import org.junit.Test
 import org.junit.contrib.java.lang.system.SystemErrRule
+import org.junit.contrib.java.lang.system.SystemOutRule
 import org.junit.runner.RunWith
 import picocli.CommandLine
 import java.io.StringReader
@@ -93,6 +97,10 @@ class IosArgsTest {
     @JvmField
     val systemErrRule = SystemErrRule().muteForSuccessfulTests()!!
 
+    @Rule
+    @JvmField
+    val systemOutRule: SystemOutRule = SystemOutRule().enableLog()
+
     @Test
     fun `empty testTargets`() {
         val emptyTestTargets = """
@@ -1032,7 +1040,7 @@ IosArgs
           use-average-test-time-for-new-tests: true
         """.trimIndent()
         val args = IosArgs.load(yaml)
-        Assert.assertTrue(args.useAverageTestTimeForNewTests)
+        assertTrue(args.useAverageTestTimeForNewTests)
     }
 
     @Test
@@ -1047,6 +1055,48 @@ IosArgs
         val args = IosArgs.load(yaml)
         assertFalse(args.useAverageTestTimeForNewTests)
     }
+
+    @Test
+    fun `should show warning message when results directory exist`() {
+        val yaml = """
+        gcloud:
+          test: $testPath
+          xctestrun-file: $testPath
+          results-dir: test
+        flank:
+          max-test-shards: -1
+        """.trimIndent()
+        mockkObject(GcStorage) {
+            every { GcStorage.exist(any(), any(), any()) } returns true
+
+            // when
+            IosArgs.load(yaml).validate()
+
+            // then
+            assertTrue(systemOutRule.log.contains("WARNING: Google cloud storage result directory should be unique, otherwise results from multiple test matrices will be overwritten or intermingled"))
+        }
+    }
+
+    @Test
+    fun `should not print result-dir warning when same directory does not exist`() {
+        val yaml = """
+        gcloud:
+          test: $testPath
+          xctestrun-file: $testPath
+          results-dir: test
+        flank:
+          max-test-shards: -1
+        """.trimIndent()
+        mockkObject(GcStorage) {
+            every { GcStorage.exist(any(), any(), any()) } returns false
+
+            // when
+            IosArgs.load(yaml).validate()
+
+            // then
+            assertFalse(systemOutRule.log.contains("WARNING: Google cloud storage result directory should be unique, otherwise results from multiple test matrices will be overwritten or intermingled"))
+        }
+    }
 }
 
 private fun IosArgs.Companion.load(yamlData: String, cli: IosRunCommand? = null): IosArgs =