From d0dc318e7726bab493670b7a4b846b67d3871f5f Mon Sep 17 00:00:00 2001 From: Carlos Sessa Date: Tue, 11 Dec 2018 11:13:58 -0600 Subject: [PATCH] Issue #428: Smart Flank - print how accurate test times are --- .../kotlin/ftl/reports/util/ReportManager.kt | 42 +++++++++++++++++++ .../src/main/kotlin/ftl/shard/Shard.kt | 30 +++++++------ .../ftl/reports/utils/ReportManagerTest.kt | 38 +++++++++++++++++ 3 files changed, 97 insertions(+), 13 deletions(-) diff --git a/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt b/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt index 8a221d647f..ff192ccc18 100644 --- a/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt +++ b/test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt @@ -11,10 +11,12 @@ import ftl.reports.MatrixResultsReport import ftl.reports.xml.model.JUnitTestResult import ftl.reports.xml.parseAllSuitesXml import ftl.reports.xml.parseOneSuiteXml +import ftl.shard.Shard import ftl.util.Artifacts import ftl.util.resolveLocalRunPath import java.io.File import java.nio.file.Paths +import kotlin.math.roundToInt object ReportManager { @@ -96,6 +98,42 @@ 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 { + val oldJunitMap = Shard.createJunitMap(oldResult, args) + val newJunitMap = Shard.createJunitMap(newResult, args) + + val timeList = mutableListOf() + 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 @@ -103,6 +141,10 @@ object ReportManager { newTestResult.mergeTestTimes(oldTestResult) + if (oldTestResult != null) { + printActual(newTestResult, oldTestResult, args) + } + GcStorage.uploadJunitXml(newTestResult, args) } } diff --git a/test_runner/src/main/kotlin/ftl/shard/Shard.kt b/test_runner/src/main/kotlin/ftl/shard/Shard.kt index ffa1c2482d..e0226fa78b 100644 --- a/test_runner/src/main/kotlin/ftl/shard/Shard.kt +++ b/test_runner/src/main/kotlin/ftl/shard/Shard.kt @@ -59,21 +59,9 @@ object Shard { args: IArgs ): List { val maxShards = args.testShards - val android = args is AndroidArgs - val junitMap = mutableMapOf() - - // 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() testsToRun.forEach { key -> @@ -117,4 +105,20 @@ object Shard { return shards } + + fun createJunitMap(junitResult: JUnitTestResult, args: IArgs): Map { + val junitMap = mutableMapOf() + + // 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 + } } diff --git a/test_runner/src/test/kotlin/ftl/reports/utils/ReportManagerTest.kt b/test_runner/src/test/kotlin/ftl/reports/utils/ReportManagerTest.kt index 2aac045336..66ca200cb7 100644 --- a/test_runner/src/test/kotlin/ftl/reports/utils/ReportManagerTest.kt +++ b/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) + } }