fix: Shards and Tests count do not match

docs/bugs/986_test_count_and_smart_sharding_ count_dont_match.md

@@ -0,0 +1,36 @@
+# Test count and smart sharding count don't match
+
+Bug reported in [this issue](https://github.com/Flank/flank/issues/986)
+
+## The problem
+
+Flank does not support parameterized tests sharding. Every class with parameterized is considered as one test during shard calculation.
+
+Flank is using [DEX parser](https://github.com/linkedin/dex-test-parser) to decompile apks and gather info about all the tests inside. As for now, Flank is unable to determine how many times a test in a parameterized class is invoked. Due to this fact scans apks for any class with an annotation that contains `JUnitParamsRunner` or `Parameterized`:
+
+```kotlin
+
+@RunWith(JUnitParamsRunner::class)
+...
+@RunWith(Parameterized::class)
+
+```
+
+## Solution
+
+1. Flank knows how many tests and classes are being sent to Firebase. So we can inform the user of how many classes we have. Example:
+
+```txt
+
+ Smart Flank cache hit: 0% (0 / 9)
+  Shard times: 240s, 240s, 240s, 360s
+
+  Uploading app-debug.apk .
+  Uploading app-multiple-flaky-debug-androidTest.apk .
+  5 tests + 4 parameterized classes / 4 shards
+
+```
+
+1. Default test time for classes should be different from the default time for test
+   1. You can set default test time for class with ```--default-class-test-time``` command
+   2. If you did not set this time, the default value is 240s

test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt

@@ -20,12 +20,12 @@ import ftl.config.FtlConstants.useMock
 import ftl.gc.GcStorage
 import ftl.gc.GcToolResults
 import ftl.reports.xml.model.JUnitTestResult
-import ftl.shard.StringShards
 import ftl.shard.createShardsByShardCount
 import ftl.shard.shardCountByTime
-import ftl.shard.stringShards
 import ftl.run.exception.FlankConfigurationError
 import ftl.run.exception.FlankGeneralError
+import ftl.shard.Chunk
+import ftl.shard.TestMethod
 import ftl.util.FlankTestMethod
 import ftl.util.assertNotEmpty
 import java.io.File
@@ -222,30 +222,25 @@ object ArgsHelper {
     ): CalculateShardsResult {
         if (filteredTests.isEmpty()) {
             // Avoid unnecessary computing if we already know there aren't tests to run.
-            return CalculateShardsResult(listOf(emptyList()), emptyList())
+            return CalculateShardsResult(emptyList(), emptyList())
         }
         val (ignoredTests, testsToExecute) = filteredTests.partition { it.ignored }
         val shards = if (args.disableSharding) {
-            // Avoid to cast it to MutableList<String> to be agnostic from the caller.
-            listOf(testsToExecute.map { it.testName }.toMutableList())
+            listOf(Chunk(testsToExecute.map { TestMethod(name = it.testName, isParameterized = it.isParameterizedClass, time = 0.0) }))
         } else {
             val oldTestResult = GcStorage.downloadJunitXml(args) ?: JUnitTestResult(mutableListOf())
             val shardCount = forcedShardCount ?: shardCountByTime(testsToExecute, oldTestResult, args)
-            createShardsByShardCount(testsToExecute, oldTestResult, args, shardCount).stringShards()
+            createShardsByShardCount(testsToExecute, oldTestResult, args, shardCount).map { Chunk(it.testMethods) }
         }
 
         return CalculateShardsResult(testMethodsAlwaysRun(shards, args), ignoredTestCases = ignoredTests.map { it.testName })
     }
 
-    private fun testMethodsAlwaysRun(shards: StringShards, args: IArgs): StringShards {
+    private fun testMethodsAlwaysRun(shards: List<Chunk>, args: IArgs): List<Chunk> {
         val alwaysRun = args.testTargetsAlwaysRun
+        val find = shards.flatMap { it.testMethods }.filter { alwaysRun.contains(it.name) }
 
-        shards.forEach { shard ->
-            shard.removeAll(alwaysRun)
-            shard.addAll(0, alwaysRun)
-        }
-
-        return shards
+        return shards.map { Chunk(find + it.testMethods.filterNot { method -> find.contains(method) }) }
     }
 }
 

test_runner/src/main/kotlin/ftl/args/CalculateShardsResult.kt

@@ -1,3 +1,5 @@
 package ftl.args
 
-data class CalculateShardsResult(val shardChunks: ShardChunks, val ignoredTestCases: IgnoredTestCases)
+import ftl.shard.Chunk
+
+data class CalculateShardsResult(val shardChunks: List<Chunk>, val ignoredTestCases: IgnoredTestCases)

test_runner/src/main/kotlin/ftl/args/CommonArgs.kt

@@ -36,5 +36,6 @@ data class CommonArgs(
     override val outputStyle: OutputStyle,
     override val disableResultsUpload: Boolean,
     override val defaultTestTime: Double,
+    override val defaultClassTestTime: Double,
     override val useAverageTestTimeForNewTests: Boolean,
 ) : IArgs

test_runner/src/main/kotlin/ftl/args/CreateCommonArgs.kt

@@ -43,6 +43,7 @@ fun CommonConfig.createCommonArgs(
     localResultDir = flank.localResultsDir!!,
     disableResultsUpload = flank.disableResultsUpload!!,
     defaultTestTime = flank.defaultTestTime!!,
+    defaultClassTestTime = flank.defaultClassTestTime!!,
     useAverageTestTimeForNewTests = flank.useAverageTestTimeForNewTests!!
 ).apply {
     ArgsHelper.createJunitBucket(project, smartFlankGcsPath)

test_runner/src/main/kotlin/ftl/args/IArgs.kt

@@ -60,6 +60,7 @@ interface IArgs {
         get() = maxTestShards in AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE
 
     val defaultTestTime: Double
+    val defaultClassTestTime: Double
     val useAverageTestTimeForNewTests: Boolean
 
     fun useLocalResultDir() = localResultDir != defaultLocalResultsDir

test_runner/src/main/kotlin/ftl/args/IosArgs.kt

@@ -3,6 +3,7 @@ package ftl.args
 import com.google.common.annotations.VisibleForTesting
 import ftl.ios.Xctestrun.findTestNames
 import ftl.run.exception.FlankConfigurationError
+import ftl.shard.Chunk
 import ftl.util.FlankTestMethod
 
 data class IosArgs(
@@ -14,7 +15,7 @@ data class IosArgs(
 ) : IArgs by commonArgs {
 
     override val useLegacyJUnitResult = true
-    val testShardChunks: ShardChunks by lazy { calculateShardChunks() }
+    val testShardChunks: List<Chunk> by lazy { calculateShardChunks() }
 
     companion object : IosArgsCompanion()
 
@@ -62,7 +63,7 @@ IosArgs
 }
 
 private fun IosArgs.calculateShardChunks() = if (disableSharding)
-    listOf(emptyList()) else
+    emptyList() else
     ArgsHelper.calculateShards(
         filteredTests = filterTests(findTestNames(xctestrunFile), testTargets)
             .distinct()

test_runner/src/main/kotlin/ftl/config/common/CommonFlankConfig.kt

@@ -7,6 +7,7 @@ import ftl.args.ArgsHelper
 import ftl.args.yml.IYmlKeys
 import ftl.config.Config
 import ftl.config.FtlConstants
+import ftl.shard.DEFAULT_CLASS_TEST_TIME_SEC
 import ftl.shard.DEFAULT_TEST_TIME_SEC
 import picocli.CommandLine
 
@@ -148,6 +149,9 @@ data class CommonFlankConfig @JsonIgnore constructor(
     @set:JsonProperty("default-test-time")
     var defaultTestTime: Double? by data
 
+    @set:JsonProperty("default-class-test-time")
+    var defaultClassTestTime: Double? by data
+
     @set:CommandLine.Option(
         names = ["--use-average-test-time-for-new-tests"],
         description = ["Enable using average time from previous tests duration when using SmartShard and tests did not run before."]
@@ -178,6 +182,9 @@ data class CommonFlankConfig @JsonIgnore constructor(
             "output-style",
             "disable-results-upload",
             "full-junit-result",
+            "local-result-dir",
+            "default-test-time",
+            "default-class-test-time",
             "local-result-dir"
         )
 
@@ -201,6 +208,7 @@ data class CommonFlankConfig @JsonIgnore constructor(
             outputStyle = null
             disableResultsUpload = false
             defaultTestTime = DEFAULT_TEST_TIME_SEC
+            defaultClassTestTime = DEFAULT_CLASS_TEST_TIME_SEC
             useAverageTestTimeForNewTests = false
         }
     }

test_runner/src/main/kotlin/ftl/run/DumpShards.kt

@@ -7,6 +7,7 @@ import ftl.run.common.prettyPrint
 import ftl.run.exception.FlankConfigurationError
 import ftl.run.model.AndroidMatrixTestShards
 import ftl.run.platform.android.getAndroidMatrixShards
+import ftl.shard.testCases
 import ftl.util.obfuscatePrettyPrinter
 import java.nio.file.Files
 import java.nio.file.Paths
@@ -35,7 +36,7 @@ fun dumpShards(
 ) {
     saveShardChunks(
         shardFilePath = shardFilePath,
-        shards = args.testShardChunks,
+        shards = args.testShardChunks.testCases,
         size = args.testShardChunks.size,
         obfuscatedOutput = obfuscatedOutput
     )

test_runner/src/main/kotlin/ftl/run/model/AndroidTestContext.kt

@@ -1,15 +1,15 @@
 package ftl.run.model
 
+import ftl.shard.Chunk
 import ftl.args.IgnoredTestCases
-import ftl.args.ShardChunks
 import ftl.util.FileReference
 
 sealed class AndroidTestContext
 
 data class InstrumentationTestContext(
     val app: FileReference,
     val test: FileReference,
-    val shards: ShardChunks = emptyList(),
+    val shards: List<Chunk> = emptyList(),
     val ignoredTestCases: IgnoredTestCases = emptyList()
 ) : AndroidTestContext()
 

test_runner/src/main/kotlin/ftl/run/platform/RunAndroidTests.kt

@@ -18,6 +18,8 @@ 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.shard.Chunk
+import ftl.shard.testCases
 import kotlinx.coroutines.Deferred
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.async
@@ -31,7 +33,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
     // GcAndroidTestMatrix.execute() 3x retry => matrix id (string)
     val devices = GcAndroidDevice.build(args.devices)
     val testMatrices = mutableListOf<Deferred<TestMatrix>>()
-    val allTestShardChunks = mutableListOf<List<String>>()
+    val allTestShardChunks = mutableListOf<Chunk>()
     val ignoredTestsShardChunks = mutableListOf<List<String>>()
 
     val history = GcToolResults.createToolResultsHistory(args)
@@ -64,7 +66,7 @@ internal suspend fun runAndroidTests(args: AndroidArgs): TestResult = coroutineS
     println(beforeRunMessage(args, allTestShardChunks))
     TestResult(
         matrixMap = afterRunTests(testMatrices.awaitAll(), runGcsPath, stopwatch, args),
-        shardChunks = allTestShardChunks,
+        shardChunks = allTestShardChunks.testCases,
         ignoredTests = ignoredTestsShardChunks.flatten()
     )
 }

test_runner/src/main/kotlin/ftl/run/platform/RunIosTests.kt

@@ -13,6 +13,7 @@ import ftl.run.model.TestResult
 import ftl.run.platform.common.afterRunTests
 import ftl.run.platform.common.beforeRunMessage
 import ftl.run.platform.common.beforeRunTests
+import ftl.shard.testCases
 import ftl.util.ShardCounter
 import kotlinx.coroutines.Deferred
 import kotlinx.coroutines.Dispatchers
@@ -47,7 +48,7 @@ internal suspend fun runIosTests(iosArgs: IosArgs): TestResult = coroutineScope
                     iosDeviceList = iosDeviceList,
                     testZipGcsPath = xcTestGcsPath,
                     runGcsPath = runGcsPath,
-                    testTargets = testTargets,
+                    testTargets = testTargets.testMethodNames,
                     xcTestParsed = xcTestParsed,
                     args = iosArgs,
                     shardCounter = shardCounter,
@@ -59,7 +60,7 @@ internal suspend fun runIosTests(iosArgs: IosArgs): TestResult = coroutineScope
 
     TestResult(
         matrixMap = afterRunTests(jobs.awaitAll(), runGcsPath, stopwatch, iosArgs),
-        shardChunks = iosArgs.testShardChunks
+        shardChunks = iosArgs.testShardChunks.testCases
     )
 }
 

test_runner/src/main/kotlin/ftl/run/platform/android/CreateAndroidTestContext.kt

@@ -55,7 +55,7 @@ private fun InstrumentationTestContext.calculateShards(
     forcedShardCount = args.numUniformShards
 ).run {
     copy(
-        shards = shardChunks.filter { it.isNotEmpty() },
+        shards = shardChunks.filter { it.testMethods.isNotEmpty() },
         ignoredTestCases = ignoredTestCases
     )
 }
@@ -86,7 +86,7 @@ private val ignoredAnnotations = listOf(
     "android.support.test.filters.Suppress"
 )
 
-private fun String.toFlankTestMethod() = FlankTestMethod("class $this", ignored = false)
+private fun String.toFlankTestMethod() = FlankTestMethod("class $this", ignored = false, isParameterizedClass = true)
 
 private fun InstrumentationTestContext.getParametrizedClasses(): List<String> =
     DexParser.readDexFiles(test.local).fold(emptyList()) { accumulator, file: DexFile ->

test_runner/src/main/kotlin/ftl/run/platform/android/CreateInstrumentationConfig.kt

@@ -2,6 +2,7 @@ package ftl.run.platform.android
 
 import ftl.args.AndroidArgs
 import ftl.run.model.InstrumentationTestContext
+import ftl.shard.testCases
 
 internal fun AndroidArgs.createInstrumentationConfig(
     testApk: InstrumentationTestContext
@@ -12,6 +13,6 @@ internal fun AndroidArgs.createInstrumentationConfig(
     orchestratorOption = "USE_ORCHESTRATOR".takeIf { useOrchestrator },
     disableSharding = disableSharding,
     numUniformShards = numUniformShards,
-    testShards = testApk.shards,
+    testShards = testApk.shards.testCases,
     keepTestTargetsEmpty = disableSharding && testTargets.isEmpty()
 )

test_runner/src/main/kotlin/ftl/run/platform/android/GetAndroidMatrixShards.kt

@@ -4,6 +4,7 @@ import ftl.args.AndroidArgs
 import ftl.run.model.AndroidMatrixTestShards
 import ftl.run.model.AndroidTestShards
 import ftl.run.model.InstrumentationTestContext
+import ftl.shard.testCases
 
 suspend fun AndroidArgs.getAndroidMatrixShards(): AndroidMatrixTestShards = this
     .createAndroidTestContexts()
@@ -15,7 +16,7 @@ private fun List<InstrumentationTestContext>.asMatrixTestShards(): AndroidMatrix
         AndroidTestShards(
             app = testApks.app.local,
             test = testApks.test.local,
-            shards = testApks.shards.mapIndexed { index, testCases ->
+            shards = testApks.shards.testCases.mapIndexed { index, testCases ->
                 "shard-$index" to testCases
             }.toMap(),
             junitIgnored = testApks.ignoredTestCases

test_runner/src/main/kotlin/ftl/run/platform/common/BeforeRunMessage.kt

@@ -1,33 +1,46 @@
 package ftl.run.platform.common
 
 import ftl.args.IArgs
-import ftl.args.ShardChunks
+import ftl.config.FtlConstants
+import ftl.shard.Chunk
+import ftl.shard.TestMethod
 
-internal fun beforeRunMessage(args: IArgs, testShardChunks: ShardChunks): String {
+internal fun beforeRunMessage(args: IArgs, testShardChunks: List<Chunk>): String {
     val runCount = args.repeatTests
     val shardCount = testShardChunks.size
-    val testsCount = testShardChunks.sumBy { it.size }
+    val (classesCount, testsCount) = testShardChunks.partitionedTestCases.testAndClassesCount
 
     val result = StringBuilder()
+    val testString = if (testsCount > 0) "$testsCount test${s(testsCount)}" else ""
+    val classString = if (classesCount > 0) "$classesCount parameterized class${es(classesCount)}" else ""
+
     result.appendLine(
-        "  $testsCount test${s(testsCount)} / $shardCount shard${s(shardCount)}"
+        "${FtlConstants.indent}$testString${if (testsCount * classesCount > 0) " + " else ""}$classString / $shardCount shard${s(shardCount)}"
     )
 
     if (runCount > 1) {
         result.appendLine("  Running ${runCount}x")
         val runDevices = runCount * shardCount
         val runTests = runCount * testsCount
+        val runClasses = runCount * classesCount
         result.appendLine("    $runDevices total shard${s(runDevices)}")
-        result.appendLine("    $runTests total test${s(runTests)}")
+        if (runTests > 0) result.appendLine("    $runTests total test${s(runTests)}")
+        if (runClasses > 0) result.appendLine("    $runClasses total parameterized class${es(runClasses)}")
     }
 
     return result.toString()
 }
 
-private fun s(amount: Int): String {
-    return if (amount > 1) {
-        "s"
-    } else {
-        ""
-    }
-}
+private val List<Chunk>.partitionedTestCases
+    get() = flatMap { it.testMethods }.partition { it.isParameterized }
+
+private val Pair<List<TestMethod>, List<TestMethod>>.testAndClassesCount
+    get() = first.count() to second.count()
+
+private fun s(amount: Int): String =
+    if (amount > 1) "s"
+    else ""
+
+private fun es(amount: Int): String =
+    if (amount > 1) "es"
+    else ""

test_runner/src/main/kotlin/ftl/shard/Chunk.kt

@@ -0,0 +1,9 @@
+package ftl.shard
+
+data class Chunk(val testMethods: List<TestMethod>) {
+    val testMethodNames = testMethods.map { it.name }
+    val size get() = testMethods.size
+}
+
+val List<Chunk>.testCases
+    get() = map { it.testMethodNames }