Fix overlapping results

release_notes.md

@@ -1,15 +1,15 @@
 ## next (unreleased)
 
-- [#784](https://github.com/Flank/flank/pull/784) Add output-style option. ([jan-gogo](https://github.com/jan-gogo)) 
+- [#784](https://github.com/Flank/flank/pull/784) Add output-style option. ([jan-gogo](https://github.com/jan-gogo))
 - [#779](https://github.com/Flank/flank/pull/779) Print retries & display additional info. ([jan-gogo](https://github.com/jan-gogo))
-- [#793](https://github.com/Flank/flank/issues/793) Better error message on file not found. ([adamfilipow92](https://github.com/adamfilipow92)) 
-- [#808](https://github.com/Flank/flank/issues/808) Fixed dry run crashes. ([piotradamczyk5](https://github.com/piotradamczyk5)) 
-- [#807](https://github.com/Flank/flank/issues/807) Fix Bugsnag being initialized during tests. ([piotradamczyk5](https://github.com/piotradamczyk5)) 
-- 
+- [#793](https://github.com/Flank/flank/issues/793) Better error message on file not found. ([adamfilipow92](https://github.com/adamfilipow92))
+- [#808](https://github.com/Flank/flank/issues/808) Fixed dry run crashes. ([piotradamczyk5](https://github.com/piotradamczyk5))
+- [#807](https://github.com/Flank/flank/issues/807) Fix Bugsnag being initialized during tests. ([piotradamczyk5](https://github.com/piotradamczyk5))
+- [#805](https://github.com/Flank/flank/pull/805) Fix overlapping results. ([pawelpasterz](https://github.com/pawelpasterz))
 
 ## v20.05.2
 
-- [#781](https://github.com/Flank/flank/pull/781) Remove local exists check on cloud results-dir. Fixes crash when results-dir is set by the user. ([adamfilipow92](https://github.com/adamfilipow92)) 
+- [#781](https://github.com/Flank/flank/pull/781) Remove local exists check on cloud results-dir. Fixes crash when results-dir is set by the user. ([adamfilipow92](https://github.com/adamfilipow92))
 - [#656](https://github.com/Flank/flank/issues/656) Improve error message reporting. ([adamfilipow92](https://github.com/adamfilipow92))
 - [#783](https://github.com/Flank/flank/pull/783) Use legacy results for iOS by default. ([pawelpasterz](https://github.com/pawelpasterz))
 

test_runner/docs/ascii/flank.jar_-android-run.adoc

@@ -144,7 +144,7 @@ Configuration is read from flank.yml
   Keeps the full path of downloaded files. Required when file names are not unique.
 
 *--output-style*=_<outputStyle>_::
-  Output style of execution status. Maybe be one of [verbose, multi, single]For runs with only one test execution default value is 'verbose', in other cases'multi' is used as default. Output style 'multi' is not displayed correctly on consoleswhich not supports ansi codes, to avoid corrupted output use `single` or `verbose`.
+  Output style of execution status. May be one of [verbose, multi, single]. For runs with only one test execution the default value is 'verbose', in other cases 'multi' is used as the default. The output style 'multi' is not displayed correctly on consoles which don't support ansi codes, to avoid corrupted output use `single` or `verbose`.
 
 *--dump-shards*::
   Dumps the shards to android_shards.json for debugging

test_runner/docs/ascii/flank.jar_-firebase-test-android-run.adoc

@@ -156,7 +156,7 @@ Configuration is read from flank.yml
   Keeps the full path of downloaded files. Required when file names are not unique.
 
 *--output-style*=_<outputStyle>_::
-  Output style of execution status. Maybe be one of [verbose, multi, single]For runs with only one test execution default value is 'verbose', in other cases'multi' is used as default. Output style 'multi' is not displayed correctly on consoleswhich not supports ansi codes, to avoid corrupted output use `single` or `verbose`.
+  Output style of execution status. May be one of [verbose, multi, single]. For runs with only one test execution the default value is 'verbose', in other cases 'multi' is used as the default. The output style 'multi' is not displayed correctly on consoles which don't support ansi codes, to avoid corrupted output use `single` or `verbose`.
 
 *--dump-shards*::
   Dumps the shards to android_shards.json for debugging

test_runner/docs/ascii/flank.jar_-firebase-test-ios-run.adoc

@@ -136,7 +136,7 @@ Configuration is read from flank.yml
   Keeps the full path of downloaded files. Required when file names are not unique.
 
 *--output-style*=_<outputStyle>_::
-  Output style of execution status. Maybe be one of [verbose, multi, single]For runs with only one test execution default value is 'verbose', in other cases'multi' is used as default. Output style 'multi' is not displayed correctly on consoleswhich not supports ansi codes, to avoid corrupted output use `single` or `verbose`.
+  Output style of execution status. May be one of [verbose, multi, single]. For runs with only one test execution the default value is 'verbose', in other cases 'multi' is used as the default. The output style 'multi' is not displayed correctly on consoles which don't support ansi codes, to avoid corrupted output use `single` or `verbose`.
 
 *--dump-shards*::
   Dumps the shards to ios_shards.json for debugging

test_runner/docs/ascii/flank.jar_-ios-run.adoc

@@ -128,7 +128,7 @@ Configuration is read from flank.yml
   Keeps the full path of downloaded files. Required when file names are not unique.
 
 *--output-style*=_<outputStyle>_::
-  Output style of execution status. Maybe be one of [verbose, multi, single]For runs with only one test execution default value is 'verbose', in other cases'multi' is used as default. Output style 'multi' is not displayed correctly on consoleswhich not supports ansi codes, to avoid corrupted output use `single` or `verbose`.
+  Output style of execution status. May be one of [verbose, multi, single]. For runs with only one test execution the default value is 'verbose', in other cases 'multi' is used as the default. The output style 'multi' is not displayed correctly on consoles which don't support ansi codes, to avoid corrupted output use `single` or `verbose`.
 
 *--dump-shards*::
   Dumps the shards to ios_shards.json for debugging

test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt

@@ -41,7 +41,7 @@ object ReportManager {
         val objName = matrices.runPath // 2019-03-22_17-20-53.594000_ftrh
 
         // shard location in path changes based on iOS or Android.
-        val matchResult = Regex("/(shard_\\d+)(-rerun_\\d+)?/").find(xmlFile.toString())
+        val matchResult = Regex("/.*(shard_\\d+)(-rerun_\\d+)?/").find(xmlFile.toString())
         val shardName = matchResult?.value?.removePrefix("/")?.removeSuffix("/") // shard_0 || shard_0-rerun_1
         val matrixPath = Paths.get(objName, shardName).toString() // 2019-03-22_17-20-53.594000_ftrh/shard_0
 

test_runner/src/main/kotlin/ftl/run/common/FetchArtifacts.kt

@@ -1,20 +1,18 @@
 package ftl.run.common
 
 import com.google.cloud.storage.Storage
-import ftl.args.AndroidArgs
 import ftl.args.IArgs
 import ftl.config.FtlConstants
-import ftl.gc.GcStorage
 import ftl.json.MatrixMap
+import ftl.gc.GcStorage
 import ftl.util.Artifacts
 import ftl.util.MatrixState
-import ftl.util.ObjPath
+import java.nio.file.Path
+import java.nio.file.Paths
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.joinAll
 import kotlinx.coroutines.launch
-import java.nio.file.Path
-import java.nio.file.Paths
 
 internal suspend fun fetchArtifacts(matrixMap: MatrixMap, args: IArgs) = coroutineScope {
     println("FetchArtifacts")
@@ -64,17 +62,13 @@ internal suspend fun fetchArtifacts(matrixMap: MatrixMap, args: IArgs) = corouti
 
 internal fun getDownloadPath(args: IArgs, blobPath: String): Path {
     val localDir = args.localResultDir
-    val p = if (args is AndroidArgs)
-        ObjPath.parse(blobPath) else
-        ObjPath.legacyParse(blobPath)
+    val parsed = Paths.get(blobPath)
+    val objName = if (args.useLocalResultDir()) "" else parsed.getName(0).toString()
+    // for iOS it is shardName, remove this comment after FTL introduce server side sharding for iOS
+    val matrixName = parsed.getName(1).toString()
+    val deviceName = parsed.getName(2).toString()
+    val filePathName = if (args.keepFilePath) parsed.parent.drop(3).joinToString("/") else ""
+    val fileName = parsed.fileName.toString()
 
-    // Store downloaded artifacts at device root.
-    return Paths.get(
-        localDir,
-        if (args.useLocalResultDir().not()) p.objName else "",
-        p.shardName,
-        p.deviceName,
-        if (args.keepFilePath) p.filePathName else "",
-        p.fileName
-    )
+    return Paths.get("$localDir/$objName/$matrixName/$deviceName/$filePathName/$fileName")
 }

test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt

@@ -38,7 +38,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
     val history = GcToolResults.createToolResultsHistory(args)
     val otherGcsFiles = args.uploadOtherFiles(runGcsPath)
 
-    args.resolveApks().forEach { apks: ResolvedApks ->
+    args.resolveApks().forEachIndexed { index: Int, apks: ResolvedApks ->
         val testShards = apks.test?.let { test ->
             AndroidTestShard.getTestShardChunks(args, test)
         }
@@ -47,7 +47,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
             val shardsWithAtLeastOneTest = testShards.filterAtLeastOneTest()
             if (shardsWithAtLeastOneTest.isEmpty()) {
                 // No tests to run, skipping the execution.
-                return@forEach
+                return@forEachIndexed
             }
             allTestShardChunks += shardsWithAtLeastOneTest
         }
@@ -67,7 +67,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
         testMatrices += executeAndroidTestMatrix(runCount = args.repeatTests) {
             GcAndroidTestMatrix.build(
                 androidTestConfig = androidTestConfig,
-                runGcsPath = runGcsPath,
+                runGcsPath = "$runGcsPath/matrix_$index/",
                 additionalApkGcsPaths = uploadedApks.additionalApks,
                 androidDeviceList = androidDeviceList,
                 args = args,

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

@@ -84,8 +84,6 @@ fun uniqueObjectName(): String {
         bucketName.append(letter)
     }
 
-    bucketName.append("/")
-
     return bucketName.toString()
 }
 

test_runner/src/test/kotlin/ftl/run/TestRunnerTest.kt

@@ -12,7 +12,6 @@ import ftl.config.FtlConstants.isWindows
 import ftl.http.executeWithRetry
 import ftl.run.common.getDownloadPath
 import ftl.test.util.FlankTestRunner
-import ftl.util.ObjPath
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.mockkStatic
@@ -29,14 +28,19 @@ import org.junit.Test
 import org.junit.contrib.java.lang.system.SystemOutRule
 import org.junit.runner.RunWith
 
+private const val OBJECT_NAME = "2019-03-22_15-30-02.189000_frjt"
+private const val FILE_NAME = "StandardOutputAndStandardError.txt"
+private const val SHARD = "shard_0"
+private const val MATRIX = "matrix_1"
+private const val ANDROID_DEVICE = "NexusLowRes-27-en-portrait-shard_0-rerun_1"
+private const val IOS_DEVICE = "iphone8-12.0-en-portrait"
+private const val LOCAL_DIR = "results"
+private const val TEST_FILE_PATH = "any/path/that/is/possible"
+private const val FULL_IOS_PATH = "$OBJECT_NAME/$SHARD/$IOS_DEVICE/$TEST_FILE_PATH/$FILE_NAME"
+private const val FULL_ANDROID_PATH = "$OBJECT_NAME/$MATRIX/$ANDROID_DEVICE/$TEST_FILE_PATH/$FILE_NAME"
+
 @RunWith(FlankTestRunner::class)
 class TestRunnerTest {
-
-    private val gcsIosPath =
-        "2019-03-22_15-30-02.189000_frjt/shard_0/iphone8-12.0-en-portrait/TestLogs/Test-Transient Testing-2019.03.22_08-29-41--0700.xcresult/1_Test/Diagnostics/EarlGreyExampleSwiftTests-C6803D8C-4BDB-4C84-8945-9AC64056FBA4/EarlGreyExampleSwiftTests-EDBFF942-A88A-46A5-87CA-A1E29555C2CA/StandardOutputAndStandardError.txt"
-    private val gcsAndroidPath =
-        "2019-03-22_15-30-02.189000_frjt/iphone8-12.0-en-portrait-shard_0/StandardOutputAndStandardError.txt"
-    private val localResultDir = "results"
     private val iosArgs = mockk<IosArgs>()
     private val androidArgs = mockk<AndroidArgs>()
 
@@ -48,123 +52,62 @@ class TestRunnerTest {
 
     @Test
     fun `Verify getDownloadPath localResultDir false and keepFilePath false - ios`() {
-        val parsed = ObjPath.legacyParse(gcsIosPath)
-
-        every { iosArgs.localResultDir } returns localResultDir
+        every { iosArgs.localResultDir } returns LOCAL_DIR
         every { iosArgs.useLocalResultDir() } returns false
         every { iosArgs.keepFilePath } returns false
 
-        val downloadFile = getDownloadPath(iosArgs, gcsIosPath)
-        assertThat(downloadFile).isEqualTo(
-            Paths.get(
-                localResultDir,
-                parsed.objName,
-                parsed.shardName,
-                parsed.deviceName,
-                parsed.fileName
-            )
-        )
+        val downloadFile = getDownloadPath(iosArgs, FULL_IOS_PATH)
+        assertThat(downloadFile).isEqualTo(Paths.get("$LOCAL_DIR/$OBJECT_NAME/$SHARD/$IOS_DEVICE/$FILE_NAME"))
     }
 
     @Test
     fun `Verify getDownloadPath localResultDir false and keepFilePath true - ios`() {
-        val parsed = ObjPath.legacyParse(gcsIosPath)
-
-        every { iosArgs.localResultDir } returns localResultDir
+        every { iosArgs.localResultDir } returns LOCAL_DIR
         every { iosArgs.useLocalResultDir() } returns false
         every { iosArgs.keepFilePath } returns true
 
-        val downloadFile = getDownloadPath(iosArgs, gcsIosPath)
-        assertThat(downloadFile).isEqualTo(
-            Paths.get(
-                localResultDir,
-                parsed.objName,
-                parsed.shardName,
-                parsed.deviceName,
-                parsed.filePathName,
-                parsed.fileName
-            )
-        )
+        val downloadFile = getDownloadPath(iosArgs, FULL_IOS_PATH)
+        assertThat(downloadFile).isEqualTo(Paths.get("$LOCAL_DIR/$FULL_IOS_PATH"))
     }
 
     @Test
     fun `Verify getDownloadPath localResultDir true and keepFilePath false - ios`() {
-        val parsed = ObjPath.legacyParse(gcsIosPath)
-
-        every { iosArgs.localResultDir } returns localResultDir
+        every { iosArgs.localResultDir } returns LOCAL_DIR
         every { iosArgs.useLocalResultDir() } returns true
         every { iosArgs.keepFilePath } returns false
 
-        val downloadFile = getDownloadPath(iosArgs, gcsIosPath)
-        assertThat(downloadFile).isEqualTo(
-            Paths.get(
-                localResultDir,
-                parsed.shardName,
-                parsed.deviceName,
-                parsed.fileName
-            )
-        )
+        val downloadFile = getDownloadPath(iosArgs, FULL_IOS_PATH)
+        assertThat(downloadFile).isEqualTo(Paths.get("$LOCAL_DIR/$SHARD/$IOS_DEVICE/$FILE_NAME"))
     }
 
     @Test
     fun `Verify getDownloadPath localResultDir true and keepFilePath true - ios`() {
-        val parsed = ObjPath.legacyParse(gcsIosPath)
-
-        every { iosArgs.localResultDir } returns localResultDir
+        every { iosArgs.localResultDir } returns LOCAL_DIR
         every { iosArgs.useLocalResultDir() } returns true
         every { iosArgs.keepFilePath } returns true
 
-        val downloadFile = getDownloadPath(iosArgs, gcsIosPath)
-        assertThat(downloadFile).isEqualTo(
-            Paths.get(
-                localResultDir,
-                parsed.shardName,
-                parsed.deviceName,
-                parsed.filePathName,
-                parsed.fileName
-            )
-        )
+        val downloadFile = getDownloadPath(iosArgs, FULL_IOS_PATH)
+        assertThat(downloadFile).isEqualTo(Paths.get("$LOCAL_DIR/$SHARD/$IOS_DEVICE/$TEST_FILE_PATH/$FILE_NAME"))
     }
 
     @Test
     fun `Verify getDownloadPath localResultDir true and keepFilePath true - android`() {
-        val parsed = ObjPath.parse(gcsAndroidPath)
-
-        every { androidArgs.localResultDir } returns localResultDir
+        every { androidArgs.localResultDir } returns LOCAL_DIR
         every { androidArgs.useLocalResultDir() } returns true
         every { androidArgs.keepFilePath } returns true
 
-        val downloadFile = getDownloadPath(androidArgs, gcsAndroidPath)
-        assertThat(downloadFile).isEqualTo(
-            Paths.get(
-                localResultDir,
-                parsed.shardName,
-                parsed.deviceName,
-                parsed.filePathName,
-                parsed.fileName
-            )
-        )
+        val downloadFile = getDownloadPath(androidArgs, FULL_ANDROID_PATH)
+        assertThat(downloadFile).isEqualTo(Paths.get("$LOCAL_DIR/$MATRIX/$ANDROID_DEVICE/$TEST_FILE_PATH/$FILE_NAME"))
     }
 
     @Test
     fun `Verify getDownloadPath localResultDir false and keepFilePath true`() {
-        val parsed = ObjPath.parse(gcsAndroidPath)
-
-        every { androidArgs.localResultDir } returns localResultDir
+        every { androidArgs.localResultDir } returns LOCAL_DIR
         every { androidArgs.useLocalResultDir() } returns false
         every { androidArgs.keepFilePath } returns true
 
-        val downloadFile = getDownloadPath(androidArgs, gcsAndroidPath)
-        assertThat(downloadFile).isEqualTo(
-            Paths.get(
-                localResultDir,
-                parsed.objName,
-                parsed.shardName,
-                parsed.deviceName,
-                parsed.filePathName,
-                parsed.fileName
-            )
-        )
+        val downloadFile = getDownloadPath(androidArgs, FULL_ANDROID_PATH)
+        assertThat(downloadFile).isEqualTo(Paths.get("$LOCAL_DIR/$FULL_ANDROID_PATH"))
     }
 
     @Test