diff --git a/release_notes.md b/release_notes.md index 4a8ed4c704..edd59b17a4 100644 --- a/release_notes.md +++ b/release_notes.md @@ -1,4 +1,5 @@ ## next (unreleased) +- [#741](https://github.com/Flank/flank/pull/741) Allow APKs with zero tests. ([fondesa](https://github.com/fondesa)) - [#737](https://github.com/Flank/flank/pull/737) Generate ascii doc. ([jan-gogo](https://github.com/jan-gogo)) - [#720](https://github.com/Flank/flank/pull/720) Update group id from `flank` to `com.github.flank` ([bootstraponline](https://github.com/bootstraponline)) - [#714](https://github.com/Flank/flank/pull/714) Add support for num-uniform-shards option. ([jan-gogo](https://github.com/jan-gogo)) diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt index 3213ee102e..6a406099fd 100644 --- a/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt +++ b/test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt @@ -27,11 +27,9 @@ object AndroidTestShard { private fun getTestMethods(args: AndroidArgs, testLocalApk: String): List { val allTestMethods = DexParser.findTestMethods(testLocalApk) - val shouldIgnoreMissingTests = allTestMethods.isEmpty() && args.disableSharding - val shouldThrowErrorIfMissingTests = allTestMethods.isEmpty() && !args.disableSharding - when { - shouldIgnoreMissingTests -> return mutableListOf() - shouldThrowErrorIfMissingTests -> throw FlankFatalError("Test APK has no tests") + 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 @@ -43,6 +41,8 @@ object AndroidTestShard { .map { FlankTestMethod("class ${it.testName}", it.isIgnored) } .toList() .also { + // Since filtering tests is a manual configuration, to avoid silent errors for an incorrect configuration, + // an exception is thrown when all tests are filtered by the user. require(FtlConstants.useMock || it.isNotEmpty()) { throw FlankFatalError("All tests filtered out") } } } diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt index e0f300dd97..05f1abe837 100644 --- a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt +++ b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt @@ -223,8 +223,13 @@ object ArgsHelper { } fun calculateShards(filteredTests: List, args: IArgs): ShardChunks { + if (filteredTests.isEmpty()) { + // Avoid unnecessary computing if we already know there aren't tests to run. + return listOf(emptyList()) + } val shards = if (args.disableSharding) { - mutableListOf(filteredTests.map { it.testName } as MutableList) + // Avoid to cast it to MutableList to be agnostic from the caller. + listOf(filteredTests.map { it.testName }.toMutableList()) } else { val oldTestResult = GcStorage.downloadJunitXml(args) ?: JUnitTestResult(mutableListOf()) val shardCount = Shard.shardCountByTime(filteredTests, oldTestResult, args) 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 af6338cca2..4e61fadd76 100644 --- a/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt +++ b/test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt @@ -4,6 +4,7 @@ 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.args.yml.ResolvedApks import ftl.gc.GcAndroidDevice import ftl.gc.GcAndroidTestMatrix @@ -12,8 +13,8 @@ import ftl.http.executeWithRetry import ftl.run.model.TestResult import ftl.run.platform.android.createAndroidTestConfig import ftl.run.platform.android.resolveApks -import ftl.run.platform.android.uploadOtherFiles import ftl.run.platform.android.uploadApks +import ftl.run.platform.android.uploadOtherFiles import ftl.run.platform.common.afterRunTests import ftl.run.platform.common.beforeRunMessage import ftl.run.platform.common.beforeRunTests @@ -38,9 +39,16 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS args.resolveApks().forEach { apks: ResolvedApks -> val testShards = apks.test?.let { test -> - AndroidTestShard.getTestShardChunks(args, test).also { - allTestShardChunks += it + AndroidTestShard.getTestShardChunks(args, test) + } + // We can't return if testShards is null since it can be a robo test. + testShards?.let { + val shardsWithAtLeastOneTest = testShards.filterAtLeastOneTest() + if (shardsWithAtLeastOneTest.isEmpty()) { + // No tests to run, skipping the execution. + return@forEach } + allTestShardChunks += shardsWithAtLeastOneTest } val uploadedApks = uploadApks( @@ -67,7 +75,6 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS ) } } - println(beforeRunMessage(args, allTestShardChunks)) val matrixMap = afterRunTests(testMatrices.awaitAll(), runGcsPath, stopwatch, args) matrixMap to allTestShardChunks @@ -83,3 +90,5 @@ private suspend fun executeAndroidTestMatrix( } } } + +private fun ShardChunks.filterAtLeastOneTest(): ShardChunks = filter { chunk -> chunk.isNotEmpty() } diff --git a/test_runner/src/main/kotlin/ftl/shard/Shard.kt b/test_runner/src/main/kotlin/ftl/shard/Shard.kt index 692c5807fc..77f9e7a854 100644 --- a/test_runner/src/main/kotlin/ftl/shard/Shard.kt +++ b/test_runner/src/main/kotlin/ftl/shard/Shard.kt @@ -23,12 +23,12 @@ data class TestShard( ) /** List of shards containing test names as a string. */ -typealias StringShards = MutableList> +typealias StringShards = List> fun List.stringShards(): StringShards { return this.map { shard -> shard.testMethods.map { it.name }.toMutableList() - }.toMutableList() + }.toList() } /* diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt index 0cc44fed03..406a6d34ed 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt @@ -18,16 +18,11 @@ 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.rules.ExpectedException import org.junit.runner.RunWith @RunWith(FlankTestRunner::class) class AndroidArgsFileTest { - @Rule - @JvmField - val exceptionRule = ExpectedException.none()!! - @Rule @JvmField val systemErrRule = SystemErrRule().muteForSuccessfulTests()!! @@ -47,6 +42,7 @@ class AndroidArgsFileTest { private val appApkAbsolutePath = appApkLocal.absolutePath() private val testApkAbsolutePath = testApkLocal.absolutePath() + // NOTE: Change working dir to '%MODULE_WORKING_DIR%' in IntelliJ to match gradle for this test to pass. @Test fun localConfigLoadsSuccessfully() { @@ -113,9 +109,13 @@ class AndroidArgsFileTest { @Test fun `calculateShards 0`() { - exceptionRule.expectMessage("Test APK has no tests") - val args = configWithTestMethods(0) - AndroidTestShard.getTestShardChunks(args, args.testApk!!) + val config = configWithTestMethods(0) + val testShardChunks = AndroidTestShard.getTestShardChunks(config, config.testApk!!) + with(config) { + assert(maxTestShards, 1) + assert(testShardChunks.size, 1) + assert(testShardChunks.first().size, 0) + } } @Test diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt index e2acb46652..3f3725459b 100644 --- a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt +++ b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt @@ -466,15 +466,16 @@ AndroidArgs assertThat(testShardChunks).hasSize(1) } - @Test(expected = FlankFatalError::class) - fun `Invalid apk throws`() { + @Test + fun `enable sharding allows using invalid apk`() { val yaml = """ gcloud: app: $invalidApk test: $invalidApk """ val androidArgs = AndroidArgs.load(yaml) - AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) + val testShardChunks = AndroidTestShard.getTestShardChunks(androidArgs, androidArgs.testApk!!) + assertThat(testShardChunks).hasSize(1) } @Test