Skip to content

Commit

Permalink
set correct max test shards when arm devices are configured
Browse files Browse the repository at this point in the history
  • Loading branch information
Adam Duke committed Jul 12, 2023
1 parent acba5bc commit 651fe0b
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 10 deletions.
9 changes: 8 additions & 1 deletion test_runner/src/main/kotlin/ftl/args/IArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ interface IArgs {
val inVirtualRange: Boolean
get() = maxTestShards in AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE

val inArmRange: Boolean
get() = maxTestShards in AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE

val defaultTestTime: Double
val defaultClassTestTime: Double
val useAverageTestTimeForNewTests: Boolean
Expand All @@ -87,10 +90,14 @@ interface IArgs {
fun useLocalResultDir() = localResultDir != defaultLocalResultsDir

companion object {
// num_shards must be >= 1, and <= 50
// num_shards must be >= 1, and <= 50 for physical devices
val AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE = 1..50

// num_shards must be >= 1, and <= 500 for non-ARM virtual devices
val AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE = 1..500

// num_shards must be >= 1, and <= 100 for ARM virtual devices
val AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE = 1..100
}

interface ICompanion {
Expand Down
19 changes: 15 additions & 4 deletions test_runner/src/main/kotlin/ftl/args/PrepareAndroidCommonConfig.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package ftl.args
import ftl.client.google.AndroidCatalog
import ftl.config.AndroidConfig
import ftl.config.Device
import ftl.config.containsArmDevices
import ftl.config.containsNonArmDevices
import ftl.config.containsPhysicalDevices
import ftl.config.containsVirtualDevices

Expand All @@ -22,12 +24,21 @@ private fun List<Device>.calculateMaxTestShards(maxTestShards: Int) =
else scaleMaxShardsByDevice(maxTestShards)

private fun List<Device>.getMaxShardsByDevice() =
if (containsPhysicalDevices()) IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last
else IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.last
when {
containsPhysicalDevices() -> IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last
containsArmDevices() -> IArgs.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last
else -> IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.last
}

private fun List<Device>.scaleMaxShardsByDevice(maxTestShards: Int) =
if (containsPhysicalDevices() && containsVirtualDevices()) maxTestShards.scaleToPhysicalShardsCount()
else maxTestShards
when {
containsPhysicalDevices() && containsVirtualDevices() -> maxTestShards.scaleToPhysicalShardsCount()
containsArmDevices() && containsNonArmDevices() -> maxTestShards.scaleToArmShardsCount()
else -> maxTestShards
}

private fun Int.scaleToPhysicalShardsCount() = if (this !in IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE)
IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last else this

private fun Int.scaleToArmShardsCount() = if (this !in IArgs.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE)
IArgs.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last else this
16 changes: 12 additions & 4 deletions test_runner/src/main/kotlin/ftl/args/ValidateAndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import ftl.client.google.SupportedDeviceConfig
import ftl.client.google.UnsupportedModelId
import ftl.client.google.UnsupportedVersionId
import ftl.config.Device
import ftl.config.containsArmDevices
import ftl.config.containsNonArmDevices
import ftl.config.containsPhysicalDevices
import ftl.config.containsVirtualDevices
import ftl.run.exception.FlankConfigurationError
Expand Down Expand Up @@ -171,17 +173,23 @@ private fun AndroidArgs.assertMaxTestShardsByDeviceType() =
}

private fun AndroidArgs.assertDevicesShards() {
if (inVirtualRange && !inPhysicalRange) logLn("Physical devices configured, but max-test-shards limit set to $maxTestShards, for physical devices range is ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} to ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}, you additionally have configured virtual devices. In this case, the physical limit will be decreased to: ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}")
else if (!inVirtualRange && !inPhysicalRange) throwMaxTestShardsLimitExceeded()
when {
inVirtualRange && !inPhysicalRange -> logLn("Physical devices configured, but max-test-shards limit set to $maxTestShards. For physical devices range is ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} to ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}, you additionally have configured virtual devices. In this case, the maximum shards will be decreased to: ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}")
!inVirtualRange && !inPhysicalRange -> throwMaxTestShardsLimitExceeded()
}
}

private fun AndroidArgs.assertVirtualDevicesShards() {
if (!inVirtualRange) throwMaxTestShardsLimitExceeded()
when {
devices.containsArmDevices() && devices.containsNonArmDevices() -> logLn("Arm devices configured, but max-test-shards limit set to $maxTestShards. For Arm devices, the range is ${IArgs.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.first} to ${IArgs.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last}, you additionally have configured other virtual devices. In this case, the maximum shards will be decreased to: ${IArgs.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last}")
devices.containsArmDevices() && !inArmRange -> throwMaxTestShardsLimitExceeded()
!inVirtualRange -> throwMaxTestShardsLimitExceeded()
}
}

private fun AndroidArgs.throwMaxTestShardsLimitExceeded(): Nothing {
throw FlankConfigurationError(
"max-test-shards must be >= ${IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.last} for virtual devices, for physical devices max-test-shards must be >= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last}, or -1. But current is $maxTestShards"
"max-test-shards must be >= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE.last} if there are physical devices configured, >= ${IArgs.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last} if there are Arm devices configured, >= ${IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.last} if there are only non-Arm virtual devices configured, or -1. Current configuration value is $maxTestShards"
)
}

Expand Down
4 changes: 4 additions & 0 deletions test_runner/src/main/kotlin/ftl/config/Device.kt
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,7 @@ fun Map<String, String>.asDevice(android: Boolean) =
fun List<Device>.containsVirtualDevices() = any { it.isVirtual }

fun List<Device>.containsPhysicalDevices() = any { !it.isVirtual }

fun List<Device>.containsArmDevices() = any { it.model.endsWith(".arm") }

fun List<Device>.containsNonArmDevices() = any { !it.model.endsWith(".arm") }
96 changes: 95 additions & 1 deletion test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ftl.args
import com.google.common.truth.Truth.assertThat
import com.google.api.services.testing.model.TestSpecification
import ftl.args.IArgs.Companion.AVAILABLE_PHYSICAL_SHARD_COUNT_RANGE
import ftl.args.IArgs.Companion.AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE
import ftl.args.IArgs.Companion.AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE
import ftl.args.yml.AppTestPair
import ftl.args.yml.Type
Expand Down Expand Up @@ -554,6 +555,36 @@ AndroidArgs
}
}

@Test
fun negativeOneTestShardsWithArmDevice() {
val androidArgs = AndroidArgs.load(
"""
gcloud:
app: $appApk
test: $testErrorApk
device:
- model: NexusLowRes
version: 23
locale: en
orientation: portrait
- model: SmallPhone.arm
version: 30
locale: en
orientation: portrait
flank:
max-test-shards: -1
"""
)

val testShardChunks = getAndroidShardChunks(androidArgs)
with(androidArgs) {
assert(maxTestShards, AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last)
assert(testShardChunks.size, 2)
testShardChunks.forEach { chunk -> assert(chunk.size, 1) }
}
}

@Test
fun `androidArgs emptyFlank`() {
val androidArgs = AndroidArgs.load(
Expand Down Expand Up @@ -1824,6 +1855,9 @@ AndroidArgs
assertTrue(testSpecification.androidInstrumentationTest.testTargets.isNotEmpty())
}

// This and the "should limit shards to virtual limit if only virtual device configured"
// test seem to basically be testing the same behavior as well as the
// negativeOneTestShards test?
@Test
fun `if set max-test-shards to -1 should give maximum amount`() {
val yaml = """
Expand Down Expand Up @@ -1865,7 +1899,7 @@ AndroidArgs
}

@Test
fun `should limit shards to virtual if only virtual device configured`() {
fun `should limit shards to virtual limit if only virtual device configured`() {
val yaml = """
gcloud:
app: $appApk
Expand All @@ -1878,6 +1912,25 @@ AndroidArgs
assertEquals(AVAILABLE_VIRTUAL_SHARD_COUNT_RANGE.last, args.maxTestShards)
}

@Test
fun `should limit shards to Arm limit if any Arm device and no physical device configured `() {
val yaml = """
gcloud:
app: $appApk
test: $testApk
device:
- model: SmallPhone.arm
version: 30
locale: en
orientation: portrait
flank:
max-test-shards: -1
disable-results-upload: true
""".trimIndent()
val args = AndroidArgs.load(yaml).validate()
assertEquals(AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last, args.maxTestShards)
}

@Test
fun `should not set shards count when maxTestShards != -1`() {
val yaml = """
Expand Down Expand Up @@ -1905,6 +1958,47 @@ AndroidArgs
AndroidArgs.load(yaml).validate()
}

@Test(expected = FlankConfigurationError::class)
fun `should throw when maximum test shards for Arm devices limit exceeded`() {
val yaml = """
gcloud:
app: $appApk
test: $testApk
device:
- model: SmallPhone.arm
version: 30
locale: en
orientation: portrait
flank:
max-test-shards: ${AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last + 1}
disable-results-upload: true
""".trimIndent()
AndroidArgs.load(yaml).validate()
}

@Test
fun `should not throw when maximum test shards for Arm devices limit exceeded and non-Arm devices configured`() {
val yaml = """
gcloud:
app: $appApk
test: $testApk
device:
- model: SmallPhone.arm
version: 30
locale: en
orientation: portrait
device:
- model: NexusLowRes
version: 23
locale: en
orientation: portrait
flank:
max-test-shards: ${AVAILABLE_VIRTUAL_ARM_SHARD_COUNT_RANGE.last + 1}
disable-results-upload: true
""".trimIndent()
AndroidArgs.load(yaml).validate()
}

@Test
fun `should not throw an error if validation is disabled (yml) -- max test shards`() {
val yaml = """
Expand Down

0 comments on commit 651fe0b

Please sign in to comment.