From d872f9514616b6c9cb137fa9c7215bde76b4cc54 Mon Sep 17 00:00:00 2001
From: bootstraponline <code@bootstraponline.com>
Date: Sun, 4 Aug 2019 20:13:48 -0700
Subject: [PATCH] Improve shard count validation (#585)

---
 release_notes.md                              |  1 +
 .../src/main/kotlin/ftl/shard/Shard.kt        | 13 +++-
 .../src/test/kotlin/ftl/shard/ShardTest.kt    | 64 +++++++++++++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/release_notes.md b/release_notes.md
index fa327b2259..020a51538a 100644
--- a/release_notes.md
+++ b/release_notes.md
@@ -3,6 +3,7 @@
 - [#574](https://github.com/TestArmada/flank/pull/574) Improve test shard error reporting. Update device catalog to use projectId. ([bootstraponline](https://github.com/bootstraponline))
 - [#582](https://github.com/TestArmada/flank/pull/582) Fix iOS exit code when using flaky-test-attempts. Don't print environment-variables to stdout for security. ([bootstraponline](https://github.com/bootstraponline))
 - [#584](https://github.com/TestArmada/flank/pull/584) Poll all test executions instead of only the first per matrix. ([bootstraponline](https://github.com/bootstraponline))
+- [#585](https://github.com/TestArmada/flank/pull/585) Fix bug in smart flank when sharding tests that run in 0 seconds. ([bootstraponline](https://github.com/bootstraponline))
 
 ## v6.2.3
 
diff --git a/test_runner/src/main/kotlin/ftl/shard/Shard.kt b/test_runner/src/main/kotlin/ftl/shard/Shard.kt
index ca3949968a..8af68ac574 100644
--- a/test_runner/src/main/kotlin/ftl/shard/Shard.kt
+++ b/test_runner/src/main/kotlin/ftl/shard/Shard.kt
@@ -63,19 +63,28 @@ object Shard {
         args: IArgs
     ): Int {
         if (args.shardTime == -1) return -1
+        if (args.shardTime < -1 || args.shardTime == 0) fatalError("Invalid shard time ${args.shardTime}")
 
         val junitMap = createJunitMap(oldTestResult, args)
         val testsTotalTime = testsToRun.sumByDouble { junitMap[it] ?: 10.0 }
 
         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
+        }
+
         // If there is no limit, use the calculated amount
         if (args.maxTestShards == -1) {
             return shardsByTime
         }
 
         // We need to respect the maxTestShards
-        return min(shardsByTime, args.maxTestShards)
+        val shardCount = min(shardsByTime, args.maxTestShards)
+
+        if (shardCount <= 0) fatalError("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
@@ -85,7 +94,7 @@ object Shard {
         args: IArgs,
         forcedShardCount: Int = -1
     ): List<TestShard> {
-        if (forcedShardCount < -1) fatalError("Invalid forcedShardCount value $forcedShardCount")
+        if (forcedShardCount < -1 || forcedShardCount == 0) fatalError("Invalid forcedShardCount value $forcedShardCount")
 
         val maxShards = if (forcedShardCount == -1) args.maxTestShards else forcedShardCount
         val junitMap = createJunitMap(oldTestResult, args)
diff --git a/test_runner/src/test/kotlin/ftl/shard/ShardTest.kt b/test_runner/src/test/kotlin/ftl/shard/ShardTest.kt
index 57ea05fa82..70d5056ce7 100644
--- a/test_runner/src/test/kotlin/ftl/shard/ShardTest.kt
+++ b/test_runner/src/test/kotlin/ftl/shard/ShardTest.kt
@@ -151,4 +151,68 @@ class ShardTest {
 
         assertThat(result).isEqualTo(3)
     }
+
+    @Test(expected = RuntimeException::class)
+    fun `createShardsByShardCount throws on forcedShardCount = 0`() {
+        Shard.createShardsByShardCount(
+                listOf(),
+                sample(),
+                mockArgs(-1, 7),
+                0)
+    }
+
+    private fun newSuite(testCases: MutableList<JUnitTestCase>): JUnitTestResult {
+        return JUnitTestResult(mutableListOf(
+                JUnitTestSuite("",
+                        "3",
+                        "0",
+                        "0",
+                        "0",
+                        "12.032",
+                        "2019-07-27T08:15:04",
+                        "localhost",
+                        testCases,
+                        null,
+                        null,
+                        null)))
+    }
+
+    private fun shardCountByTime(shardTime: Int): Int {
+        val testsToRun = listOf("a/a", "b/b", "c/c")
+        val testCases = mutableListOf(
+                JUnitTestCase("a", "a", "0.001"),
+                JUnitTestCase("b", "b", "0.0"),
+                JUnitTestCase("c", "c", "0.0")
+        )
+
+        val oldTestResult = newSuite(testCases)
+
+        return Shard.shardCountByTime(
+                testsToRun,
+                oldTestResult,
+                mockArgs(-1, shardTime))
+    }
+
+    @Test(expected = RuntimeException::class)
+    fun `shardCountByTime throws on invalid shard time -3`() {
+        shardCountByTime(-3)
+    }
+
+    @Test(expected = RuntimeException::class)
+    fun `shardCountByTime throws on invalid shard time -2`() {
+        shardCountByTime(-2)
+    }
+
+    @Test(expected = RuntimeException::class)
+    fun `shardCountByTime throws on invalid shard time 0`() {
+        shardCountByTime(0)
+    }
+
+    @Test
+    fun `shardCountByTime shards valid times`() {
+        assertThat(shardCountByTime(-1)).isEqualTo(-1)
+        assertThat(shardCountByTime(1)).isEqualTo(1)
+        assertThat(shardCountByTime(2)).isEqualTo(1)
+        assertThat(shardCountByTime(3)).isEqualTo(1)
+    }
 }