Issue #428: Smart Flank - print how accurate test times are

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

@@ -9,12 +9,14 @@ import ftl.reports.HtmlErrorReport
 import ftl.reports.JUnitReport
 import ftl.reports.MatrixResultsReport
 import ftl.reports.xml.model.JUnitTestResult
-import ftl.reports.xml.parseOneSuiteXml
 import ftl.reports.xml.parseAllSuitesXml
+import ftl.reports.xml.parseOneSuiteXml
+import ftl.shard.Shard
 import ftl.util.ArtifactRegex
 import ftl.util.resolveLocalRunPath
 import java.io.File
 import java.nio.file.Paths
+import kotlin.math.roundToInt
 
 object ReportManager {
 
@@ -96,13 +98,53 @@ object ReportManager {
         return matrices.exitCode()
     }
 
+    data class ShardEfficiency(
+        val expectedTime: Double,
+        val finalTime: Double,
+        val efficiency: Double
+    )
+
+    fun createShardEfficiencyList(oldResult: JUnitTestResult, newResult: JUnitTestResult, args: IArgs):
+            List<ShardEfficiency> {
+        val oldJunitMap = Shard.createJunitMap(oldResult, args)
+        val newJunitMap = Shard.createJunitMap(newResult, args)
+
+        val timeList = mutableListOf<ShardEfficiency>()
+        args.testShardChunks.forEach { testSuite ->
+
+            var expectedTime = 0.0
+            var finalTime = 0.0
+            testSuite.forEach { testCase ->
+                expectedTime += oldJunitMap[testCase] ?: 0.0
+                finalTime += newJunitMap[testCase] ?: 0.0
+            }
+
+            val efficiency = 100 - (expectedTime * 100.0 / finalTime)
+            timeList.add(ShardEfficiency(expectedTime, finalTime, efficiency))
+        }
+
+        return timeList
+    }
+
+    private fun printActual(oldResult: JUnitTestResult, newResult: JUnitTestResult, args: IArgs) {
+        val list = createShardEfficiencyList(oldResult, newResult, args)
+
+        println("  Actual shard times: " + list.joinToString(", ") {
+            "${it.finalTime.roundToInt()}s (${it.efficiency.roundToInt()}%)"
+        } + "\n")
+    }
+
     private fun processJunitXml(newTestResult: JUnitTestResult?, args: IArgs) {
         if (newTestResult == null) return
 
         val oldTestResult = GcStorage.downloadJunitXml(args)
 
         newTestResult.mergeTestTimes(oldTestResult)
 
+        if (oldTestResult != null) {
+            printActual(newTestResult, oldTestResult, args)
+        }
+
         GcStorage.uploadJunitXml(newTestResult, args)
     }
 }

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

@@ -59,21 +59,9 @@ object Shard {
         args: IArgs
     ): List<TestShard> {
         val maxShards = args.testShards
-        val android = args is AndroidArgs
-        val junitMap = mutableMapOf<String, Double>()
-
-        // Create a map with information from previous junit run
-        oldTestResult.testsuites?.forEach { testsuite ->
-            testsuite.testcases?.forEach { testcase ->
-                if (!testcase.empty() && testcase.time != null) {
-                    val key = if (android) testcase.androidKey() else testcase.iosKey()
-                    junitMap[key] = testcase.time.toDouble()
-                }
-            }
-        }
+        val junitMap = createJunitMap(oldTestResult, args)
 
         var cacheMiss = 0
-        // junitMap doesn't include `class `, we remove it to search in the map
         val testcases = mutableListOf<TestMethod>()
 
         testsToRun.forEach { key ->
@@ -117,4 +105,20 @@ object Shard {
 
         return shards
     }
+
+    fun createJunitMap(junitResult: JUnitTestResult, args: IArgs): Map<String, Double> {
+        val junitMap = mutableMapOf<String, Double>()
+
+        // Create a map with information from previous junit run
+        junitResult.testsuites?.forEach { testsuite ->
+            testsuite.testcases?.forEach { testcase ->
+                if (!testcase.empty() && testcase.time != null) {
+                    val key = if (args is AndroidArgs) testcase.androidKey() else testcase.iosKey()
+                    junitMap[key] = testcase.time.toDouble()
+                }
+            }
+        }
+
+        return junitMap
+    }
 }

test_runner/src/test/kotlin/ftl/reports/utils/ReportManagerTest.kt

@@ -1,7 +1,11 @@
 package ftl.reports.utils
 
+import com.google.common.truth.Truth.assertThat
 import ftl.args.AndroidArgs
 import ftl.reports.util.ReportManager
+import ftl.reports.xml.model.JUnitTestCase
+import ftl.reports.xml.model.JUnitTestResult
+import ftl.reports.xml.model.JUnitTestSuite
 import ftl.run.TestRunner
 import ftl.test.util.FlankTestRunner
 import org.junit.Rule
@@ -38,4 +42,38 @@ class ReportManagerTest {
         `when`(mockArgs.smartFlankGcsPath).thenReturn("")
         ReportManager.generate(matrix, mockArgs)
     }
+
+    @Test
+    fun createShardEfficiencyListTest() {
+        val oldRunTestCases = mutableListOf(
+            JUnitTestCase("a", "a", "10.0"),
+            JUnitTestCase("b", "b", "20.0"),
+            JUnitTestCase("c", "c", "30.0")
+        )
+        val oldRunSuite = JUnitTestSuite("", "-1", "-1", "-1", "-1", "-1", "-1", "-1", oldRunTestCases, null, null, null)
+        val oldTestResult = JUnitTestResult(mutableListOf(oldRunSuite))
+
+        val newRunTestCases = mutableListOf(
+            JUnitTestCase("a", "a", "9.0"),
+            JUnitTestCase("b", "b", "21.0"),
+            JUnitTestCase("c", "c", "30.0")
+        )
+        val newRunSuite = JUnitTestSuite("", "-1", "-1", "-1", "-1", "-1", "-1", "-1", newRunTestCases, null, null, null)
+        val newTestResult = JUnitTestResult(mutableListOf(newRunSuite))
+
+        val mockArgs = mock(AndroidArgs::class.java)
+
+        `when`(mockArgs.testShardChunks).thenReturn(listOf(listOf("class a#a"), listOf("class b#b"), listOf("class c#c")))
+        val result = ReportManager.createShardEfficiencyList(oldTestResult, newTestResult, mockArgs)
+
+        // We can do a better assertion when we decide on the efficiency calculation
+        assertThat(result[0].expectedTime).isEqualTo(10.0)
+        assertThat(result[0].finalTime).isEqualTo(9.0)
+
+        assertThat(result[1].expectedTime).isEqualTo(20.0)
+        assertThat(result[1].finalTime).isEqualTo(21.0)
+
+        assertThat(result[2].expectedTime).isEqualTo(30.0)
+        assertThat(result[2].finalTime).isEqualTo(30.0)
+    }
 }