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

#824 Automatically convert -1 in maximum-test-shards to the maximum shard amount #825

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
3 changes: 1 addition & 2 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@
- [#798](https://github.com/Flank/flank/pull/798) Remove failure nodes from tests that passed on retry so that Jenkins JUnit plugin marks them as successful. ([adamfilipow92](https://github.com/adamfilipow92))
- [#822](https://github.com/Flank/flank/pull/822) Allow runtime test discovery when sharding is disabled by not setting test-targets. This unblocks cucumber testing. ([adamfilipow92](https://github.com/adamfilipow92))
- [#819](https://github.com/Flank/flank/pull/819) Display matrix results in a table format. ([piotradamczyk5](https://github.com/piotradamczyk5))
- [#825](https://github.com/Flank/flank/pull/825) Automatically convert -1 in maximum-test-shards to the maximum shard amount. ([adamfilipow92](https://github.com/adamfilipow92))
-
-
-

## v20.05.2

Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class AndroidArgs(
val devices = cli?.device ?: androidGcloud.device

private val flank = flankYml.flank
override val maxTestShards = cli?.maxTestShards ?: flank.maxTestShards
override val maxTestShards = convertToShardCount(cli?.maxTestShards ?: flank.maxTestShards)
override val shardTime = cli?.shardTime ?: flank.shardTime
override val repeatTests = cli?.repeatTests ?: flank.repeatTests
override val smartFlankGcsPath = cli?.smartFlankGcsPath ?: flank.smartFlankGcsPath
Expand Down
5 changes: 3 additions & 2 deletions test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.google.cloud.storage.BucketInfo
import com.google.cloud.storage.Storage
import com.google.cloud.storage.StorageClass
import com.google.cloud.storage.StorageOptions
import ftl.args.IArgs.Companion.AVAILABLE_SHARD_COUNT_RANGE
import ftl.args.yml.IYmlMap
import ftl.args.yml.YamlObjectMapper
import ftl.config.FtlConstants
Expand Down Expand Up @@ -64,8 +65,8 @@ object ArgsHelper {
" See https://github.com/GoogleCloudPlatform/google-cloud-java#specifying-a-project-id"
)

if (args.maxTestShards !in IArgs.AVAILABLE_SHARD_COUNT_RANGE && args.maxTestShards != -1)
throw FlankFatalError("max-test-shards must be >= 1 and <= 50, or -1. But current is ${args.maxTestShards}")
if (args.maxTestShards !in AVAILABLE_SHARD_COUNT_RANGE && args.maxTestShards != -1)
throw FlankFatalError("max-test-shards must be >= ${AVAILABLE_SHARD_COUNT_RANGE.first} and <= ${AVAILABLE_SHARD_COUNT_RANGE.last}. But current is ${args.maxTestShards}")

if (args.shardTime <= 0 && args.shardTime != -1) throw FlankFatalError("shard-time must be >= 1 or -1")
if (args.repeatTests < 1) throw FlankFatalError("num-test-runs must be >= 1")
Expand Down
6 changes: 6 additions & 0 deletions test_runner/src/main/kotlin/ftl/args/IArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ interface IArgs {

fun useLocalResultDir() = localResultDir != FlankYmlParams.defaultLocalResultsDir

fun convertToShardCount(inputValue: Int): Int = if (inputValue == -1) {
AVAILABLE_SHARD_COUNT_RANGE.last
} else {
inputValue
}

companion object {
// num_shards must be >= 1, and <= 50
val AVAILABLE_SHARD_COUNT_RANGE = 1..50
Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/main/kotlin/ftl/args/IosArgs.kt
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class IosArgs(
val devices = cli?.device ?: iosGcloud.device

private val flank = flankYml.flank
override val maxTestShards = cli?.maxTestShards ?: flank.maxTestShards
override val maxTestShards = convertToShardCount(cli?.maxTestShards ?: flank.maxTestShards)
override val shardTime = cli?.shardTime ?: flank.shardTime
override val repeatTests = cli?.repeatTests ?: flank.repeatTests
override val smartFlankGcsPath = cli?.smartFlankGcsPath ?: flank.smartFlankGcsPath
Expand Down
24 changes: 19 additions & 5 deletions test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ftl.args

import com.google.api.services.testing.model.TestSpecification
import com.google.common.truth.Truth.assertThat
import ftl.args.IArgs.Companion.AVAILABLE_SHARD_COUNT_RANGE
import ftl.args.yml.AppTestPair
import ftl.cli.firebase.test.android.AndroidRunCommand
import ftl.config.Device
Expand Down Expand Up @@ -462,7 +463,7 @@ AndroidArgs

val testShardChunks = getAndroidShardChunks(androidArgs)
with(androidArgs) {
assert(maxTestShards, -1)
assert(maxTestShards, AVAILABLE_SHARD_COUNT_RANGE.last)
assert(testShardChunks.size, 2)
testShardChunks.forEach { chunk -> assert(chunk.size, 1) }
}
Expand Down Expand Up @@ -670,7 +671,7 @@ AndroidArgs

@Test
fun `cli numUniformShards`() {
val expected = 50
val expected = AVAILABLE_SHARD_COUNT_RANGE.last
val cli = AndroidRunCommand()
CommandLine(cli).parseArgs("--num-uniform-shards=$expected")

Expand All @@ -691,9 +692,9 @@ AndroidArgs
gcloud:
app: $appApk
test: $testApk
num-uniform-shards: 50
num-uniform-shards: ${AVAILABLE_SHARD_COUNT_RANGE.last}
flank:
max-test-shards: 50
max-test-shards: ${AVAILABLE_SHARD_COUNT_RANGE.last}
"""
AndroidArgs.load(yaml)
}
Expand Down Expand Up @@ -1346,7 +1347,7 @@ AndroidArgs
" num-flaky-test-attempts: 3",
"""
flank:
max-test-shards: 50
max-test-shards: ${AVAILABLE_SHARD_COUNT_RANGE.last}
""".trimIndent(),
"""
flank:
Expand Down Expand Up @@ -1414,6 +1415,19 @@ AndroidArgs
val testSpecification = TestSpecification().setupAndroidTest(androidTestConfig)
assertTrue(testSpecification.androidInstrumentationTest.testTargets.isNotEmpty())
}

@Test
fun `if set max-test-shards to -1 should give maximum amount`() {
val yaml = """
gcloud:
app: $appApk
test: $testApk
flank:
max-test-shards: -1
""".trimIndent()
val args = AndroidArgs.load(yaml)
assertEquals(AVAILABLE_SHARD_COUNT_RANGE.last, args.maxTestShards)
}
}

private fun AndroidArgs.Companion.load(yamlData: String, cli: AndroidRunCommand? = null): AndroidArgs = load(StringReader(yamlData), cli)
Expand Down
2 changes: 1 addition & 1 deletion test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class ArgsHelperTest {
@Test
fun testInvalidTestShards() {
val maxTestShards = -2
exceptionRule.expectMessage("max-test-shards must be >= 1 and <= 50, or -1. But current is $maxTestShards")
exceptionRule.expectMessage("max-test-shards must be >= ${IArgs.AVAILABLE_SHARD_COUNT_RANGE.first} and <= ${IArgs.AVAILABLE_SHARD_COUNT_RANGE.last}. But current is $maxTestShards")

val args = spyk(AndroidArgs.default())
every { args.maxTestShards } returns maxTestShards
Expand Down
15 changes: 14 additions & 1 deletion test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ IosArgs
)

with(args) {
assert(maxTestShards, -1)
assert(maxTestShards, IArgs.AVAILABLE_SHARD_COUNT_RANGE.last)
assert(testShardChunks.size, 17)
testShardChunks.forEach { chunk -> assert(chunk.size, 1) }
}
Expand Down Expand Up @@ -904,6 +904,19 @@ IosArgs
val iosArgs = IosArgs.load(simpleFlankPath)
assertFalse(iosArgs.keepFilePath)
}

@Test
fun `if set max-test-shards to -1 should give maximum amount`() {
val yaml = """
gcloud:
test: $testPath
xctestrun-file: $testPath
flank:
max-test-shards: -1
""".trimIndent()
val args = IosArgs.load(yaml)
assertEquals(IArgs.AVAILABLE_SHARD_COUNT_RANGE.last, args.maxTestShards)
}
}

private fun IosArgs.Companion.load(yamlData: String, cli: IosRunCommand? = null): IosArgs = load(StringReader(yamlData), cli)
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ftl.gc.android

import com.google.api.services.testing.model.AndroidInstrumentationTest
import com.google.api.services.testing.model.FileReference
import ftl.args.IArgs
import ftl.args.ShardChunks
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNotNull
Expand Down Expand Up @@ -40,7 +41,7 @@ class UtilsKtTest {
.setupTestTargets(
disableSharding = false,
testShards = testShards,
numUniformShards = 50,
numUniformShards = IArgs.AVAILABLE_SHARD_COUNT_RANGE.last,
keepTestTargetsEmpty = false
)

Expand Down
3 changes: 2 additions & 1 deletion test_runner/src/test/kotlin/ftl/shard/ShardTest.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ftl.shard

import com.google.common.truth.Truth.assertThat
import ftl.args.IArgs
import ftl.args.IosArgs
import ftl.reports.xml.model.JUnitTestCase
import ftl.reports.xml.model.JUnitTestResult
Expand Down Expand Up @@ -211,7 +212,7 @@ class ShardTest {
@Test
fun `tests annotated with @Ignore should not produce additional shards`() {
val androidMockedArgs = mockk<IosArgs>()
every { androidMockedArgs.maxTestShards } returns 50
every { androidMockedArgs.maxTestShards } returns IArgs.AVAILABLE_SHARD_COUNT_RANGE.last
every { androidMockedArgs.shardTime } returns -1

val testsToRun = listOf(
Expand Down