Fix test sharding and improve test coverage (#346)

Flank · Oct 11, 2018 · 7bb7531 · 7bb7531
1 parent 86a530f
commit 7bb7531
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 37 deletions.
diff --git a/test_runner/gradle.properties b/test_runner/gradle.properties
@@ -10,8 +10,6 @@
 # Specifies the JVM arguments used for the daemon process.
 # The setting is particularly useful for tweaking memory settings.
 org.gradle.jvmargs=-Xmx2560m
+kotlin.code.style=official
+org.gradle.parallel=true
 
-# When configured, Gradle will run in incubating parallel mode.
-# This option should only be used with decoupled projects. More details, visit
-# http://www.gradle.org/docs/current/userguide/multi_project_builds.html#sec:decoupled_projects
-# org.gradle.parallel=true
diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
@@ -89,18 +89,17 @@ class AndroidArgs(
         val filteredTests = getTestMethods(testLocalApk)
 
         testShardChunks = calculateShards(
-            testTargets,
             filteredTests,
             testTargetsAlwaysRun,
             testShards
         )
     }
 
     private fun getTestMethods(testLocalApk: String): List<String> {
-        val validTestMethods = DexParser.findTestMethods(testLocalApk)
-        require(validTestMethods.isNotEmpty()) { Utils.fatalError("Test APK has no tests") }
+        val allTestMethods = DexParser.findTestMethods(testLocalApk)
+        require(allTestMethods.isNotEmpty()) { Utils.fatalError("Test APK has no tests") }
         val testFilter = TestFilters.fromTestTargets(testTargets)
-        val filteredTests = validTestMethods
+        val filteredTests = allTestMethods
             .asSequence()
             .filter(testFilter.shouldRun)
             .map(TestMethod::testName)

diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
@@ -68,16 +68,11 @@ object ArgsHelper {
     }
 
     fun calculateShards(
-        testTargets: Collection<String>,
-        validTestNames: Collection<String>,
+        testMethodsToShard: Collection<String>,
         testMethodsAlwaysRun: Collection<String>,
         testShards: Int
     ): List<List<String>> {
-        val testShardMethods = if (testTargets.isNotEmpty()) {
-            testTargets
-        } else {
-            validTestNames
-        }.distinct().toMutableList()
+        val testShardMethods = testMethodsToShard.distinct().toMutableList()
         testShardMethods.removeAll(testMethodsAlwaysRun)
 
         val oneTestPerChunk = testShards == -1
@@ -87,7 +82,10 @@ object ArgsHelper {
             chunkSize = 1
         }
 
-        val testShardChunks = testShardMethods.chunked(chunkSize).map { testMethodsAlwaysRun + it }
+        val testShardChunks = testShardMethods.asSequence()
+            .chunked(chunkSize)
+            .map { testMethodsAlwaysRun + it }
+            .toList()
 
         // Ensure we don't create more VMs than requested. VM count per run should be <= testShards
         if (!oneTestPerChunk && testShardChunks.size > testShards) {

diff --git a/test_runner/src/main/kotlin/ftl/args/IosArgs.kt b/test_runner/src/main/kotlin/ftl/args/IosArgs.kt
@@ -63,12 +63,16 @@ class IosArgs(
 
         val validTestMethods = Xctestrun.findTestNames(xctestrunFile)
         validateTestMethods(testTargets, validTestMethods, "xctest binary")
+        val testsToShard = if (testTargets.isEmpty()) {
+            validTestMethods
+        } else {
+            testTargets
+        }
 
         testShardChunks = ArgsHelper.calculateShards(
-            testTargets,
-            validTestMethods,
-            testTargetsAlwaysRun,
-            testShards
+            testMethodsToShard = testsToShard,
+            testMethodsAlwaysRun = testTargetsAlwaysRun,
+            testShards = testShards
         )
     }
 

diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt
@@ -15,11 +15,16 @@ 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()!!
@@ -85,17 +90,13 @@ class AndroidArgsFileTest {
     }
 
     private fun configWithTestMethods(amount: Int, testShards: Int = 1): AndroidArgs {
-        val testMethods = mutableListOf<String>()
-        // test names must be unique otherwise the Set<String> will add them only once.
-        repeat(amount) { index -> testMethods.add(testName + index) }
 
         return AndroidArgs(
             GcloudYml(GcloudYmlParams()),
             AndroidGcloudYml(
                 AndroidGcloudYmlParams(
                     app = appApkLocal,
-                    test = testApkLocal,
-                    testTargets = testMethods
+                    test = getString("src/test/kotlin/ftl/fixtures/tmp/apk/app-debug-androidTest_$amount.apk")
                 )
             ),
             FlankYml(
@@ -107,22 +108,34 @@ class AndroidArgsFileTest {
     }
 
     @Test
-    fun calculateShards() {
-        var config = configWithTestMethods(1)
+    fun calculateShards_0() {
+        exceptionRule.expectMessage("Test APK has no tests")
+        configWithTestMethods(0)
+    }
+
+    @Test
+    fun calculateShards_1() {
+        val config = configWithTestMethods(1)
         with(config) {
             assert(testShards, 1)
             assert(testShardChunks.size, 1)
             assert(testShardChunks.first().size, 1)
         }
+    }
 
-        config = configWithTestMethods(155)
+    @Test
+    fun calculateShards_155() {
+        val config = configWithTestMethods(155)
         with(config) {
             assert(testShards, 1)
             assert(testShardChunks.size, 1)
             assert(testShardChunks.first().size, 155)
         }
+    }
 
-        config = configWithTestMethods(155, testShards = 40)
+    @Test
+    fun calculateShards_155_40() {
+        val config = configWithTestMethods(155, testShards = 40)
         with(config) {
             assert(testShards, 40)
             assert(testShardChunks.size, 39)

diff --git a/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt b/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt
@@ -98,8 +98,7 @@ class ArgsHelperTest {
     fun calculateShards_fails_emptyShardChunks() {
         exceptionRule.expectMessage("Failed to populate test shard chunks")
         calculateShards(
-            testTargets = listOf(""),
-            validTestNames = listOf(""),
+            testMethodsToShard = listOf(""),
             testMethodsAlwaysRun = listOf(""),
             testShards = 1
         )
@@ -108,21 +107,43 @@ class ArgsHelperTest {
     @Test
     fun calculateShards_succeeds() {
         calculateShards(
-            testTargets = listOf("a", "b"),
-            validTestNames = listOf("a", "b", "c"),
+            testMethodsToShard = listOf("a", "b", "c"),
             testMethodsAlwaysRun = listOf("c"),
             testShards = -1
         )
     }
 
     @Test
     fun calculateShards_emptyTestTargets() {
-        calculateShards(
-            testTargets = listOf(),
-            validTestNames = listOf("a", "b", "c"),
+        val tests = listOf(
+            "class com.example.profile.ProfileTest#testOne",
+            "class com.example.profile.ProfileTest#testTwo"
+        )
+        val shards = calculateShards(
+            testMethodsToShard = tests,
+            testMethodsAlwaysRun = emptyList(),
+            testShards = -1
+        )
+        val expectedShards = listOf(
+            listOf(tests[0]),
+            listOf(tests[1])
+        )
+        assertThat(shards).isEqualTo(expectedShards)
+    }
+
+    @Test
+    fun calculateShards_packageTarget() {
+        val shards = calculateShards(
+            testMethodsToShard = listOf("a", "b", "c"),
             testMethodsAlwaysRun = listOf("c"),
             testShards = 2
         )
+
+        val expectedShards = listOf(
+            listOf("c", "a"),
+            listOf("c", "b")
+        )
+        assertThat(shards).isEqualTo(expectedShards)
     }
 
     @Test

diff --git a/test_runner/src/test/kotlin/ftl/args/FlankYmlTest.kt b/test_runner/src/test/kotlin/ftl/args/FlankYmlTest.kt
@@ -15,7 +15,7 @@ class FlankYmlTest {
 
     @Rule
     @JvmField
-    val exceptionRule = ExpectedException.none()
+    val exceptionRule = ExpectedException.none()!!
 
     @Rule
     @JvmField