Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set correct max test shards when arm devices are configured #2404

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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