SmartFlank algo

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

@@ -9,8 +9,7 @@ import ftl.android.UnsupportedModelId
 import ftl.android.UnsupportedVersionId
 import ftl.args.ArgsHelper.assertFileExists
 import ftl.args.ArgsHelper.assertGcsFileExists
-import ftl.args.ArgsHelper.calculateShards
-import ftl.args.ArgsHelper.evaluateFilePath
+import ftl.args.ArgsHelper.convertShards
 import ftl.args.ArgsHelper.createGcsBucket
 import ftl.args.ArgsHelper.createJunitBucket
 import ftl.args.ArgsHelper.evaluateFilePath
@@ -28,6 +27,8 @@ import ftl.config.FtlConstants
 import ftl.config.FtlConstants.useMock
 import ftl.filter.TestFilters
 import ftl.gc.GcStorage
+import ftl.reports.xml.model.JUnitTestResult
+import ftl.shard.Shard
 import ftl.util.Utils
 import kotlinx.coroutines.runBlocking
 import java.nio.file.Files
@@ -79,12 +80,8 @@ class AndroidArgs(
         }
 
         val filteredTests = getTestMethods(testLocalApk)
-
-        calculateShards(
-            filteredTests,
-            testTargetsAlwaysRun,
-            testShards
-            )
+        val oldTestResult = GcStorage.downloadJunitXml(this) ?: JUnitTestResult(mutableListOf())
+        convertShards(Shard.calculateShardsByTime(filteredTests, oldTestResult, testShards)).toMutableList()
     }
 
     init {

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

@@ -11,16 +11,15 @@ import com.google.cloud.storage.BucketInfo
 import com.google.cloud.storage.Storage
 import com.google.cloud.storage.StorageClass
 import com.google.cloud.storage.StorageOptions
-import com.google.common.math.IntMath
 import ftl.args.yml.IYmlMap
 import ftl.config.FtlConstants
 import ftl.config.FtlConstants.GCS_PREFIX
 import ftl.config.FtlConstants.JSON_FACTORY
 import ftl.config.FtlConstants.defaultCredentialPath
 import ftl.gc.GcStorage
+import ftl.shard.TestShard
 import ftl.util.Utils
 import java.io.File
-import java.math.RoundingMode
 import java.net.URI
 import java.nio.file.Files
 import java.nio.file.Path
@@ -95,39 +94,10 @@ object ArgsHelper {
         if (validTestMethods.isEmpty()) Utils.fatalError("$from has no tests")
     }
 
-    fun calculateShards(
-        testMethodsToShard: Collection<String>,
-        testMethodsAlwaysRun: Collection<String>,
-        testShards: Int
-    ): List<List<String>> {
-        val testShardMethods = testMethodsToShard.distinct().toMutableList()
-        testShardMethods.removeAll(testMethodsAlwaysRun)
-
-        val oneTestPerChunk = testShards == -1
-        var chunkSize = IntMath.divide(testShardMethods.size, testShards, RoundingMode.UP)
-
-        if (oneTestPerChunk || chunkSize < 1) {
-            chunkSize = 1
-        }
-
-        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) {
-            Utils.fatalError("Calculated chunks $testShardChunks is > requested $testShards testShards.")
-        }
-        if (testShardChunks.isEmpty()) Utils.fatalError("Failed to populate test shard chunks")
-
-        return testShardChunks
-    }
-
     fun createJunitBucket(projectId: String, junitGcsPath: String) {
         if (FtlConstants.useMock || junitGcsPath.isEmpty()) return
         val bucket = junitGcsPath.drop(GCS_PREFIX.length).substringBefore('/')
-        createGcsBucket(projectId, bucket)
+            createGcsBucket(projectId, bucket)
     }
 
     fun createGcsBucket(projectId: String, bucket: String): String {
@@ -182,6 +152,7 @@ object ArgsHelper {
 
     // https://stackoverflow.com/a/2821201/2450315
     private val envRegex = Pattern.compile("\\$([a-zA-Z_]+[a-zA-Z0-9_]*)")
+
     private fun evaluateEnvVars(text: String): String {
         val buffer = StringBuffer()
         val matcher = envRegex.matcher(text)
@@ -202,4 +173,10 @@ object ArgsHelper {
 
         return ArgsFileVisitor("glob:$filePath").walk(searchDir)
     }
+
+    fun convertShards(shards: List<TestShard>): List<List<String>> {
+        return shards.map { shard ->
+            shard.testMethods.map { it.name }
+        }
+    }
 }

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

@@ -63,11 +63,8 @@ class IosArgs(
             testTargets
         }
 
-        ArgsHelper.calculateShards(
-            testMethodsToShard = testsToShard,
-            testMethodsAlwaysRun = testTargetsAlwaysRun,
-            testShards = testShards
-            )
+        // TODO: Use Shard.calculateShardsByTime
+        emptyList<List<String>>()
     }
 
     init {

test_runner/src/main/kotlin/ftl/gc/GcStorage.kt

@@ -9,6 +9,9 @@ import ftl.args.IArgs
 import ftl.args.IosArgs
 import ftl.config.FtlConstants
 import ftl.config.FtlConstants.GCS_PREFIX
+import ftl.reports.xml.model.JUnitTestResult
+import ftl.reports.xml.parseIosXml
+import ftl.reports.xml.xmlToString
 import ftl.util.Utils.fatalError
 import ftl.util.Utils.join
 import java.io.File
@@ -41,9 +44,8 @@ object GcStorage {
             runGcsPath = runGcsPath
         )
 
-    fun uploadJunitXml(localJunitXml: String, args: IArgs) {
+    fun uploadJunitXml(testResult: JUnitTestResult, args: IArgs) {
         if (args.junitGcsPath.isEmpty()) return
-        if (File(localJunitXml).exists().not()) return
 
         // bucket/path/to/object
         val rawPath = args.junitGcsPath.drop(GCS_PREFIX.length)
@@ -53,7 +55,7 @@ object GcStorage {
         val fileBlob = BlobInfo.newBuilder(bucket, name).build()
 
         try {
-            storage.create(fileBlob, Files.readAllBytes(Paths.get(localJunitXml)))
+            storage.create(fileBlob, testResult.xmlToString().toByteArray())
         } catch (e: Exception) {
             fatalError(e)
         }
@@ -80,8 +82,14 @@ object GcStorage {
         download(args.testApk)
 
     // junit xml may not exist. ignore error if it doesn't exist
-    fun downloadJunitXml(args: IArgs): String =
-        download(args.junitGcsPath, ignoreError = true)
+    fun downloadJunitXml(args: IArgs): JUnitTestResult? {
+        val oldXmlPath = download(args.junitGcsPath, ignoreError = true)
+        if (oldXmlPath.isNotEmpty()) {
+            return parseIosXml(Paths.get(oldXmlPath))
+        }
+
+        return null
+    }
 
     private fun upload(file: String, fileBytes: ByteArray, rootGcsBucket: String, runGcsPath: String): String {
         val fileName = Paths.get(file).fileName.toString()

test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt

@@ -11,7 +11,6 @@ import ftl.reports.MatrixResultsReport
 import ftl.reports.xml.model.JUnitTestResult
 import ftl.reports.xml.parseAndroidXml
 import ftl.reports.xml.parseIosXml
-import ftl.reports.xml.xmlToString
 import ftl.util.ArtifactRegex
 import ftl.util.resolveLocalRunPath
 import java.io.File
@@ -100,11 +99,10 @@ object ReportManager {
     private fun processJunitXml(newTestResult: JUnitTestResult?, args: IArgs) {
         if (newTestResult == null) return
 
-        val oldXmlPath = GcStorage.downloadJunitXml(args)
-        val oldTestResult = if (oldXmlPath.isNotEmpty()) parseIosXml(oldXmlPath) else null
+        val oldTestResult = GcStorage.downloadJunitXml(args)
 
         newTestResult.mergeTestTimes(oldTestResult)
 
-        GcStorage.uploadJunitXml(newTestResult.xmlToString(), args)
+        GcStorage.uploadJunitXml(newTestResult, args)
     }
 }

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

@@ -1,64 +1,61 @@
 package ftl.shard
 
+import ftl.reports.xml.model.JUnitTestResult
+
 data class TestMethod(
     val name: String,
     val time: Double
 )
 
 data class TestShard(
-    val time: Double,
+    var time: Double,
     val testMethods: MutableList<TestMethod>
 )
 
-data class TestShards(
-    val shards: MutableList<TestShard> = mutableListOf()
-)
-
 object Shard {
 
-    /**
-     * Build shard by removing remaining methods that total to targetShardDuration
-     * At least one method per shard will always be returned, regardless of targetShardDuration.
-     *
-     * remainingMethods must be sorted in order of fastest execution time to slowest.
-     * remainingMethods.sortBy { it.time }
-     *
-     * Based on createConfigurableShard from Flank Java
-     * https://github.com/TestArmada/flank/blob/ceda6d2c3d9eb2a366f19b826e04289cd24bddf3/Flank/src/main/java/com/walmart/otto/shards/ShardCreator.java#L99
-     */
-    fun build(remainingMethods: MutableList<TestMethod>, targetShardDuration: Double): TestShard {
-        var timeBudget = targetShardDuration
-        var shardTime = 0.0
-
-        val testMethods = remainingMethods.iterator()
-        val shardTests = mutableListOf<TestMethod>()
-
-        while (testMethods.hasNext()) {
-            val test = testMethods.next()
-            val testWithinBudget = timeBudget - test.time >= 0
-
-            if (testWithinBudget) {
-                timeBudget -= test.time
-
-                shardTime += test.time
-                shardTests.add(test)
-                testMethods.remove()
-
-                continue
+    // take in the XML with timing info then return list of shards
+    fun calculateShardsByTime(runningTests: List<String>, testResult: JUnitTestResult, maxShards: Int): List<TestShard> {
+
+        val junitMap = mutableMapOf<String, Double>()
+
+        // Create a map with information from previous junit run
+        testResult.testsuites?.forEach { testsuite ->
+            testsuite.testcases?.forEach { testcase ->
+                val key = "${testsuite.name}${testcase.classname}#${testcase.name}".trim()
+                junitMap[key] = testcase.time.toDouble()
             }
+        }
+
+        val testcases = mutableListOf<TestMethod>()
+        runningTests.forEach {
+            // junitMap doesn't include `class `, we remove it to search in the map
+            val key = it.replaceFirst("class ", "")
+            val time = junitMap[key] ?: 10.0
+            testcases.add(TestMethod(it, time))
+        }
 
-            val noTestsAdded = timeBudget == targetShardDuration
-            val testOverBudget = test.time >= timeBudget
+        val testCount = testcases.size
 
-            if (noTestsAdded && testOverBudget) {
-                // Always add at least 1 test per shard regardless of budget
-                shardTime += test.time
-                shardTests.add(test)
-                testMethods.remove()
+        // If maxShards is infinite or we have more shards than tests, let's match it
+        val shardsCount = if (maxShards == -1 || maxShards > testCount) testCount else maxShards
 
-                return TestShard(shardTime, shardTests)
-            }
+        // Create the list of shards we will return
+        var shards = List(shardsCount) { TestShard(0.0, mutableListOf()) }
+
+        // We want to iterate over testcase going from slowest to fastest
+        testcases.sortByDescending { it.time }
+
+        testcases.forEach { testMethod ->
+            val shard = shards.first()
+
+            shard.testMethods.add(testMethod)
+            shard.time += testMethod.time
+
+            // Sort the shards to keep the most empty shard first
+            shards = shards.sortedBy { it.time }
         }
-        return TestShard(shardTime, shardTests)
+
+        return shards
     }
 }

test_runner/src/test/kotlin/ftl/args/AndroidArgsFileTest.kt

@@ -143,8 +143,8 @@ class AndroidArgsFileTest {
         val config = configWithTestMethods(155, testShards = 40)
         with(config) {
             assert(testShards, 40)
-            assert(testShardChunks.size, 39)
-            assert(testShardChunks.first().size, 4)
+            assert(testShardChunks.size, 40)
+            assert(testShardChunks.first().size, 3)
         }
     }