#824 Automatically convert -1 in maximum-test-shards to the maximum s…

…hard amount (#825)
Flank · Jun 3, 2020 · 56aed84 · 56aed84
1 parent 4161dc6
commit 56aed84
Show file tree

Hide file tree

Showing 10 changed files with 50 additions and 15 deletions.
diff --git a/release_notes.md b/release_notes.md
@@ -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
 

diff --git a/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt b/test_runner/src/main/kotlin/ftl/args/AndroidArgs.kt
@@ -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

diff --git a/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt b/test_runner/src/main/kotlin/ftl/args/ArgsHelper.kt
@@ -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
@@ -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")

diff --git a/test_runner/src/main/kotlin/ftl/args/IArgs.kt b/test_runner/src/main/kotlin/ftl/args/IArgs.kt
@@ -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

diff --git a/test_runner/src/main/kotlin/ftl/args/IosArgs.kt b/test_runner/src/main/kotlin/ftl/args/IosArgs.kt
@@ -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

diff --git a/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/AndroidArgsTest.kt
@@ -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
@@ -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) }
         }
@@ -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")
 
@@ -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)
     }
@@ -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:
@@ -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)

diff --git a/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt b/test_runner/src/test/kotlin/ftl/args/ArgsHelperTest.kt
@@ -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

diff --git a/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt b/test_runner/src/test/kotlin/ftl/args/IosArgsTest.kt
@@ -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) }
         }
@@ -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)
diff --git a/test_runner/src/test/kotlin/ftl/gc/android/CreateAndroidInstrumentationTestTest.kt b/test_runner/src/test/kotlin/ftl/gc/android/CreateAndroidInstrumentationTestTest.kt
@@ -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
@@ -40,7 +41,7 @@ class UtilsKtTest {
             .setupTestTargets(
                 disableSharding = false,
                 testShards = testShards,
-                numUniformShards = 50,
+                numUniformShards = IArgs.AVAILABLE_SHARD_COUNT_RANGE.last,
                 keepTestTargetsEmpty = false
             )
 

diff --git a/test_runner/src/test/kotlin/ftl/shard/ShardTest.kt b/test_runner/src/test/kotlin/ftl/shard/ShardTest.kt
@@ -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
@@ -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(