Skip to content

Commit

Permalink
#707 Refactor Shard object by splitting it into smaller functions (#799)
Browse files Browse the repository at this point in the history
* #707 Refactor Shared object by splitting it into smaller functions

* #707 Updated Release notes

* #707 Refactor Shared object by splitting it into smaller functions

* #707 Refactor Shared object by splitting it into smaller functions

* #707 Refactor Shared object by splitting it into smaller functions

* #707 fixed PR comments

* #707 fixed PR comments

* #707 Refactor Shared object by splitting it into smaller functions

* #707 Refactor Shared object by splitting it into smaller functions

* #707 Refactor Shared object by splitting it into smaller functions

* #707 Refactor Shared object by splitting it into smaller functions

* #707 Refactor Shared object by splitting it into smaller functions

* #707 Refactor Shared object by splitting it into smaller functions

* #707 Refactor Shared object by splitting it into smaller functions
  • Loading branch information
piotradamczyk5 authored May 25, 2020
1 parent e95ac5c commit c5e7608
Show file tree
Hide file tree
Showing 14 changed files with 408 additions and 245 deletions.
1 change: 1 addition & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- [#807](https://github.com/Flank/flank/issues/807) Fix Bugsnag being initialized during tests. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#805](https://github.com/Flank/flank/pull/805) Fix overlapping results. ([pawelpasterz](https://github.com/pawelpasterz))
- [#812](https://github.com/Flank/flank/issues/812) Convert bitrise macOS workflow to github action. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#799](https://github.com/Flank/flank/pull/799) Refactor Shared object by splitting it into smaller functions. ([piotradamczyk5](https://github.com/piotradamczyk5))

## v20.05.2

Expand Down
7 changes: 4 additions & 3 deletions test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import ftl.config.FtlConstants.useMock
import ftl.gc.GcStorage
import ftl.gc.GcToolResults
import ftl.reports.xml.model.JUnitTestResult
import ftl.shard.Shard
import ftl.shard.StringShards
import ftl.shard.createShardsByShardCount
import ftl.shard.shardCountByTime
import ftl.shard.stringShards
import ftl.util.FlankFatalError
import ftl.util.FlankTestMethod
Expand Down Expand Up @@ -238,8 +239,8 @@ object ArgsHelper {
listOf(filteredTests.map { it.testName }.toMutableList())
} else {
val oldTestResult = GcStorage.downloadJunitXml(args) ?: JUnitTestResult(mutableListOf())
val shardCount = forcedShardCount ?: Shard.shardCountByTime(filteredTests, oldTestResult, args)
Shard.createShardsByShardCount(filteredTests, oldTestResult, args, shardCount).stringShards()
val shardCount = forcedShardCount ?: shardCountByTime(filteredTests, oldTestResult, args)
createShardsByShardCount(filteredTests, oldTestResult, args, shardCount).stringShards()
}

return testMethodsAlwaysRun(shards, args)
Expand Down
6 changes: 3 additions & 3 deletions test_runner/src/main/kotlin/ftl/reports/util/ReportManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import ftl.reports.api.processXmlFromApi
import ftl.reports.xml.model.JUnitTestResult
import ftl.reports.xml.parseAllSuitesXml
import ftl.reports.xml.parseOneSuiteXml
import ftl.shard.Shard
import ftl.shard.createTestMethodDurationMap
import ftl.util.Artifacts
import ftl.util.resolveLocalRunPath
import java.io.File
Expand Down Expand Up @@ -137,8 +137,8 @@ object ReportManager {
args: IArgs,
testShardChunks: ShardChunks
): List<ShardEfficiency> {
val oldDurations = Shard.createTestMethodDurationMap(oldResult, args)
val newDurations = Shard.createTestMethodDurationMap(newResult, args)
val oldDurations = createTestMethodDurationMap(oldResult, args)
val newDurations = createTestMethodDurationMap(newResult, args)

return testShardChunks.mapIndexed { index, testSuite ->

Expand Down
21 changes: 21 additions & 0 deletions test_runner/src/main/kotlin/ftl/shard/CacheReport.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package ftl.shard

import ftl.util.FlankTestMethod
import kotlin.math.roundToInt

fun printCacheInfo(testsToRun: List<FlankTestMethod>, previousMethodDurations: Map<String, Double>) {
val allTestCount = testsToRun.size
val cacheHit = cacheHit(allTestCount, calculateCacheMiss(testsToRun, previousMethodDurations))
val cachePercent = cachePercent(allTestCount, cacheHit)
println()
println(" Smart Flank cache hit: ${cachePercent.roundToInt()}% ($cacheHit / $allTestCount)")
}

private fun cacheHit(allTestCount: Int, cacheMiss: Int) = allTestCount - cacheMiss

private fun calculateCacheMiss(testsToRun: List<FlankTestMethod>, previousMethodDurations: Map<String, Double>): Int {
return testsToRun.count { !previousMethodDurations.containsKey(it.testName) }
}

private fun cachePercent(allTestCount: Int, cacheHit: Int): Double =
if (allTestCount == 0) 0.0 else cacheHit.toDouble() / allTestCount * 100.0
190 changes: 57 additions & 133 deletions test_runner/src/main/kotlin/ftl/shard/Shard.kt
Original file line number Diff line number Diff line change
@@ -1,27 +1,16 @@
package ftl.shard

import com.google.common.annotations.VisibleForTesting
import ftl.args.AndroidArgs
import ftl.args.IArgs
import ftl.args.IosArgs
import ftl.reports.xml.model.JUnitTestCase
import ftl.reports.xml.model.JUnitTestResult
import ftl.util.FlankTestMethod
import ftl.util.FlankFatalError
import kotlin.math.ceil
import kotlin.math.min
import ftl.util.FlankTestMethod
import kotlin.math.roundToInt

data class TestMethod(
val name: String,
val time: Double
)

data class TestShard(
var time: Double,
val testMethods: MutableList<TestMethod>
)

/** List of shards containing test names as a string. */
typealias StringShards = List<MutableList<String>>

Expand All @@ -45,137 +34,72 @@ class com.foo.ClassName#testMethodToSkip
*/

object Shard {
// When a test does not have previous results to reference, fall back to this run time.
@VisibleForTesting
const val DEFAULT_TEST_TIME_SEC = 120.0
private const val IGNORE_TEST_TIME = 0.0

private fun JUnitTestCase.androidKey(): String {
return "class $classname#$name"
}

private fun JUnitTestCase.iosKey(): String {
// FTL iOS XML appends `()` to each test name. ex: `testBasicSelection()`
// xctestrun file requires classname/name with no `()`
val testName = name?.substringBefore('(')
return "$classname/$testName"
}

// take in the XML with timing info then return the shard count based on execution time
fun shardCountByTime(
testsToRun: List<FlankTestMethod>,
oldTestResult: JUnitTestResult,
args: IArgs
): Int {
if (args.shardTime == -1) return -1
if (args.shardTime < -1 || args.shardTime == 0) throw FlankFatalError("Invalid shard time ${args.shardTime}")

val oldDurations = createTestMethodDurationMap(oldTestResult, args)
val testsTotalTime = testsToRun.sumByDouble { if (it.ignored) IGNORE_TEST_TIME else oldDurations[it.testName] ?: DEFAULT_TEST_TIME_SEC }

val shardsByTime = ceil(testsTotalTime / args.shardTime).toInt()

// Use a single shard unless total test time is greater than shardTime.
if (testsTotalTime <= args.shardTime) {
return 1
}
// take in the XML with timing info then return list of shards based on the amount of shards to use
fun createShardsByShardCount(
testsToRun: List<FlankTestMethod>,
oldTestResult: JUnitTestResult,
args: IArgs,
forcedShardCount: Int = -1
): List<TestShard> {
if (forcedShardCount < -1 || forcedShardCount == 0) throw FlankFatalError("Invalid forcedShardCount value $forcedShardCount")
val maxShards = maxShards(args.maxTestShards, forcedShardCount)

// If there is no limit, use the calculated amount
if (args.maxTestShards == -1) {
return shardsByTime
}
val previousMethodDurations = createTestMethodDurationMap(oldTestResult, args)
val testCases = createTestCases(testsToRun, previousMethodDurations)
.sortedByDescending(TestMethod::time) // We want to iterate over testcase going from slowest to fastest

val testCount = getNumberOfNotIgnoredTestCases(testCases)

// We need to respect the maxTestShards
val shardCount = min(shardsByTime, args.maxTestShards)

if (shardCount <= 0) throw FlankFatalError("Invalid shard count $shardCount")
return shardCount
}

// take in the XML with timing info then return list of shards based on the amount of shards to use
fun createShardsByShardCount(
testsToRun: List<FlankTestMethod>,
oldTestResult: JUnitTestResult,
args: IArgs,
forcedShardCount: Int = -1
): List<TestShard> {
if (forcedShardCount < -1 || forcedShardCount == 0) throw FlankFatalError("Invalid forcedShardCount value $forcedShardCount")

val maxShards = if (forcedShardCount == -1) args.maxTestShards else forcedShardCount
val previousMethodDurations = createTestMethodDurationMap(oldTestResult, args)

var cacheMiss = 0
val testCases: List<TestMethod> = testsToRun
.map {
TestMethod(
name = it.testName,
time = if (it.ignored) IGNORE_TEST_TIME else previousMethodDurations[it.testName] ?: DEFAULT_TEST_TIME_SEC.also {
cacheMiss += 1
}
)
}
// We want to iterate over testcase going from slowest to fastest
.sortedByDescending(TestMethod::time)

// Ugly hotfix for case when all test cases are annotated with @Ignore
// we need to filter them because they have time == 0.0 which cause empty shards creation, few lines later
// and we don't need additional shards for ignored tests.
val testCount =
if (testCases.isEmpty()) 0
else testCases.filter { it.time > 0.0 }.takeIf { it.isNotEmpty() }?.size ?: 1

// 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

// Create the list of shards we will return
if (shardsCount <= 0) {
val platform = if (args is IosArgs) "ios" else "android"
throw FlankFatalError(
"""Invalid shard count. To debug try: flank $platform run --dump-shards
// If maxShards is infinite or we have more shards than tests, let's match it
val shardsCount = matchNumberOfShardsWithTestCount(maxShards, testCount)

// Create the list of shards we will return
if (shardsCount <= 0) throw FlankFatalError(
"""Invalid shard count. To debug try: flank ${args.platformName} run --dump-shards
| args.maxTestShards: ${args.maxTestShards}
| forcedShardCount: $forcedShardCount
| testCount: $testCount
| maxShards: $maxShards
| shardsCount: $shardsCount""".trimMargin()
)
}
var shards = List(shardsCount) { TestShard(0.0, mutableListOf()) }
)

testCases.forEach { testMethod ->
val shard = shards.first()
val shards = createShardsForTestCases(testCases, shardsCount)

shard.testMethods.add(testMethod)
shard.time += testMethod.time
printCacheInfo(testsToRun, previousMethodDurations)
printShardsInfo(shards)
return shards
}

// Sort the shards to keep the most empty shard first
shards = shards.sortedBy { it.time }
}
private fun maxShards(maxShardsCount: Int, forcedShardCount: Int) =
if (forcedShardCount == -1) maxShardsCount else forcedShardCount

val allTests = testsToRun.size // zero when test targets is empty
val cacheHit = allTests - cacheMiss
val cachePercent = if (allTests == 0) 0.0 else cacheHit.toDouble() / allTests * 100.0
println()
println(" Smart Flank cache hit: ${cachePercent.roundToInt()}% ($cacheHit / $allTests)")
println(" Shard times: " + shards.joinToString(", ") { "${it.time.roundToInt()}s" } + "\n")

return shards
}

fun createTestMethodDurationMap(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()
val time = testcase.time.toDouble()
if (time >= 0) junitMap[key] = time
}
}
}
private fun getNumberOfNotIgnoredTestCases(testCases: List<TestMethod>): Int {
// Ugly hotfix for case when all test cases are annotated with @Ignore
// we need to filter them because they have time == 0.0 which cause empty shards creation, few lines later
// and we don't need additional shards for ignored tests.
return if (testCases.isEmpty()) 0 else testCases.filter { it.time > IGNORE_TEST_TIME }
.takeIf { it.isNotEmpty() }?.size ?: 1
}

private fun matchNumberOfShardsWithTestCount(maxShards: Int, testCount: Int) =
if (maxShards == -1 || maxShards > testCount) testCount else maxShards

return junitMap
}
private val IArgs.platformName get() = if (this is IosArgs) "ios" else "android"

private fun printShardsInfo(shards: List<TestShard>) {
println(" Shard times: " + shards.joinToString(", ") { "${it.time.roundToInt()}s" } + "\n")
}

private fun createShardsForTestCases(
testCases: List<TestMethod>,
shardsCount: Int
): List<TestShard> = testCases.fold(
initial = List(shardsCount) { TestShard(0.0, mutableListOf()) }
) { shards, testMethod ->
shards.apply {
first().apply {
testMethods += testMethod
time += testMethod.time
}
}.sortedBy(TestShard::time)
}
53 changes: 53 additions & 0 deletions test_runner/src/main/kotlin/ftl/shard/ShardCount.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package ftl.shard

import ftl.args.IArgs
import ftl.args.IArgs.Companion.AVAILABLE_SHARD_COUNT_RANGE
import ftl.reports.xml.model.JUnitTestResult
import ftl.util.FlankFatalError
import ftl.util.FlankTestMethod
import kotlin.math.ceil
import kotlin.math.min

private const val SINGLE_SHARD = 1
private const val NO_LIMIT = -1

// take in the XML with timing info then return the shard count based on execution time
fun shardCountByTime(
testsToRun: List<FlankTestMethod>,
oldTestResult: JUnitTestResult,
args: IArgs
): Int = when {
args.shardTime == NO_LIMIT -> NO_LIMIT
args.shardTime < NO_LIMIT || args.shardTime == 0 -> throw FlankFatalError("Invalid shard time ${args.shardTime}")
else -> calculateShardCount(testsToRun, oldTestResult, args)
}

private fun calculateShardCount(
testsToRun: List<FlankTestMethod>,
oldTestResult: JUnitTestResult,
args: IArgs
): Int = calculateShardCount(
args = args,
testsTotalTime = testTotalTime(testsToRun, createTestMethodDurationMap(oldTestResult, args)),
testsToRunCount = testsToRun.size
)

private fun testTotalTime(testsToRun: List<FlankTestMethod>, previousMethodDurations: Map<String, Double>): Double =
testsToRun.sumByDouble { flankTestMethod -> getTestMethodTime(flankTestMethod, previousMethodDurations) }

private fun calculateShardCount(
args: IArgs,
testsTotalTime: Double,
testsToRunCount: Int
): Int = when {
testsTotalTime <= args.shardTime -> SINGLE_SHARD
args.maxTestShards == NO_LIMIT -> min(AVAILABLE_SHARD_COUNT_RANGE.last, testsToRunCount)
else -> shardCount(testsTotalTime, args).also { count ->
if (count <= 0) throw FlankFatalError("Invalid shard count $count")
}
}

private fun shardCount(testsTotalTime: Double, args: IArgs) =
min(shardsByTime(testsTotalTime, args), args.maxTestShards)

private fun shardsByTime(testsTotalTime: Double, args: IArgs) = ceil(testsTotalTime / args.shardTime).toInt()
12 changes: 12 additions & 0 deletions test_runner/src/main/kotlin/ftl/shard/TestCasesCreator.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package ftl.shard

import ftl.util.FlankTestMethod

data class TestMethod(
val name: String,
val time: Double
)

fun createTestCases(testsToRun: List<FlankTestMethod>, previousMethodDurations: Map<String, Double>): List<TestMethod> {
return testsToRun.map { TestMethod(it.testName, getTestMethodTime(it, previousMethodDurations)) }
}
35 changes: 35 additions & 0 deletions test_runner/src/main/kotlin/ftl/shard/TestMethodDuration.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package ftl.shard

import ftl.args.AndroidArgs
import ftl.args.IArgs
import ftl.reports.xml.model.JUnitTestCase
import ftl.reports.xml.model.JUnitTestResult

fun createTestMethodDurationMap(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()
val time = testcase.time.toDouble()
if (time >= 0) junitMap[key] = time
}
}
}

return junitMap
}

private fun JUnitTestCase.androidKey(): String {
return "class $classname#$name"
}

private fun JUnitTestCase.iosKey(): String {
// FTL iOS XML appends `()` to each test name. ex: `testBasicSelection()`
// xctestrun file requires classname/name with no `()`
val testName = name?.substringBefore('(')
return "$classname/$testName"
}
Loading

0 comments on commit c5e7608

Please sign in to comment.