Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow APKs with zero tests without throwing an exception #741

Merged
merged 6 commits into from
Apr 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
@@ -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))
Expand Down
10 changes: 5 additions & 5 deletions test_runner/src/main/kotlin/ftl/args/AndroidTestShard.kt
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,9 @@ object AndroidTestShard {

private fun getTestMethods(args: AndroidArgs, testLocalApk: String): List<FlankTestMethod> {
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
Expand All @@ -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") }
}
}
Expand Down
7 changes: 6 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,13 @@ object ArgsHelper {
}

fun calculateShards(filteredTests: List<FlankTestMethod>, 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<String>)
// Avoid to cast it to MutableList<String> to be agnostic from the caller.
listOf(filteredTests.map { it.testName }.toMutableList())
fondesa marked this conversation as resolved.
Show resolved Hide resolved
} else {
val oldTestResult = GcStorage.downloadJunitXml(args) ?: JUnitTestResult(mutableListOf())
val shardCount = Shard.shardCountByTime(filteredTests, oldTestResult, args)
Expand Down
17 changes: 13 additions & 4 deletions test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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
Expand All @@ -83,3 +90,5 @@ private suspend fun executeAndroidTestMatrix(
}
}
}

private fun ShardChunks.filterAtLeastOneTest(): ShardChunks = filter { chunk -> chunk.isNotEmpty() }
4 changes: 2 additions & 2 deletions test_runner/src/main/kotlin/ftl/shard/Shard.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ data class TestShard(
)

/** List of shards containing test names as a string. */
typealias StringShards = MutableList<MutableList<String>>
typealias StringShards = List<MutableList<String>>

fun List<TestShard>.stringShards(): StringShards {
return this.map { shard ->
shard.testMethods.map { it.name }.toMutableList()
}.toMutableList()
}.toList()
}

/*
Expand Down
16 changes: 8 additions & 8 deletions test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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()!!
Expand All @@ -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() {
Expand Down Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down