Fix no tests being run when flowsOrder specified all tests in the w…

…orkspace (#2003)
mobile-dev-inc · Sep 3, 2024 · 3384806 · 3384806
1 parent 9ad6e39
commit 3384806
Show file tree

Hide file tree

Showing 8 changed files with 162 additions and 78 deletions.
diff --git a/e2e/workspaces/demo_app/fail.yaml b/e2e/workspaces/demo_app/fail.yaml
@@ -1,10 +1,9 @@
 appId: com.example.example
-name: Fail
 tags:
     - android
     - failing
 ---
 - launchApp:
     clearState: true
 - tapOn:
-    id: non-existent-id-to-fail-this-test
+    id: non-existent-id-to-fail-this-test
diff --git a/e2e/workspaces/demo_app/fill_form.yaml b/e2e/workspaces/demo_app/fill_form.yaml
@@ -1,5 +1,4 @@
 appId: com.example.example
-name: Fill out form
 tags:
     - android
     - passing

diff --git a/e2e/workspaces/demo_app/swipe.yaml b/e2e/workspaces/demo_app/swipe.yaml
@@ -0,0 +1,23 @@
+appId: com.example.example
+tags:
+    - android
+    - passing
+---
+- launchApp:
+    clearState: true
+- tapOn: Swipe Test
+- swipe:
+    start: 50%, 15%
+    end: 15%, 50%
+    duration: 1000
+- swipe:
+    start: 15%, 50%
+    end: 85%, 85%
+    duration: 1000
+- swipe:
+    start: 85%, 85%
+    end: 85%, 50%
+    duration: 1000
+- tapOn:
+    point: 85%, 50%
+- assertVisible: All green
diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt
@@ -63,9 +63,7 @@ import maestro.orchestra.util.Env.withDefaultEnvVars
 
 @CommandLine.Command(
     name = "test",
-    description = [
-        "Test a Flow or set of Flows on a local iOS Simulator or Android Emulator"
-    ]
+    description = ["Test a Flow or set of Flows on a local iOS Simulator or Android Emulator"],
 )
 class TestCommand : Callable<Int> {
 
@@ -90,13 +88,13 @@ class TestCommand : Callable<Int> {
 
     @Option(
         names = ["--shard-split"],
-        description = ["Splits the tests across N connected devices"],
+        description = ["Run the tests across N connected devices, splitting the tests evenly across them"],
     )
     private var shardSplit: Int? = null
 
     @Option(
         names = ["--shard-all"],
-        description = ["Replicates all the tests across N connected devices"],
+        description = ["Run all the tests across N connected devices"],
     )
     private var shardAll: Int? = null
 
@@ -123,7 +121,7 @@ class TestCommand : Callable<Int> {
 
     @Option(
         names = ["--debug-output"],
-        description = ["Configures the debug output in this path, instead of default"]
+        description = ["Configures the debug output in this path, instead of default"],
     )
     private var debugOutput: String? = null
 
@@ -165,6 +163,12 @@ class TestCommand : Callable<Int> {
     }
 
     override fun call(): Int {
+        TestDebugReporter.install(
+            debugOutputPathAsString = debugOutput,
+            flattenDebugOutput = flattenDebugOutput,
+            printToConsole = parent?.verbose == true,
+        )
+
         if (shardSplit != null && shardAll != null) {
             throw CliError("Options --shard-split and --shard-all are mutually exclusive.")
         }
@@ -173,11 +177,12 @@ class TestCommand : Callable<Int> {
             PrintUtils.warn("--shards option is deprecated and will be removed in the next Maestro version. Use --shard-split or --shard-all instead.")
             shardSplit = legacyShardCount
         }
+
         val executionPlan = try {
             WorkspaceExecutionPlanner.plan(
-                flowFile.toPath().toAbsolutePath(),
-                includeTags,
-                excludeTags
+                input = flowFile.toPath().toAbsolutePath(),
+                includeTags = includeTags,
+                excludeTags = excludeTags,
             )
         } catch (e: ValidationError) {
             throw CliError(e.message)
@@ -187,17 +192,18 @@ class TestCommand : Callable<Int> {
             .withInjectedShellEnvVars()
             .withDefaultEnvVars(flowFile)
 
-        TestDebugReporter.install(
-            debugOutputPathAsString = debugOutput,
-            flattenDebugOutput = flattenDebugOutput,
-            printToConsole = parent?.verbose == true,
-        )
         val debugOutputPath = TestDebugReporter.getDebugOutputPath()
 
         return handleSessions(debugOutputPath, executionPlan)
     }
 
     private fun handleSessions(debugOutputPath: Path, plan: ExecutionPlan): Int = runBlocking(Dispatchers.IO) {
+        val requestedShards = shardSplit ?: shardAll ?: 1
+        if (requestedShards > 1 && plan.sequence.flows.isNotEmpty()) {
+            error("Cannot run sharded tests with sequential execution")
+        }
+
+        val onlySequenceFlows = plan.sequence.flows.isNotEmpty() && plan.flowsToRun.isEmpty() // An edge case
 
         runCatching {
             val deviceIds = (if (isWebFlow())
@@ -214,29 +220,57 @@ class TestCommand : Callable<Int> {
                 it.instanceId
             }.toMutableSet())
 
-            val shards = shardSplit ?: shardAll ?: 1
-
             val availableDevices = if (deviceIds.isNotEmpty()) deviceIds.size else initialActiveDevices.size
-            val effectiveShards = shards.coerceAtMost(plan.flowsToRun.size)
-            val sharded = effectiveShards > 1
-
-            val chunkPlans =
-                if (shardAll != null) (0 until effectiveShards).map { plan.copy() }
-                else plan.flowsToRun
-                .withIndex()
-                .groupBy { it.index % effectiveShards }
-                .map { (shardIndex, files) ->
-                    ExecutionPlan(
-                        files.map { it.value },
-                        plan.sequence.also {
-                            if (it?.flows?.isNotEmpty() == true && sharded)
-                                error("Cannot run sharded tests with sequential execution.")
-                        }
-                    )
+            val effectiveShards = if (onlySequenceFlows) 1 else requestedShards.coerceAtMost(plan.flowsToRun.size)
+
+            if (requestedShards > plan.flowsToRun.size) {
+                PrintUtils.warn("Requested $requestedShards shards, but it cannot be higher than the number of flows (${plan.flowsToRun.size}). Will use $effectiveShards shards instead.")
+            }
+
+            val chunkPlans: List<ExecutionPlan> = if (onlySequenceFlows) {
+                // Handle an edge case
+                // We only want to run sequential flows in this case.
+                listOf(plan)
+            } else if (shardAll != null) {
+                (0 until effectiveShards).map { plan.copy() }
+            } else {
+                plan.flowsToRun
+                    .withIndex()
+                    .groupBy { it.index % effectiveShards }
+                    .map { (shardIndex, files) ->
+                        ExecutionPlan(
+                            flowsToRun = files.map { it.value },
+                            sequence = plan.sequence,
+                        )
+                    }
+            }
+
+            PrintUtils.info(buildString {
+                val flowCount = if (onlySequenceFlows) plan.sequence.flows.size else plan.flowsToRun.size
+
+                val message = when {
+                    shardAll != null -> "Will run $effectiveShards shards, with all $flowCount flows in each shard"
+
+                    shardSplit != null -> {
+                        val flowsPerShard = flowCount / effectiveShards
+                        val isApprox = flowCount % effectiveShards != 0
+                        val prefix = if (isApprox) "approx. " else ""
+                        "Will split $flowCount flows across $effectiveShards shards (${prefix}$flowsPerShard flows per shard)"
+                    }
+
+                    else -> "Will run $flowCount flows in a single shard"
                 }
 
+                appendLine(message)
+            })
+
             // Collect device configurations for missing shards, if any
             val missing = effectiveShards - availableDevices
+
+            if (missing > 0) {
+                PrintUtils.warn("$availableDevices device(s) connected, which is not enough to run $effectiveShards shards. Missing $missing device(s).")
+            }
+
             val allDeviceConfigs = if (shardAll == null) (0 until missing).map { shardIndex ->
                 PrintUtils.message("------------------ Shard ${shardIndex + 1} ------------------")
                 // Collect device configurations here, one per shard
@@ -247,10 +281,13 @@ class TestCommand : Callable<Int> {
 
             val results = (0 until effectiveShards).map { shardIndex ->
                 async(Dispatchers.IO) {
-                    val driverHostPort = if (!sharded) parent?.port ?: 7001 else
+                    val driverHostPort = if (effectiveShards == 1) {
+                        parent?.port ?: 7001
+                    } else {
                         (7001..7128).shuffled().find { port ->
                             usedPorts.putIfAbsent(port, true) == null
                         } ?: error("No available ports found")
+                    }
 
                     // Acquire lock to execute device creation block
                     deviceCreationSemaphore.acquire()
@@ -262,18 +299,11 @@ class TestCommand : Callable<Int> {
                                 val cfg = allDeviceConfigs.first()
                                 allDeviceConfigs.remove(cfg)
                                 val deviceCreated = DeviceCreateUtil.getOrCreateDevice(
-                                    cfg.platform,
-                                    cfg.osVersion,
-                                    null,
-                                    null,
-                                    true,
-                                    shardIndex
+                                    cfg.platform, cfg.osVersion, null, null, true, shardIndex
                                 )
 
                                 DeviceService.startDevice(
-                                    deviceCreated,
-                                    driverHostPort,
-                                    initialActiveDevices + currentActiveDevices
+                                    deviceCreated, driverHostPort, initialActiveDevices + currentActiveDevices
                                 ).instanceId.also {
                                     currentActiveDevices.add(it)
                                     delay(2.seconds)
@@ -335,12 +365,7 @@ class TestCommand : Callable<Int> {
                                     if (DisableAnsiMixin.ansiEnabled && parent?.verbose == false) AnsiResultView()
                                     else PlainTextResultView()
                                 val resultSingle = TestRunner.runSingle(
-                                    maestro,
-                                    device,
-                                    flowFile,
-                                    env,
-                                    resultView,
-                                    debugOutputPath
+                                    maestro, device, flowFile, env, resultView, debugOutputPath
                                 )
                                 if (resultSingle == 1) {
                                     printExitDebugMessage()
@@ -362,7 +387,7 @@ class TestCommand : Callable<Int> {
 
             suites.mergeSummaries()?.saveReport()
 
-            if (sharded) printShardsMessage(passed, total, suites)
+            if (effectiveShards > 1) printShardsMessage(passed, total, suites)
             if (passed == total) 0 else 1
         }.onFailure {
             PrintUtils.message("❌ Error: ${it.message}")
@@ -380,10 +405,9 @@ class TestCommand : Callable<Int> {
 
     private fun printShardsMessage(passedTests: Int, totalTests: Int, shardResults: List<TestExecutionSummary>) {
         val box = buildString {
-            val lines = listOf("Passed: $passedTests/$totalTests") +
-                    shardResults.mapIndexed { index, result ->
-                        "[ ${result.suites.first().deviceName} ] - ${result.passedCount ?: 0}/${result.totalTests ?: 0}"
-                    }
+            val lines = listOf("Passed: $passedTests/$totalTests") + shardResults.mapIndexed { index, result ->
+                "[ ${result.suites.first().deviceName} ] - ${result.passedCount ?: 0}/${result.totalTests ?: 0}"
+            }
 
             val lineWidth = lines.maxOf(String::length)
             append("┌${"─".repeat(lineWidth)}┐\n")
@@ -397,8 +421,7 @@ class TestCommand : Callable<Int> {
         val reporter = ReporterFactory.buildReporter(format, testSuiteName)
 
         format.fileExtension?.let { extension ->
-            (output ?: File("report$extension"))
-                .sink()
+            (output ?: File("report$extension")).sink()
         }?.also { sink ->
             reporter.report(
                 this,
@@ -408,11 +431,9 @@ class TestCommand : Callable<Int> {
     }
 
     private fun List<TestExecutionSummary>.mergeSummaries(): TestExecutionSummary? = reduceOrNull { acc, summary ->
-        TestExecutionSummary(
-            passed = acc.passed && summary.passed,
+        TestExecutionSummary(passed = acc.passed && summary.passed,
             suites = acc.suites + summary.suites,
             passedCount = sumOf { it.passedCount ?: 0 },
-            totalTests = sumOf { it.totalTests ?: 0 }
-        )
+            totalTests = sumOf { it.totalTests ?: 0 })
     }
 }
diff --git a/maestro-cli/src/main/java/maestro/cli/report/TestDebugReporter.kt b/maestro-cli/src/main/java/maestro/cli/report/TestDebugReporter.kt
@@ -134,6 +134,10 @@ object TestDebugReporter {
         logger.info("---------------------")
     }
 
+    /**
+     * Calls to this method should be done as soon as possible, to make all
+     * loggers use our custom configuration rather than the defaults.
+     */
     fun install(debugOutputPathAsString: String?, flattenDebugOutput: Boolean = false, printToConsole: Boolean) {
         this.debugOutputPathAsString = debugOutputPathAsString
         this.flattenDebugOutput = flattenDebugOutput

diff --git a/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt b/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt
@@ -33,6 +33,8 @@ import kotlin.time.Duration.Companion.seconds
  * Similar to [TestRunner], but:
  *  * can run many flows at once
  *  * does not support continuous mode
+ *
+ *  Does not care about sharding. It only has to know the index of the shard it's running it, for logging purposes.
  */
 class TestSuiteInteractor(
     private val maestro: Maestro,

diff --git a/maestro-cli/src/main/java/maestro/cli/util/PrintUtils.kt b/maestro-cli/src/main/java/maestro/cli/util/PrintUtils.kt
@@ -6,6 +6,16 @@ import kotlin.system.exitProcess
 
 object PrintUtils {
 
+    fun info(message: String, bold: Boolean = false, newline: Boolean = true) {
+        val function: (Any) -> Unit = if (newline) ::println else ::print
+        function(
+            Ansi.ansi()
+                .bold(apply = bold)
+                .render(message)
+                .boldOff()
+        )
+    }
+
     fun message(message: String) {
         println(Ansi.ansi().render("@|cyan \n$message|@"))
     }