fix: Always dump shards (#1221)

* Add dump shards on run ios/android tests * Added tests and upload on gcs * Add ios tests * Add android test to verify calculation shards is called one * Add obfuscate option to args and use in testRun * Update DumpShardsKtTest.kt * Update CreateIosArgs.kt * remove unnecessary json * Update DumpShardsKtTest.kt * Update GetAndroidMatrixShards.kt * Cr fixes * Fix creating empty dump on robo tests * Update test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidRunCommand.kt Co-authored-by: Jan Góral <[email protected]> Co-authored-by: Jan Góral <[email protected]> Co-authored-by: piotradamczyk5 <[email protected]>
Flank · Oct 13, 2020 · f34ee09 · f34ee09
1 parent b2501e2
commit f34ee09
Show file tree

Hide file tree

Showing 16 changed files with 130 additions and 24 deletions.
diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
@@ -20,7 +20,8 @@ data class AndroidArgs(
     val testRunnerClass: String?,
     val testTargets: List<String>,
     val additionalAppTestApks: List<AppTestPair>,
-    override val useLegacyJUnitResult: Boolean
+    override val useLegacyJUnitResult: Boolean,
+    val obfuscateDumpShards: Boolean
 ) : IArgs by commonArgs {
     companion object : AndroidArgsCompanion()
 

diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidArgsCompanion.kt b/test_runner/src/main/kotlin/ftl/args/AndroidArgsCompanion.kt
@@ -38,7 +38,8 @@ open class AndroidArgsCompanion : IArgs.ICompanion {
     internal fun load(yamlReader: Reader, cli: AndroidRunCommand? = null) =
         createAndroidArgs(
             config = defaultAndroidConfig() +
-                    loadAndroidConfig(reader = yamlReader) +
-                    cli?.config
+                loadAndroidConfig(reader = yamlReader) +
+                cli?.config,
+            obfuscate = cli?.obfuscate ?: false
         )
 }
diff --git a/test_runner/src/main/kotlin/ftl/args/CommonArgs.kt b/test_runner/src/main/kotlin/ftl/args/CommonArgs.kt
@@ -37,5 +37,5 @@ data class CommonArgs(
     override val disableResultsUpload: Boolean,
     override val defaultTestTime: Double,
     override val defaultClassTestTime: Double,
-    override val useAverageTestTimeForNewTests: Boolean,
+    override val useAverageTestTimeForNewTests: Boolean
 ) : IArgs
diff --git a/test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/CreateAndroidArgs.kt
@@ -9,7 +9,8 @@ fun createAndroidArgs(
     config: AndroidConfig? = null,
     gcloud: AndroidGcloudConfig = config!!.platform.gcloud,
     flank: AndroidFlankConfig = config!!.platform.flank,
-    commonArgs: CommonArgs = config!!.prepareAndroidCommonConfig()
+    commonArgs: CommonArgs = config!!.prepareAndroidCommonConfig(),
+    obfuscate: Boolean = false
 ) = AndroidArgs(
     commonArgs = commonArgs,
     // gcloud
@@ -37,5 +38,6 @@ fun createAndroidArgs(
         )
     } ?: emptyList(),
     useLegacyJUnitResult = flank.useLegacyJUnitResult!!,
+    obfuscateDumpShards = obfuscate,
     grantPermissions = gcloud.grantPermissions
 )
diff --git a/test_runner/src/main/kotlin/ftl/args/CreateIosArgs.kt b/test_runner/src/main/kotlin/ftl/args/CreateIosArgs.kt
@@ -5,23 +5,27 @@ import ftl.config.ios.IosFlankConfig
 import ftl.config.ios.IosGcloudConfig
 
 fun createIosArgs(
-    config: IosConfig
+    config: IosConfig,
+    obfuscate: Boolean = false
 ) = createIosArgs(
     gcloud = config.platform.gcloud,
     flank = config.platform.flank,
-    commonArgs = config.common.createCommonArgs(config.data)
+    commonArgs = config.common.createCommonArgs(config.data),
+    obfuscate = obfuscate
 )
 
 private fun createIosArgs(
     gcloud: IosGcloudConfig,
     flank: IosFlankConfig,
-    commonArgs: CommonArgs
+    commonArgs: CommonArgs,
+    obfuscate: Boolean = false
 ) = IosArgs(
     commonArgs = commonArgs.copy(maxTestShards = convertToShardCount(commonArgs.maxTestShards)),
     xctestrunZip = gcloud.test?.normalizeFilePath().orEmpty(),
     xctestrunFile = gcloud.xctestrunFile?.normalizeFilePath().orEmpty(),
     xcodeVersion = gcloud.xcodeVersion,
-    testTargets = flank.testTargets?.filterNotNull().orEmpty()
+    testTargets = flank.testTargets?.filterNotNull().orEmpty(),
+    obfuscateDumpShards = obfuscate
 )
 
 private fun convertToShardCount(inputValue: Int) =

diff --git a/test_runner/src/main/kotlin/ftl/args/IosArgs.kt b/test_runner/src/main/kotlin/ftl/args/IosArgs.kt
@@ -11,7 +11,8 @@ data class IosArgs(
     val xctestrunZip: String,
     val xctestrunFile: String,
     val xcodeVersion: String?,
-    val testTargets: List<String>
+    val testTargets: List<String>,
+    val obfuscateDumpShards: Boolean
 ) : IArgs by commonArgs {
 
     override val useLegacyJUnitResult = true

diff --git a/test_runner/src/main/kotlin/ftl/args/IosArgsCompanion.kt b/test_runner/src/main/kotlin/ftl/args/IosArgsCompanion.kt
@@ -38,7 +38,8 @@ open class IosArgsCompanion : IArgs.ICompanion {
     internal fun load(yamlReader: Reader, cli: IosRunCommand? = null) =
         createIosArgs(
             config = defaultIosConfig() +
-                    loadIosConfig(reader = yamlReader) +
-                    cli?.config
+                loadIosConfig(reader = yamlReader) +
+                cli?.config,
+            obfuscate = cli?.obfuscate ?: false
         )
 }
diff --git a/test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidRunCommand.kt b/test_runner/src/main/kotlin/ftl/cli/firebase/test/android/AndroidRunCommand.kt
@@ -46,7 +46,7 @@ class AndroidRunCommand : CommonRunCommand(), Runnable {
 
         val config = AndroidArgs.load(Paths.get(configPath), cli = this).validate()
         runBlocking {
-            if (dumpShards) dumpShards(args = config, obfuscatedOutput = obfuscate)
+            if (dumpShards) dumpShards(args = config)
             else newTestRun(config)
         }
     }

diff --git a/test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosRunCommand.kt b/test_runner/src/main/kotlin/ftl/cli/firebase/test/ios/IosRunCommand.kt
@@ -47,7 +47,7 @@ class IosRunCommand : CommonRunCommand(), Runnable {
         val config = IosArgs.load(Paths.get(configPath), cli = this).validate()
 
         if (dumpShards) {
-            dumpShards(args = config, obfuscatedOutput = obfuscate)
+            dumpShards(args = config)
         } else runBlocking {
             newTestRun(config)
         }

diff --git a/test_runner/src/main/kotlin/ftl/run/DumpShards.kt b/test_runner/src/main/kotlin/ftl/run/DumpShards.kt
@@ -15,7 +15,6 @@ import java.nio.file.Paths
 suspend fun dumpShards(
     args: AndroidArgs,
     shardFilePath: String = ANDROID_SHARD_FILE,
-    obfuscatedOutput: Boolean = false
 ) {
     if (!args.isInstrumentationTest) throw FlankConfigurationError(
         "Cannot dump shards for non instrumentation test, ensure test apk has been set."
@@ -25,24 +24,23 @@ suspend fun dumpShards(
         shardFilePath = shardFilePath,
         shards = shards,
         size = shards.size,
-        obfuscatedOutput = obfuscatedOutput
+        obfuscatedOutput = args.obfuscateDumpShards
     )
 }
 
 fun dumpShards(
     args: IosArgs,
     shardFilePath: String = IOS_SHARD_FILE,
-    obfuscatedOutput: Boolean = false
 ) {
     saveShardChunks(
         shardFilePath = shardFilePath,
         shards = args.testShardChunks.testCases,
         size = args.testShardChunks.size,
-        obfuscatedOutput = obfuscatedOutput
+        obfuscatedOutput = args.obfuscateDumpShards
     )
 }
 
-private fun saveShardChunks(
+fun saveShardChunks(
     shardFilePath: String,
     shards: Any,
     size: Int,

diff --git a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt
@@ -3,10 +3,13 @@ package ftl.run.platform
 import com.google.api.services.testing.Testing
 import com.google.api.services.testing.model.TestMatrix
 import ftl.args.AndroidArgs
+import ftl.args.isInstrumentationTest
 import ftl.gc.GcAndroidDevice
 import ftl.gc.GcAndroidTestMatrix
+import ftl.gc.GcStorage
 import ftl.gc.GcToolResults
 import ftl.http.executeWithRetry
+import ftl.run.ANDROID_SHARD_FILE
 import ftl.run.model.InstrumentationTestContext
 import ftl.run.model.TestResult
 import ftl.run.platform.android.createAndroidTestConfig
@@ -18,6 +21,10 @@ import ftl.run.platform.common.afterRunTests
 import ftl.run.platform.common.beforeRunMessage
 import ftl.run.platform.common.beforeRunTests
 import ftl.run.exception.FlankGeneralError
+import ftl.run.model.AndroidMatrixTestShards
+import ftl.run.model.AndroidTestContext
+import ftl.run.platform.android.asMatrixTestShards
+import ftl.run.saveShardChunks
 import ftl.shard.Chunk
 import ftl.shard.testCases
 import kotlinx.coroutines.Deferred
@@ -40,8 +47,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
     val otherGcsFiles = args.uploadOtherFiles(runGcsPath)
     val additionalApks = args.uploadAdditionalApks(runGcsPath)
 
-    args.createAndroidTestContexts()
-        .upload(args.resultsBucket, runGcsPath)
+    args.createAndroidTestContexts().dumpShards(args).upload(args.resultsBucket, runGcsPath)
         .forEachIndexed { index, context ->
             if (context is InstrumentationTestContext) {
                 ignoredTestsShardChunks += context.ignoredTestCases
@@ -64,13 +70,26 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
     if (testMatrices.isEmpty()) throw FlankGeneralError("There are no tests to run.")
 
     println(beforeRunMessage(args, allTestShardChunks))
+
     TestResult(
         matrixMap = afterRunTests(testMatrices.awaitAll(), runGcsPath, stopwatch, args),
         shardChunks = allTestShardChunks.testCases,
         ignoredTests = ignoredTestsShardChunks.flatten()
     )
 }
 
+private fun List<AndroidTestContext>.dumpShards(config: AndroidArgs) = takeIf { config.isInstrumentationTest }?.apply {
+    filterIsInstance<InstrumentationTestContext>().asMatrixTestShards().saveShards(config.obfuscateDumpShards)
+    if (config.disableResultsUpload.not()) GcStorage.upload(ANDROID_SHARD_FILE, config.resultsBucket, config.resultsDir)
+} ?: this
+
+private fun AndroidMatrixTestShards.saveShards(obfuscateOutput: Boolean) = saveShardChunks(
+    shardFilePath = ANDROID_SHARD_FILE,
+    shards = this,
+    size = size,
+    obfuscatedOutput = obfuscateOutput
+)
+
 private suspend fun executeAndroidTestMatrix(
     runCount: Int,
     createTestMatrix: () -> Testing.Projects.TestMatrices.Create

diff --git a/test_runner/src/main/kotlin/ftl/run/platform/RunIosTests.kt b/test_runner/src/main/kotlin/ftl/run/platform/RunIosTests.kt
@@ -9,6 +9,8 @@ import ftl.gc.GcStorage
 import ftl.gc.GcToolResults
 import ftl.http.executeWithRetry
 import ftl.ios.Xctestrun
+import ftl.run.IOS_SHARD_FILE
+import ftl.run.dumpShards
 import ftl.run.model.TestResult
 import ftl.run.platform.common.afterRunTests
 import ftl.run.platform.common.beforeRunMessage
@@ -36,6 +38,10 @@ internal suspend fun runIosTests(iosArgs: IosArgs): TestResult = coroutineScope
     val shardCounter = ShardCounter()
     val history = GcToolResults.createToolResultsHistory(iosArgs)
 
+    dumpShards(iosArgs)
+    if (iosArgs.disableResultsUpload.not())
+        GcStorage.upload(IOS_SHARD_FILE, iosArgs.resultsBucket, iosArgs.resultsDir)
+
     // Upload only after parsing shards to detect missing methods early.
     val xcTestGcsPath = getXcTestGcPath(iosArgs, runGcsPath)
 

diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidMatrixShards.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidMatrixShards.kt
@@ -11,7 +11,7 @@ suspend fun AndroidArgs.getAndroidMatrixShards(): AndroidMatrixTestShards = this
     .filterIsInstance<InstrumentationTestContext>()
     .asMatrixTestShards()
 
-private fun List<InstrumentationTestContext>.asMatrixTestShards(): AndroidMatrixTestShards =
+fun List<InstrumentationTestContext>.asMatrixTestShards(): AndroidMatrixTestShards =
     map { testApks ->
         AndroidTestShards(
             app = testApks.app.local,

diff --git a/test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt b/test_runner/src/test/kotlin/ftl/cli/firebase/test/android/AndroidRunCommandTest.kt
@@ -1,11 +1,20 @@
 package ftl.cli.firebase.test.android
 
 import com.google.common.truth.Truth.assertThat
+import ftl.args.AndroidArgs
 import ftl.args.yml.AppTestPair
 import ftl.config.Device
 import ftl.config.FtlConstants
+import ftl.gc.GcStorage
+import ftl.run.ANDROID_SHARD_FILE
 import ftl.run.exception.FlankConfigurationError
+import ftl.run.platform.android.createAndroidTestContexts
+import ftl.run.saveShardChunks
 import ftl.test.util.FlankTestRunner
+import io.mockk.coVerify
+import io.mockk.mockkObject
+import io.mockk.mockkStatic
+import io.mockk.verify
 import org.junit.Assert.assertEquals
 import org.junit.Rule
 import org.junit.Test
@@ -523,4 +532,37 @@ class AndroidRunCommandTest {
         )
         cmd.run()
     }
+
+    @Test
+    fun `should dump shards on android test run`() {
+        mockkStatic("ftl.run.DumpShardsKt")
+        val runCmd = AndroidRunCommand()
+        runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-android-flank.yml"
+        runCmd.run()
+        verify { saveShardChunks(any(), any(), any(), any()) }
+    }
+
+    @Test
+    fun `should dump shards on android test run and not upload when disable-upload-results set`() {
+        mockkStatic("ftl.run.DumpShardsKt")
+        mockkObject(GcStorage) {
+            val runCmd = AndroidRunCommand()
+            runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-android-flank.yml"
+            CommandLine(runCmd).parseArgs("--disable-results-upload")
+            runCmd.run()
+            verify { saveShardChunks(any(), any(), any(), any()) }
+            verify(inverse = true) { GcStorage.upload(ANDROID_SHARD_FILE, any(), any()) }
+        }
+    }
+
+    @Test
+    fun `should calculate shards only one time on newRun`() {
+        mockkStatic("ftl.run.DumpShardsKt", "ftl.run.platform.android.CreateAndroidTestContextKt")
+        mockkObject(GcStorage) {
+            val runCmd = AndroidRunCommand()
+            runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-android-flank.yml"
+            runCmd.run()
+            coVerify(exactly = 1) { any<AndroidArgs>().createAndroidTestContexts() }
+        }
+    }
 }
diff --git a/test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosRunCommandTest.kt b/test_runner/src/test/kotlin/ftl/cli/firebase/test/ios/IosRunCommandTest.kt
@@ -1,10 +1,17 @@
 package ftl.cli.firebase.test.ios
 
 import com.google.common.truth.Truth.assertThat
+import ftl.args.IosArgs
 import ftl.config.Device
 import ftl.config.FtlConstants
 import ftl.config.FtlConstants.isWindows
+import ftl.gc.GcStorage
+import ftl.run.IOS_SHARD_FILE
+import ftl.run.dumpShards
 import ftl.test.util.FlankTestRunner
+import io.mockk.mockkObject
+import io.mockk.mockkStatic
+import io.mockk.verify
 import org.junit.Assume.assumeFalse
 import org.junit.Rule
 import org.junit.Test
@@ -337,4 +344,26 @@ class IosRunCommandTest {
 
         assertThat(cmd.config.common.flank.useAverageTestTimeForNewTests).isTrue()
     }
+
+    @Test
+    fun `should dump shards on ios test run`() {
+        mockkStatic("ftl.run.DumpShardsKt")
+        val runCmd = IosRunCommand()
+        runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-ios-flank.yml"
+        runCmd.run()
+        verify { dumpShards(any<IosArgs>(), any()) }
+    }
+
+    @Test
+    fun `should dump shards on ios test run and not upload when disable-upload-results set`() {
+        mockkStatic("ftl.run.DumpShardsKt")
+        mockkObject(GcStorage) {
+            val runCmd = IosRunCommand()
+            runCmd.configPath = "./src/test/kotlin/ftl/fixtures/simple-ios-flank.yml"
+            CommandLine(runCmd).parseArgs("--disable-results-upload")
+            runCmd.run()
+            verify { dumpShards(any<IosArgs>(), any()) }
+            verify(inverse = true) { GcStorage.upload(IOS_SHARD_FILE, any(), any()) }
+        }
+    }
 }
diff --git a/test_runner/src/test/kotlin/ftl/run/DumpShardsKtTest.kt b/test_runner/src/test/kotlin/ftl/run/DumpShardsKtTest.kt
@@ -3,6 +3,8 @@ package ftl.run
 import com.google.common.truth.Truth.assertThat
 import ftl.args.AndroidArgs
 import ftl.args.IosArgs
+import ftl.cli.firebase.test.android.AndroidRunCommand
+import ftl.cli.firebase.test.ios.IosRunCommand
 import ftl.config.FtlConstants
 import ftl.doctor.assertEqualsIgnoreNewlineStyle
 import ftl.test.util.FlankTestRunner
@@ -123,7 +125,7 @@ class DumpShardsKtTest {
 
         // when
         val actual = runBlocking {
-            dumpShards(AndroidArgs.load(mixedConfigYaml), TEST_SHARD_FILE, true)
+            dumpShards(AndroidArgs.load(mixedConfigYaml, AndroidRunCommand().apply { obfuscate = true }), TEST_SHARD_FILE)
             File(TEST_SHARD_FILE).apply { deleteOnExit() }.readText()
         }
 
@@ -183,7 +185,7 @@ class DumpShardsKtTest {
         if (FtlConstants.isWindows) return // TODO Windows Linux subsytem does not contain all expected commands
         // when
         val actual = runBlocking {
-            dumpShards(IosArgs.load(ios2ConfigYaml), TEST_SHARD_FILE, true)
+            dumpShards(IosArgs.load(ios2ConfigYaml, IosRunCommand().apply { obfuscate = true }), TEST_SHARD_FILE)
             File(TEST_SHARD_FILE).apply { deleteOnExit() }.readText()
         }