From cfa8c9d36b2c0b617743c64dea46066dde7c5e59 Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Sat, 16 May 2020 10:15:33 -0700 Subject: [PATCH 01/11] Dump shards for all test apks --- .../main/kotlin/ftl/args/AndroidTestShard.kt | 21 +++++++++++-------- .../test/android/AndroidRunCommand.kt | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt index 0c08a96605..1e560a2a5f 100644 --- a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt +++ b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt @@ -11,20 +11,23 @@ import java.io.File object AndroidTestShard { - // computed properties not specified in yaml - fun getTestShardChunks(args: AndroidArgs, testApk: String): ShardChunks { + // Output the test shards from all the local test apks (including addtional test apks) + fun getTestShardChunks(args: AndroidArgs): ShardChunks { // Download test APK if necessary so it can be used to validate test methods - val testLocalApk = if (testApk.startsWith(FtlConstants.GCS_PREFIX)) - GcStorage.download(testApk) else - testApk + val testApks = mutableListOf() + val mainTestApk = args.testApk + if (mainTestApk != null) testApks.add(mainTestApk) + testApks.addAll(args.additionalAppTestApks.map { it.test }) - val filteredTests = getTestMethods(args, testLocalApk) + val filteredTests = testApks.map { getTestMethods(args, it) }.flatten() - if (filteredTests.isEmpty()) println("${FtlConstants.indent}No tests for ${testLocalApk.apkFileName}") + if (filteredTests.isEmpty()) println("${FtlConstants.indent}No tests for ${testApks.joinToString(", ")}") - return if (args.numUniformShards == null) - ArgsHelper.calculateShards(filteredTests, args) else + return if (args.numUniformShards == null) { + ArgsHelper.calculateShards(filteredTests, args) + } else { listOf(filteredTests.map(FlankTestMethod::testName)) + } } private fun getTestMethods(args: AndroidArgs, testLocalApk: String): List { 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 index 05b3001d7e..32b1ff14c7 100644 --- 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 @@ -45,7 +45,7 @@ class AndroidRunCommand : CommonRunCommand(), Runnable { val config = AndroidArgs.load(Paths.get(configPath), cli = this) if (dumpShards) { - val testShardChunks: ShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks: ShardChunks = AndroidTestShard.getTestShardChunks(config) val testShardChunksJson: String = prettyPrint.toJson(testShardChunks) Files.write(Paths.get(shardFile), testShardChunksJson.toByteArray()) From d6dc2ef1e78368be6d642c579adc0912c3ce501c Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Sat, 16 May 2020 10:25:49 -0700 Subject: [PATCH 02/11] Dump shards for all test apks --- .../main/kotlin/ftl/args/AndroidTestShard.kt | 18 ++++++++++++++++-- .../firebase/test/android/AndroidRunCommand.kt | 4 ++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt index 1e560a2a5f..7377709991 100644 --- a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt +++ b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt @@ -11,9 +11,23 @@ import java.io.File object AndroidTestShard { - // Output the test shards from all the local test apks (including addtional test apks) - fun getTestShardChunks(args: AndroidArgs): ShardChunks { + // computed properties not specified in yaml + fun getTestShardChunks(args: AndroidArgs, testApk: String): ShardChunks { // Download test APK if necessary so it can be used to validate test methods + val testLocalApk = if (testApk.startsWith(FtlConstants.GCS_PREFIX)) + GcStorage.download(testApk) else + testApk + + val filteredTests = getTestMethods(args, testLocalApk) + + if (filteredTests.isEmpty()) println("${FtlConstants.indent}No tests for ${testLocalApk.apkFileName}") + + return if (args.numUniformShards == null) + ArgsHelper.calculateShards(filteredTests, args) else + listOf(filteredTests.map(FlankTestMethod::testName)) + } + + fun getAllLocalTestShardChunks(args: AndroidArgs): ShardChunks { val testApks = mutableListOf() val mainTestApk = args.testApk if (mainTestApk != null) testApks.add(mainTestApk) 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 index 32b1ff14c7..04a71a83e4 100644 --- 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 @@ -45,11 +45,11 @@ class AndroidRunCommand : CommonRunCommand(), Runnable { val config = AndroidArgs.load(Paths.get(configPath), cli = this) if (dumpShards) { - val testShardChunks: ShardChunks = AndroidTestShard.getTestShardChunks(config) + val testShardChunks: ShardChunks = AndroidTestShard.getAllLocalTestShardChunks(config) val testShardChunksJson: String = prettyPrint.toJson(testShardChunks) Files.write(Paths.get(shardFile), testShardChunksJson.toByteArray()) - println("Saved shards to $shardFile") + println("Saved ${testShardChunks.size} shards to $shardFile") } else { runBlocking { newTestRun(config) From d2e8881194e5cde20572e83759ab1c95825b170a Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Mon, 18 May 2020 22:15:34 -0700 Subject: [PATCH 03/11] Refactor getTestShardChunks --- .../main/kotlin/ftl/args/AndroidTestShard.kt | 43 ++++++++------- .../test/android/AndroidRunCommand.kt | 2 +- .../ftl/run/platform/RunAndroidTests.kt | 2 +- .../kotlin/ftl/args/AndroidArgsFileTest.kt | 53 ++++++++++++++++--- .../test/kotlin/ftl/args/AndroidArgsTest.kt | 10 ++-- 5 files changed, 75 insertions(+), 35 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt index 7377709991..dc9d0eb811 100644 --- a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt +++ b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt @@ -10,32 +10,31 @@ import ftl.util.FlankTestMethod import java.io.File object AndroidTestShard { + private fun getTestApks(args: AndroidArgs, origTestApks: List): List { + val testApks = if (origTestApks.isNotEmpty()) { + origTestApks + } else { + val allTestApks = mutableListOf() + val mainTestApk = args.testApk + if (mainTestApk != null) allTestApks.add(mainTestApk) + allTestApks.addAll(args.additionalAppTestApks.map { it.test }) + allTestApks + } - // computed properties not specified in yaml - fun getTestShardChunks(args: AndroidArgs, testApk: String): ShardChunks { - // Download test APK if necessary so it can be used to validate test methods - val testLocalApk = if (testApk.startsWith(FtlConstants.GCS_PREFIX)) - GcStorage.download(testApk) else - testApk - - val filteredTests = getTestMethods(args, testLocalApk) - - if (filteredTests.isEmpty()) println("${FtlConstants.indent}No tests for ${testLocalApk.apkFileName}") - - return if (args.numUniformShards == null) - ArgsHelper.calculateShards(filteredTests, args) else - listOf(filteredTests.map(FlankTestMethod::testName)) + return testApks.map { testApk -> + if (testApk.startsWith(FtlConstants.GCS_PREFIX)) GcStorage.download(testApk) else + testApk + } } - fun getAllLocalTestShardChunks(args: AndroidArgs): ShardChunks { - val testApks = mutableListOf() - val mainTestApk = args.testApk - if (mainTestApk != null) testApks.add(mainTestApk) - testApks.addAll(args.additionalAppTestApks.map { it.test }) - - val filteredTests = testApks.map { getTestMethods(args, it) }.flatten() + fun getTestShardChunks(args: AndroidArgs, testApks: List = listOf()): ShardChunks { + val resolvedApks = getTestApks(args, testApks) + val filteredTests = resolvedApks.map { getTestMethods(args, it) }.flatten() - if (filteredTests.isEmpty()) println("${FtlConstants.indent}No tests for ${testApks.joinToString(", ")}") + if (filteredTests.isEmpty()) { + println("${FtlConstants.indent}No tests for ${testApks.joinToString(", ")}") + return emptyList() + } return if (args.numUniformShards == null) { ArgsHelper.calculateShards(filteredTests, args) 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 index 04a71a83e4..8376bde9c5 100644 --- 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 @@ -45,7 +45,7 @@ class AndroidRunCommand : CommonRunCommand(), Runnable { val config = AndroidArgs.load(Paths.get(configPath), cli = this) if (dumpShards) { - val testShardChunks: ShardChunks = AndroidTestShard.getAllLocalTestShardChunks(config) + val testShardChunks: ShardChunks = AndroidTestShard.getTestShardChunks(config) val testShardChunksJson: String = prettyPrint.toJson(testShardChunks) Files.write(Paths.get(shardFile), testShardChunksJson.toByteArray()) diff --git a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt index 4af82e4302..bf2f8ffea8 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt @@ -40,7 +40,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS args.resolveApks().forEachIndexed { index: Int, apks: ResolvedApks -> val testShards = apks.test?.let { test -> - AndroidTestShard.getTestShardChunks(args, test) + AndroidTestShard.getTestShardChunks(args, listOf(test)) } // We can't return if testShards is null since it can be a robo test. testShards?.let { diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt index 56d826dc46..1e70a6107e 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt @@ -1,8 +1,10 @@ package ftl.args import ftl.args.yml.AndroidFlankYml +import ftl.args.yml.AndroidFlankYmlParams import ftl.args.yml.AndroidGcloudYml import ftl.args.yml.AndroidGcloudYmlParams +import ftl.args.yml.AppTestPair import ftl.args.yml.FlankYml import ftl.args.yml.FlankYmlParams import ftl.args.yml.GcloudYml @@ -108,21 +110,60 @@ class AndroidArgsFileTest { ) } + @Test + fun `calculateShards additionalAppTestApks`() { + val test1 = "src/test/kotlin/ftl/fixtures/tmp/apk/app-debug-androidTest_1.apk" + val test155 = "src/test/kotlin/ftl/fixtures/tmp/apk/app-debug-androidTest_155.apk" + val config = AndroidArgs( + GcloudYml(GcloudYmlParams()), + AndroidGcloudYml( + AndroidGcloudYmlParams( + app = appApkLocal, + test = getString(test1) + ) + ), + FlankYml( + FlankYmlParams( + maxTestShards = 3 + ) + ), + AndroidFlankYml( + AndroidFlankYmlParams( + additionalAppTestApks = listOf( + AppTestPair( + app = appApkLocal, + test = getString(test155) + ) + ) + ) + ), + "" + ) + + val testShardChunks = AndroidTestShard.getTestShardChunks(config) + with(config) { + assert(maxTestShards, 3) + assert(testShardChunks.size, 3) + assert(testShardChunks[0].size, 52) + assert(testShardChunks[1].size, 52) + assert(testShardChunks[2].size, 52) + } + } + @Test fun `calculateShards 0`() { val config = configWithTestMethods(0) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk ?: "")) with(config) { assert(maxTestShards, 1) - assert(testShardChunks.size, 1) - assert(testShardChunks.first().size, 0) + assert(testShardChunks.size, 0) } } @Test fun `calculateShards 1`() { val config = configWithTestMethods(1) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk ?: "")) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 1) @@ -133,7 +174,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 155`() { val config = configWithTestMethods(155) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk ?: "")) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 1) @@ -144,7 +185,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 155 40`() { val config = configWithTestMethods(155, maxTestShards = 40) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk ?: "")) with(config) { assert(maxTestShards, 40) assert(testShardChunks.size, 40) diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt index b495de22cb..1de11c6971 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt @@ -447,7 +447,7 @@ AndroidArgs """ ) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) + val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, listOf(androidArgs.testApk ?: "")) with(androidArgs) { assert(maxTestShards, -1) assert(testShardChunks.size, 2) @@ -481,8 +481,8 @@ AndroidArgs disable-sharding: true """ val androidArgs = AndroidArgs.load(yaml) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) - assertThat(testShardChunks).hasSize(1) + val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, listOf(androidArgs.testApk ?: "")) + assertThat(testShardChunks).hasSize(0) } @Test @@ -493,8 +493,8 @@ AndroidArgs test: $invalidApk """ val androidArgs = AndroidArgs.load(yaml) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) - assertThat(testShardChunks).hasSize(1) + val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, listOf(androidArgs.testApk ?: "")) + assertThat(testShardChunks).hasSize(0) } @Test From b7abfe30c22c4842ea8e6f128edeaaeac03c76e2 Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Mon, 18 May 2020 22:22:42 -0700 Subject: [PATCH 04/11] Update release_notes.md --- release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/release_notes.md b/release_notes.md index 539f93bb3a..03d23e5eb2 100644 --- a/release_notes.md +++ b/release_notes.md @@ -12,6 +12,7 @@ - [#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)) +- [#794](https://github.com/Flank/flank/pull/794) Enhance `--dump-shards` to dump shards from all test apks ([bootstraponline](https://github.com/bootstraponline)) ## v20.05.1 From 5a93d6c868be7d576698e80471cbf9a5e1787fc0 Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Mon, 18 May 2020 22:43:00 -0700 Subject: [PATCH 05/11] Fix lint --- test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt index dc9d0eb811..fcc4ce0c03 100644 --- a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt +++ b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt @@ -32,7 +32,8 @@ object AndroidTestShard { val filteredTests = resolvedApks.map { getTestMethods(args, it) }.flatten() if (filteredTests.isEmpty()) { - println("${FtlConstants.indent}No tests for ${testApks.joinToString(", ")}") + val testApkNames = testApks.joinToString(", ") { it.apkFileName } + println("${FtlConstants.indent}No tests for $testApkNames") return emptyList() } From 9aa7f52a3cfe3a5d5883ae6b46a950b836647638 Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Tue, 19 May 2020 08:03:32 -0700 Subject: [PATCH 06/11] Remove ?: "" May replace !! with .orEmpty() in the future --- .../src/test/kotlin/ftl/args/AndroidArgsFileTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt index 1e70a6107e..9532065d92 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt @@ -153,7 +153,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 0`() { val config = configWithTestMethods(0) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk ?: "")) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk!!)) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 0) @@ -163,7 +163,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 1`() { val config = configWithTestMethods(1) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk ?: "")) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk!!)) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 1) @@ -174,7 +174,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 155`() { val config = configWithTestMethods(155) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk ?: "")) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk!!)) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 1) @@ -185,7 +185,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 155 40`() { val config = configWithTestMethods(155, maxTestShards = 40) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk ?: "")) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk!!)) with(config) { assert(maxTestShards, 40) assert(testShardChunks.size, 40) From 6a6d46454c30a921660c60787938d3cdd524fd76 Mon Sep 17 00:00:00 2001 From: bootstraponline Date: Tue, 19 May 2020 08:14:45 -0700 Subject: [PATCH 07/11] Clean up code --- test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt | 2 ++ .../src/main/kotlin/ftl/run/platform/RunAndroidTests.kt | 2 +- .../src/test/kotlin/ftl/args/AndroidArgsFileTest.kt | 8 ++++---- test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt | 8 ++++---- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt index fcc4ce0c03..810b723d9c 100644 --- a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt +++ b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt @@ -44,6 +44,8 @@ object AndroidTestShard { } } + fun getTestShardChunks(args: AndroidArgs, testApk: String) = getTestShardChunks(args, listOf(testApk)) + private fun getTestMethods(args: AndroidArgs, testLocalApk: String): List { val allTestMethods = DexParser.findTestMethods(testLocalApk) if (allTestMethods.isEmpty()) { diff --git a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt index bf2f8ffea8..4af82e4302 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt @@ -40,7 +40,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS args.resolveApks().forEachIndexed { index: Int, apks: ResolvedApks -> val testShards = apks.test?.let { test -> - AndroidTestShard.getTestShardChunks(args, listOf(test)) + AndroidTestShard.getTestShardChunks(args, test) } // We can't return if testShards is null since it can be a robo test. testShards?.let { diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt index 9532065d92..6e592e9c1b 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt @@ -153,7 +153,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 0`() { val config = configWithTestMethods(0) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk!!)) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 0) @@ -163,7 +163,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 1`() { val config = configWithTestMethods(1) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk!!)) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 1) @@ -174,7 +174,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 155`() { val config = configWithTestMethods(155) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk!!)) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 1) @@ -185,7 +185,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 155 40`() { val config = configWithTestMethods(155, maxTestShards = 40) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, listOf(config.testApk!!)) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) with(config) { assert(maxTestShards, 40) assert(testShardChunks.size, 40) diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt index 1de11c6971..ffb8585593 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt @@ -447,7 +447,7 @@ AndroidArgs """ ) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, listOf(androidArgs.testApk ?: "")) + val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) with(androidArgs) { assert(maxTestShards, -1) assert(testShardChunks.size, 2) @@ -481,7 +481,7 @@ AndroidArgs disable-sharding: true """ val androidArgs = AndroidArgs.load(yaml) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, listOf(androidArgs.testApk ?: "")) + val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) assertThat(testShardChunks).hasSize(0) } @@ -493,7 +493,7 @@ AndroidArgs test: $invalidApk """ val androidArgs = AndroidArgs.load(yaml) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, listOf(androidArgs.testApk ?: "")) + val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) assertThat(testShardChunks).hasSize(0) } @@ -1275,7 +1275,7 @@ AndroidArgs """.trimIndent() mockkObject(AndroidTestShard) - every { AndroidTestShard.getTestShardChunks(any(), any()) } returns listOf() + every { AndroidTestShard.getTestShardChunks(any(), any()) } returns listOf() val parsedYml = AndroidArgs.load(yaml) runBlocking { runAndroidTests(parsedYml) } From e673d41e70e08a0c812467acd60edc4b49c686ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janek=20G=C3=B3ral?= Date: Fri, 22 May 2020 21:15:58 +0200 Subject: [PATCH 08/11] Implement new output format for android_shards.json --- .../main/kotlin/ftl/args/AndroidTestShard.kt | 70 ------------------- .../yml/errors/ConfigurationErrorParser.kt | 2 +- .../test/android/AndroidRunCommand.kt | 24 ++----- .../cli/firebase/test/ios/IosRunCommand.kt | 20 ++---- .../src/main/kotlin/ftl/run/DumpShards.kt | 45 ++++++++++++ .../ftl/run/model/AndroidMatrixTestShards.kt | 3 + .../kotlin/ftl/run/model/AndroidTestShards.kt | 7 ++ .../ftl/run/model/InstrumentationTestApk.kt | 8 +++ .../ftl/run/platform/RunAndroidTests.kt | 4 +- .../android/GetAndroidMatrixShards.kt | 44 ++++++++++++ .../platform/android/GetAndroidShardChunks.kt | 15 ++++ .../android/GetInstrumentationShardChunks.kt | 54 ++++++++++++++ .../src/main/kotlin/ftl/util/FileReference.kt | 20 ++++++ .../kotlin/ftl/args/AndroidArgsFileTest.kt | 23 +++--- .../test/kotlin/ftl/args/AndroidArgsTest.kt | 13 ++-- 15 files changed, 229 insertions(+), 123 deletions(-) delete mode 100644 test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt create mode 100644 test_runner/src/main/kotlin/ftl/run/DumpShards.kt create mode 100644 test_runner/src/main/kotlin/ftl/run/model/AndroidMatrixTestShards.kt create mode 100644 test_runner/src/main/kotlin/ftl/run/model/AndroidTestShards.kt create mode 100644 test_runner/src/main/kotlin/ftl/run/model/InstrumentationTestApk.kt create mode 100644 test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidMatrixShards.kt create mode 100644 test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidShardChunks.kt create mode 100644 test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt create mode 100644 test_runner/src/main/kotlin/ftl/util/FileReference.kt diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt deleted file mode 100644 index 810b723d9c..0000000000 --- a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt +++ /dev/null @@ -1,70 +0,0 @@ -package ftl.args - -import com.linkedin.dex.parser.DexParser -import com.linkedin.dex.parser.TestMethod -import ftl.config.FtlConstants -import ftl.filter.TestFilter -import ftl.filter.TestFilters -import ftl.gc.GcStorage -import ftl.util.FlankTestMethod -import java.io.File - -object AndroidTestShard { - private fun getTestApks(args: AndroidArgs, origTestApks: List): List { - val testApks = if (origTestApks.isNotEmpty()) { - origTestApks - } else { - val allTestApks = mutableListOf() - val mainTestApk = args.testApk - if (mainTestApk != null) allTestApks.add(mainTestApk) - allTestApks.addAll(args.additionalAppTestApks.map { it.test }) - allTestApks - } - - return testApks.map { testApk -> - if (testApk.startsWith(FtlConstants.GCS_PREFIX)) GcStorage.download(testApk) else - testApk - } - } - - fun getTestShardChunks(args: AndroidArgs, testApks: List = listOf()): ShardChunks { - val resolvedApks = getTestApks(args, testApks) - val filteredTests = resolvedApks.map { getTestMethods(args, it) }.flatten() - - if (filteredTests.isEmpty()) { - val testApkNames = testApks.joinToString(", ") { it.apkFileName } - println("${FtlConstants.indent}No tests for $testApkNames") - return emptyList() - } - - return if (args.numUniformShards == null) { - ArgsHelper.calculateShards(filteredTests, args) - } else { - listOf(filteredTests.map(FlankTestMethod::testName)) - } - } - - fun getTestShardChunks(args: AndroidArgs, testApk: String) = getTestShardChunks(args, listOf(testApk)) - - private fun getTestMethods(args: AndroidArgs, testLocalApk: String): List { - val allTestMethods = DexParser.findTestMethods(testLocalApk) - if (allTestMethods.isEmpty()) { - // Avoid unnecessary computation if we already know there aren't tests. - return emptyList() - } - val testFilter = TestFilters.fromTestTargets(args.testTargets) - return allTestMethods filterWith testFilter - } - - private infix fun List.filterWith(filter: TestFilter) = asSequence() - .distinct() - .filter(filter.shouldRun) - .map { FlankTestMethod("class ${it.testName}", it.isIgnored) } - .toList() -} - -private val TestMethod.isIgnored: Boolean - get() = annotations.map { it.name }.contains("org.junit.Ignore") - -private inline val String.apkFileName: String - get() = File(this).name diff --git a/test_runner/src/main/kotlin/ftl/args/yml/errors/ConfigurationErrorParser.kt b/test_runner/src/main/kotlin/ftl/args/yml/errors/ConfigurationErrorParser.kt index 1f037a1f9a..ff8eac3c61 100644 --- a/test_runner/src/main/kotlin/ftl/args/yml/errors/ConfigurationErrorParser.kt +++ b/test_runner/src/main/kotlin/ftl/args/yml/errors/ConfigurationErrorParser.kt @@ -5,7 +5,7 @@ internal object ConfigurationErrorParser { //region regex patterns private val propertyNameRegex = "(?<=property\\s)[a-z]*".toRegex() private val referenceChainRegex = "(?<=chain:\\s).*(?=[)])".toRegex() - private val referenceChainCleanUpRegex = "(?<=[\\[])\"?[\\w]*\"?(?=])".toRegex() + private val referenceChainCleanUpRegex = "(?<=[\\[])\"?(\\w|-)*\"?(?=])".toRegex() private val lineAndColumnRegex = "((?<=line:\\s)\\d*), column:\\s(\\d*)".toRegex() //endregion 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 index 8376bde9c5..c83c3b54fa 100644 --- 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 @@ -1,8 +1,6 @@ package ftl.cli.firebase.test.android import ftl.args.AndroidArgs -import ftl.args.AndroidTestShard -import ftl.args.ShardChunks import ftl.args.yml.AppTestPair import ftl.cli.firebase.test.CommonRunCommand import ftl.config.Device @@ -12,12 +10,12 @@ import ftl.config.FtlConstants.defaultAndroidVersion import ftl.config.FtlConstants.defaultLocale import ftl.config.FtlConstants.defaultOrientation import ftl.mock.MockServer -import ftl.run.common.prettyPrint +import ftl.run.IOS_SHARD_FILE +import ftl.run.dumpShards import ftl.run.newTestRun import kotlinx.coroutines.runBlocking import picocli.CommandLine.Command import picocli.CommandLine.Option -import java.nio.file.Files import java.nio.file.Paths @Command( @@ -45,25 +43,15 @@ class AndroidRunCommand : CommonRunCommand(), Runnable { val config = AndroidArgs.load(Paths.get(configPath), cli = this) if (dumpShards) { - val testShardChunks: ShardChunks = AndroidTestShard.getTestShardChunks(config) - val testShardChunksJson: String = prettyPrint.toJson(testShardChunks) - - Files.write(Paths.get(shardFile), testShardChunksJson.toByteArray()) - println("Saved ${testShardChunks.size} shards to $shardFile") - } else { - runBlocking { - newTestRun(config) - } + dumpShards(config) + } else runBlocking { + newTestRun(config) } } - companion object { - private const val shardFile = "android_shards.json" - } - // Flank debug - @Option(names = ["--dump-shards"], description = ["Dumps the shards to $shardFile for debugging"]) + @Option(names = ["--dump-shards"], description = ["Dumps the shards to $IOS_SHARD_FILE for debugging"]) var dumpShards: Boolean = false // Flank specific 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 index 29c40dbb91..c9f7de7315 100644 --- 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 @@ -7,12 +7,12 @@ import ftl.config.FtlConstants import ftl.config.FtlConstants.defaultIosModel import ftl.config.FtlConstants.defaultIosVersion import ftl.mock.MockServer -import ftl.run.common.prettyPrint +import ftl.run.ANDROID_SHARD_FILE +import ftl.run.dumpShards import ftl.run.newTestRun import kotlinx.coroutines.runBlocking import picocli.CommandLine.Command import picocli.CommandLine.Option -import java.nio.file.Files import java.nio.file.Paths @Command( @@ -39,23 +39,15 @@ class IosRunCommand : CommonRunCommand(), Runnable { val config = IosArgs.load(Paths.get(configPath), cli = this) if (dumpShards) { - val testShardChunksJson: String = prettyPrint.toJson(config.testShardChunks) - Files.write(Paths.get(shardFile), testShardChunksJson.toByteArray()) - println("Saved shards to $shardFile") - } else { - runBlocking { - newTestRun(config) - } + dumpShards(config) + } else runBlocking { + newTestRun(config) } } - companion object { - private const val shardFile = "ios_shards.json" - } - // Flank debug - @Option(names = ["--dump-shards"], description = ["Dumps the shards to $shardFile for debugging"]) + @Option(names = ["--dump-shards"], description = ["Dumps the shards to $ANDROID_SHARD_FILE for debugging"]) var dumpShards: Boolean = false // Flank specific diff --git a/test_runner/src/main/kotlin/ftl/run/DumpShards.kt b/test_runner/src/main/kotlin/ftl/run/DumpShards.kt new file mode 100644 index 0000000000..06a62c5593 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/run/DumpShards.kt @@ -0,0 +1,45 @@ +package ftl.run + +import ftl.args.AndroidArgs +import ftl.args.IosArgs +import ftl.run.common.prettyPrint +import ftl.run.model.AndroidMatrixTestShards +import ftl.run.platform.android.getAndroidMatrixShards +import ftl.util.FlankFatalError +import java.nio.file.Files +import java.nio.file.Paths + +fun dumpShards(args: AndroidArgs) { + if (!args.isInstrumentationTest) throw FlankFatalError( + "Cannot dump shards for non instrumentation test, ensure test apk has been set." + ) + val shards: AndroidMatrixTestShards = getAndroidMatrixShards(args) + saveShardChunks( + shardFilePath = ANDROID_SHARD_FILE, + shards = shards, + size = shards.size + ) +} + +fun dumpShards(args: IosArgs) { + saveShardChunks( + shardFilePath = IOS_SHARD_FILE, + shards = args.testShardChunks, + size = args.testShardChunks.size + ) +} + +private fun saveShardChunks( + shardFilePath: String, + shards: Any, + size: Int +) { + Files.write( + Paths.get(shardFilePath), + prettyPrint.toJson(shards).toByteArray() + ) + println("Saved $size shards to $shardFilePath") +} + +const val ANDROID_SHARD_FILE = "android_shards.json" +const val IOS_SHARD_FILE = "ios_shards.json" diff --git a/test_runner/src/main/kotlin/ftl/run/model/AndroidMatrixTestShards.kt b/test_runner/src/main/kotlin/ftl/run/model/AndroidMatrixTestShards.kt new file mode 100644 index 0000000000..e69edf4605 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/run/model/AndroidMatrixTestShards.kt @@ -0,0 +1,3 @@ +package ftl.run.model + +typealias AndroidMatrixTestShards = Map diff --git a/test_runner/src/main/kotlin/ftl/run/model/AndroidTestShards.kt b/test_runner/src/main/kotlin/ftl/run/model/AndroidTestShards.kt new file mode 100644 index 0000000000..2dc0ff1d21 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/run/model/AndroidTestShards.kt @@ -0,0 +1,7 @@ +package ftl.run.model + +data class AndroidTestShards( + val app: String, + val test: String, + val shards: Map> = emptyMap() +) diff --git a/test_runner/src/main/kotlin/ftl/run/model/InstrumentationTestApk.kt b/test_runner/src/main/kotlin/ftl/run/model/InstrumentationTestApk.kt new file mode 100644 index 0000000000..13966d94a8 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/run/model/InstrumentationTestApk.kt @@ -0,0 +1,8 @@ +package ftl.run.model + +import ftl.util.FileReference + +data class InstrumentationTestApk( + val app: FileReference = FileReference(), + val test: FileReference = FileReference() +) diff --git a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt index 4af82e4302..f9dabd7cce 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt @@ -3,8 +3,8 @@ 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.AndroidTestShard import ftl.args.ShardChunks +import ftl.run.platform.android.getAndroidShardChunks import ftl.args.yml.ResolvedApks import ftl.gc.GcAndroidDevice import ftl.gc.GcAndroidTestMatrix @@ -40,7 +40,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS args.resolveApks().forEachIndexed { index: Int, apks: ResolvedApks -> val testShards = apks.test?.let { test -> - AndroidTestShard.getTestShardChunks(args, test) + getAndroidShardChunks(args, test) } // We can't return if testShards is null since it can be a robo test. testShards?.let { 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 new file mode 100644 index 0000000000..e0898287ee --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidMatrixShards.kt @@ -0,0 +1,44 @@ +package ftl.run.platform.android + +import ftl.args.AndroidArgs +import ftl.args.ShardChunks +import ftl.run.model.AndroidMatrixTestShards +import ftl.run.model.AndroidTestShards +import ftl.run.model.InstrumentationTestApk +import ftl.util.asFileReference + +fun getAndroidMatrixShards( + args: AndroidArgs +): AndroidMatrixTestShards = + getInstrumentationShardChunks( + args = args, + testApks = args.createInstrumentationTestApks() + ).asMatrixTestShards() + +private fun AndroidArgs.createInstrumentationTestApks(): List = + listOfNotNull( + testApk?.let { testApk -> + InstrumentationTestApk( + app = appApk.asFileReference(), + test = testApk.asFileReference() + ) + } + ) + additionalAppTestApks.map { + InstrumentationTestApk( + app = (it.app ?: appApk).asFileReference(), + test = it.test.asFileReference() + ) + } + +private fun Map.asMatrixTestShards(): AndroidMatrixTestShards = + map { (testApks, shards: List>) -> + AndroidTestShards( + app = testApks.app.local, + test = testApks.test.local, + shards = shards.mapIndexed { index, testCases -> + "shard-$index" to testCases + }.toMap() + ) + }.mapIndexed { index, androidTestShards -> + "matrix-$index" to androidTestShards + }.toMap() diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidShardChunks.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidShardChunks.kt new file mode 100644 index 0000000000..709c1f51fb --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidShardChunks.kt @@ -0,0 +1,15 @@ +package ftl.run.platform.android + +import ftl.args.AndroidArgs +import ftl.args.ShardChunks +import ftl.run.model.InstrumentationTestApk +import ftl.util.asFileReference + +fun getAndroidShardChunks( + args: AndroidArgs, + testApk: String +): ShardChunks = + getInstrumentationShardChunks( + args = args, + testApks = listOf(InstrumentationTestApk(test = testApk.asFileReference())) + ).flatMap { (_, shardChunks) -> shardChunks } diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt new file mode 100644 index 0000000000..4ffac33d88 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt @@ -0,0 +1,54 @@ +package ftl.run.platform.android + +import com.linkedin.dex.parser.DexParser +import ftl.args.AndroidArgs +import ftl.args.ArgsHelper +import ftl.args.ShardChunks +import ftl.config.FtlConstants +import ftl.filter.TestFilter +import ftl.filter.TestFilters +import ftl.run.model.InstrumentationTestApk +import ftl.util.FlankTestMethod +import ftl.util.downloadIfNeeded +import java.io.File + +fun getInstrumentationShardChunks( + args: AndroidArgs, + testApks: List +): Map = + getFlankTestMethods( + testApks = testApks.download(), + testFilter = TestFilters.fromTestTargets(args.testTargets) + ).mapValues { (_, testMethods: List) -> + when { + testMethods.isEmpty() -> emptyList>().also { printNoTests(testApks) } + args.numUniformShards == null -> ArgsHelper.calculateShards(testMethods, args) + else -> listOf(testMethods.map(FlankTestMethod::testName)) + } + } + +private fun getFlankTestMethods( + testApks: List, + testFilter: TestFilter +): Map> = + testApks.associateWith { testApk -> + DexParser.findTestMethods(testApk.test.local).asSequence().distinct().filter(testFilter.shouldRun).map { testMethod -> + FlankTestMethod( + testName = "class ${testMethod.testName}", + ignored = testMethod.annotations.any { it.name == "org.junit.Ignore" } + ) + }.toList() + } + +private fun List.download(): List = + map { reference -> + reference.copy( + app = reference.app.downloadIfNeeded(), + test = reference.test.downloadIfNeeded() + ) + } + +private fun printNoTests(testApks: List) { + val testApkNames = testApks.joinToString(", ") { pathname -> File(pathname.test.local).name } + println("${FtlConstants.indent}No tests for $testApkNames") +} diff --git a/test_runner/src/main/kotlin/ftl/util/FileReference.kt b/test_runner/src/main/kotlin/ftl/util/FileReference.kt new file mode 100644 index 0000000000..d66a97a9d9 --- /dev/null +++ b/test_runner/src/main/kotlin/ftl/util/FileReference.kt @@ -0,0 +1,20 @@ +package ftl.util + +import ftl.config.FtlConstants +import ftl.gc.GcStorage + +data class FileReference( + val local: String = "", + val gcs: String = "" +) + +fun String.asFileReference(): FileReference = + if (startsWith(FtlConstants.GCS_PREFIX)) + FileReference(gcs = this) else + FileReference(local = this) + +fun FileReference.downloadIfNeeded() = when { + local.isNotBlank() -> this + gcs.isNotBlank() -> copy(local = GcStorage.download(gcs)) + else -> this +} diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt index 6e592e9c1b..113bf12b49 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt @@ -10,6 +10,8 @@ import ftl.args.yml.FlankYmlParams import ftl.args.yml.GcloudYml import ftl.args.yml.GcloudYmlParams import ftl.config.Device +import ftl.run.platform.android.getAndroidMatrixShards +import ftl.run.platform.android.getAndroidShardChunks import ftl.run.status.OutputStyle import ftl.test.util.FlankTestRunner import ftl.test.util.TestHelper.absolutePath @@ -139,21 +141,18 @@ class AndroidArgsFileTest { ), "" ) - - val testShardChunks = AndroidTestShard.getTestShardChunks(config) - with(config) { - assert(maxTestShards, 3) - assert(testShardChunks.size, 3) - assert(testShardChunks[0].size, 52) - assert(testShardChunks[1].size, 52) - assert(testShardChunks[2].size, 52) + with(getAndroidMatrixShards(config)) { + assertEquals(1, get("matrix-0")!!.shards["shard-0"]!!.size) + assertEquals(51, get("matrix-1")!!.shards["shard-0"]!!.size) + assertEquals(52, get("matrix-1")!!.shards["shard-1"]!!.size) + assertEquals(52, get("matrix-1")!!.shards["shard-2"]!!.size) } } @Test fun `calculateShards 0`() { val config = configWithTestMethods(0) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks = getAndroidShardChunks(config, config.testApk!!) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 0) @@ -163,7 +162,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 1`() { val config = configWithTestMethods(1) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks = getAndroidShardChunks(config, config.testApk!!) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 1) @@ -174,7 +173,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 155`() { val config = configWithTestMethods(155) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks = getAndroidShardChunks(config, config.testApk!!) with(config) { assert(maxTestShards, 1) assert(testShardChunks.size, 1) @@ -185,7 +184,7 @@ class AndroidArgsFileTest { @Test fun `calculateShards 155 40`() { val config = configWithTestMethods(155, maxTestShards = 40) - val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + val testShardChunks = getAndroidShardChunks(config, config.testApk!!) with(config) { assert(maxTestShards, 40) assert(testShardChunks.size, 40) diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt index ffb8585593..23e9841166 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt @@ -8,6 +8,7 @@ import ftl.config.FlankRoboDirective import ftl.config.FtlConstants.defaultAndroidModel import ftl.config.FtlConstants.defaultAndroidVersion import ftl.run.platform.runAndroidTests +import ftl.run.platform.android.getAndroidShardChunks import ftl.run.status.OutputStyle import ftl.test.util.FlankTestRunner import ftl.test.util.TestHelper.absolutePath @@ -16,7 +17,7 @@ import ftl.test.util.TestHelper.getPath import ftl.util.FlankCommonException import ftl.util.FlankFatalError import io.mockk.every -import io.mockk.mockkObject +import io.mockk.mockkStatic import io.mockk.unmockkAll import kotlinx.coroutines.runBlocking import org.junit.After @@ -447,7 +448,7 @@ AndroidArgs """ ) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) + val testShardChunks = getAndroidShardChunks(androidArgs, androidArgs.testApk!!) with(androidArgs) { assert(maxTestShards, -1) assert(testShardChunks.size, 2) @@ -481,7 +482,7 @@ AndroidArgs disable-sharding: true """ val androidArgs = AndroidArgs.load(yaml) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) + val testShardChunks = getAndroidShardChunks(androidArgs, androidArgs.testApk!!) assertThat(testShardChunks).hasSize(0) } @@ -493,7 +494,7 @@ AndroidArgs test: $invalidApk """ val androidArgs = AndroidArgs.load(yaml) - val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) + val testShardChunks = getAndroidShardChunks(androidArgs, androidArgs.testApk!!) assertThat(testShardChunks).hasSize(0) } @@ -1274,8 +1275,8 @@ AndroidArgs test: $testApk """.trimIndent() - mockkObject(AndroidTestShard) - every { AndroidTestShard.getTestShardChunks(any(), any()) } returns listOf() + mockkStatic("ftl.run.platform.android.GetAndroidShardChunksKt") + every { getAndroidShardChunks(any(), any()) } returns listOf() val parsedYml = AndroidArgs.load(yaml) runBlocking { runAndroidTests(parsedYml) } From 07ed2c35ea6684650689f5656d945cdd12769905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janek=20G=C3=B3ral?= Date: Fri, 22 May 2020 21:19:51 +0200 Subject: [PATCH 09/11] Update release_notes.md --- release_notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_notes.md b/release_notes.md index 03d23e5eb2..30f3eba13c 100644 --- a/release_notes.md +++ b/release_notes.md @@ -12,7 +12,7 @@ - [#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)) -- [#794](https://github.com/Flank/flank/pull/794) Enhance `--dump-shards` to dump shards from all test apks ([bootstraponline](https://github.com/bootstraponline)) +- [#794](https://github.com/Flank/flank/pull/794) Enhance `--dump-shards` to dump shards from all test apks ([bootstraponline](https://github.com/bootstraponline), [jan-gogo](https://github.com/jan-gogo)) ## v20.05.1 From 560656186ad98472e2303382313ca1b384580684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janek=20G=C3=B3ral?= Date: Sat, 23 May 2020 16:18:13 +0200 Subject: [PATCH 10/11] Handle num-uniform-shards with dump-shards & fix descriptions --- test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt | 8 ++++++-- .../ftl/cli/firebase/test/android/AndroidRunCommand.kt | 7 +++++-- .../kotlin/ftl/cli/firebase/test/ios/IosRunCommand.kt | 7 +++++-- .../run/platform/android/GetInstrumentationShardChunks.kt | 8 +++----- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt index 7ef73ea3e8..4db5eead49 100644 --- a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt +++ b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt @@ -224,7 +224,11 @@ object ArgsHelper { return ArgsFileVisitor("glob:$filePath").walk(searchDir) } - fun calculateShards(filteredTests: List, args: IArgs): ShardChunks { + fun calculateShards( + filteredTests: List, + args: IArgs, + forcedShardCount: Int? = null + ): ShardChunks { if (filteredTests.isEmpty()) { // Avoid unnecessary computing if we already know there aren't tests to run. return listOf(emptyList()) @@ -234,7 +238,7 @@ object ArgsHelper { listOf(filteredTests.map { it.testName }.toMutableList()) } else { val oldTestResult = GcStorage.downloadJunitXml(args) ?: JUnitTestResult(mutableListOf()) - val shardCount = Shard.shardCountByTime(filteredTests, oldTestResult, args) + val shardCount = forcedShardCount ?: Shard.shardCountByTime(filteredTests, oldTestResult, args) Shard.createShardsByShardCount(filteredTests, oldTestResult, args, shardCount).stringShards() } 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 index c83c3b54fa..3cc9e619ad 100644 --- 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 @@ -10,7 +10,7 @@ import ftl.config.FtlConstants.defaultAndroidVersion import ftl.config.FtlConstants.defaultLocale import ftl.config.FtlConstants.defaultOrientation import ftl.mock.MockServer -import ftl.run.IOS_SHARD_FILE +import ftl.run.ANDROID_SHARD_FILE import ftl.run.dumpShards import ftl.run.newTestRun import kotlinx.coroutines.runBlocking @@ -51,7 +51,10 @@ class AndroidRunCommand : CommonRunCommand(), Runnable { // Flank debug - @Option(names = ["--dump-shards"], description = ["Dumps the shards to $IOS_SHARD_FILE for debugging"]) + @Option( + names = ["--dump-shards"], + description = ["Measures test shards from given test apks and writes them into $ANDROID_SHARD_FILE file instead of executing."] + ) var dumpShards: Boolean = false // Flank specific 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 index c9f7de7315..52e362ca29 100644 --- 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 @@ -7,7 +7,7 @@ import ftl.config.FtlConstants import ftl.config.FtlConstants.defaultIosModel import ftl.config.FtlConstants.defaultIosVersion import ftl.mock.MockServer -import ftl.run.ANDROID_SHARD_FILE +import ftl.run.IOS_SHARD_FILE import ftl.run.dumpShards import ftl.run.newTestRun import kotlinx.coroutines.runBlocking @@ -47,7 +47,10 @@ class IosRunCommand : CommonRunCommand(), Runnable { // Flank debug - @Option(names = ["--dump-shards"], description = ["Dumps the shards to $ANDROID_SHARD_FILE for debugging"]) + @Option( + names = ["--dump-shards"], + description = ["Measures test shards from given test apks and writes them into $IOS_SHARD_FILE file instead of executing."] + ) var dumpShards: Boolean = false // Flank specific diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt index 4ffac33d88..935b08c99d 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt @@ -20,11 +20,9 @@ fun getInstrumentationShardChunks( testApks = testApks.download(), testFilter = TestFilters.fromTestTargets(args.testTargets) ).mapValues { (_, testMethods: List) -> - when { - testMethods.isEmpty() -> emptyList>().also { printNoTests(testApks) } - args.numUniformShards == null -> ArgsHelper.calculateShards(testMethods, args) - else -> listOf(testMethods.map(FlankTestMethod::testName)) - } + if (testMethods.isEmpty()) + emptyList>().also { printNoTests(testApks) } else + ArgsHelper.calculateShards(testMethods, args, args.numUniformShards) } private fun getFlankTestMethods( From e878905170784a762a9a7c8c5e65086ec75632de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janek=20G=C3=B3ral?= Date: Sat, 23 May 2020 16:40:35 +0200 Subject: [PATCH 11/11] Remove unnecessary `also` --- .../run/platform/android/GetInstrumentationShardChunks.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt b/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt index 935b08c99d..a15a8ea113 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/android/GetInstrumentationShardChunks.kt @@ -20,9 +20,12 @@ fun getInstrumentationShardChunks( testApks = testApks.download(), testFilter = TestFilters.fromTestTargets(args.testTargets) ).mapValues { (_, testMethods: List) -> - if (testMethods.isEmpty()) - emptyList>().also { printNoTests(testApks) } else + if (testMethods.isNotEmpty()) { ArgsHelper.calculateShards(testMethods, args, args.numUniformShards) + } else { + printNoTests(testApks) + emptyList() + } } private fun getFlankTestMethods(