From 6c12d34ac08352fe104c96abb490904776f9459b Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Tue, 27 Aug 2024 01:32:43 +0100 Subject: [PATCH 01/15] Fix coding style --- .../maestro/cli/device/DeviceCreateUtil.kt | 88 +++++++++---------- 1 file changed, 40 insertions(+), 48 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/device/DeviceCreateUtil.kt b/maestro-cli/src/main/java/maestro/cli/device/DeviceCreateUtil.kt index 1e0ad83d71..7480247895 100644 --- a/maestro-cli/src/main/java/maestro/cli/device/DeviceCreateUtil.kt +++ b/maestro-cli/src/main/java/maestro/cli/device/DeviceCreateUtil.kt @@ -5,30 +5,23 @@ import maestro.cli.util.* internal object DeviceCreateUtil { - fun getOrCreateDevice(platform: Platform, - osVersion: Int?, - language: String?, - country: String?, - forceCreate: Boolean, - shardIndex: Int? = null): Device.AvailableForLaunch { - return when (platform) { - Platform.ANDROID -> { - getOrCreateAndroidDevice(osVersion, language, country, forceCreate, shardIndex) - } - - Platform.IOS -> { - getOrCreateIosDevice(osVersion, language, country, forceCreate, shardIndex) - } - - else -> throw CliError("Unsupported platform $platform. Please specify one of: android, ios") - } + fun getOrCreateDevice( + platform: Platform, + osVersion: Int? = null, + language: String? = null, + country: String? = null, + forceCreate: Boolean = false, + shardIndex: Int? = null, + ): Device.AvailableForLaunch = when (platform) { + Platform.ANDROID -> getOrCreateAndroidDevice(osVersion, language, country, forceCreate, shardIndex) + Platform.IOS -> getOrCreateIosDevice(osVersion, language, country, forceCreate, shardIndex) + else -> throw CliError("Unsupported platform $platform. Please specify one of: android, ios") } - private fun getOrCreateIosDevice(version: Int?, - language: String?, - country: String?, - forceCreate: Boolean, - shardIndex: Int? = null): Device.AvailableForLaunch { + private fun getOrCreateIosDevice( + version: Int?, language: String?, country: String?, forceCreate: Boolean, shardIndex: Int? = null + ): Device.AvailableForLaunch { + @Suppress("NAME_SHADOWING") val version = version ?: DeviceConfigIos.defaultVersion if (version !in DeviceConfigIos.versions) { throw CliError("Provided iOS version is not supported. Please use one of ${DeviceConfigIos.versions}") } @@ -38,7 +31,7 @@ internal object DeviceCreateUtil { throw CliError("Provided iOS runtime is not supported $runtime") } - val deviceName = DeviceConfigIos.generateDeviceName(version!!) + shardIndex?.let { "_${it + 1}" }.orEmpty() + val deviceName = DeviceConfigIos.generateDeviceName(version) + shardIndex?.let { "_${it + 1}" }.orEmpty() val device = DeviceConfigIos.device // check connected device @@ -63,9 +56,12 @@ internal object DeviceCreateUtil { } catch (e: IllegalStateException) { val error = e.message ?: "" if (error.contains("Invalid runtime")) { - val msg = "Required runtime to create the simulator is not installed: $runtime\n\n" + - "To install additional iOS runtimes checkout this guide:\n" + - "* https://developer.apple.com/documentation/xcode/installing-additional-simulator-runtimes" + val msg = """ + Required runtime to create the simulator is not installed: $runtime + + To install additional iOS runtimes checkout this guide: + * https://developer.apple.com/documentation/xcode/installing-additional-simulator-runtimes + """.trimIndent() throw CliError(msg) } else if (error.contains("Invalid device type")) { throw CliError("Device type $device is either not supported or not found.") @@ -86,11 +82,10 @@ internal object DeviceCreateUtil { } - private fun getOrCreateAndroidDevice(version: Int?, - language: String?, - country: String?, - forceCreate: Boolean, - shardIndex: Int? = null): Device.AvailableForLaunch { + private fun getOrCreateAndroidDevice( + version: Int?, language: String?, country: String?, forceCreate: Boolean, shardIndex: Int? = null + ): Device.AvailableForLaunch { + @Suppress("NAME_SHADOWING") val version = version ?: DeviceConfigAndroid.defaultVersion if (version !in DeviceConfigAndroid.versions) { throw CliError("Provided Android version is not supported. Please use one of ${DeviceConfigAndroid.versions}") } @@ -100,7 +95,7 @@ internal object DeviceCreateUtil { val pixel = DeviceConfigAndroid.choosePixelDevice(pixels) ?: AvdDevice("-1", "Pixel 6", "pixel_6") val config = try { - DeviceConfigAndroid.createConfig(version!!, pixel, architecture) + DeviceConfigAndroid.createConfig(version, pixel, architecture) } catch (e: IllegalStateException) { throw CliError(e.message ?: "Unable to create android device config") } @@ -114,7 +109,8 @@ internal object DeviceCreateUtil { // existing device val existingDevice = - if (forceCreate) null else DeviceService.isDeviceAvailableToLaunch(deviceName, Platform.ANDROID)?.modelId + if (forceCreate) null + else DeviceService.isDeviceAvailableToLaunch(deviceName, Platform.ANDROID)?.modelId // dependencies if (existingDevice == null && !DeviceService.isAndroidSystemImageInstalled(systemImage)) { @@ -125,22 +121,18 @@ internal object DeviceCreateUtil { if (r == "y" || r == "yes") { PrintUtils.message("Attempting to install $systemImage via Android SDK Manager...\n") if (!DeviceService.installAndroidSystemImage(systemImage)) { - throw CliError( - "Unable to install required dependencies. You can install the system image manually by running this command:\n${ - DeviceService.getAndroidSystemImageInstallCommand( - systemImage - ) - }" - ) + val message = """ + Unable to install required dependencies. You can install the system image manually by running this command: + ${DeviceService.getAndroidSystemImageInstallCommand(systemImage)} + """.trimIndent() + throw CliError(message) } } else { - throw CliError( - "To install the system image manually, you can run this command:\n${ - DeviceService.getAndroidSystemImageInstallCommand( - systemImage - ) - }" - ) + val message = """ + To install the system image manually, you can run this command: + ${DeviceService.getAndroidSystemImageInstallCommand(systemImage)} + """.trimIndent() + throw CliError(message) } } @@ -171,4 +163,4 @@ internal object DeviceCreateUtil { country = country, ) } -} \ No newline at end of file +} From fd84ce5fb89861d9ff886d170fe22cc218e74747 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Tue, 27 Aug 2024 02:32:18 +0100 Subject: [PATCH 02/15] Fix device picking logic --- .../java/maestro/cli/command/TestCommand.kt | 61 +++++++++++-------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index c53a74b3ea..f527ee8291 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -59,6 +59,7 @@ import java.util.concurrent.CountDownLatch import kotlin.io.path.absolutePathString import kotlin.system.exitProcess import kotlin.time.Duration.Companion.seconds +import maestro.cli.device.PickDeviceInteractor @CommandLine.Command( name = "test", @@ -187,9 +188,8 @@ class TestCommand : Callable { .map { it.trim() } .filter { it.isNotBlank() } - initialActiveDevices.addAll(DeviceService.listConnectedDevices().map { - it.instanceId - }.toMutableSet()) + val connectedDevices = DeviceService.listConnectedDevices() + initialActiveDevices.addAll(connectedDevices.map { it.instanceId }.toSet()) val effectiveShards = shards.coerceAtMost(plan.flowsToRun.size) val chunkPlans = plan.flowsToRun .withIndex() @@ -205,8 +205,9 @@ class TestCommand : Callable { } // Collect device configurations for missing shards, if any - val missing = effectiveShards - if (deviceIds.isNotEmpty()) deviceIds.size else initialActiveDevices.size - val allDeviceConfigs = (0 until missing).map { shardIndex -> + val availableDevices = if (deviceIds.isNotEmpty()) deviceIds.size else initialActiveDevices.size + val missingDevices = effectiveShards - availableDevices + val missingDevicesConfigs = (0 until missingDevices).map { shardIndex -> PrintUtils.message("------------------ Shard ${shardIndex + 1} ------------------") // Collect device configurations here, one per shard PickDeviceView.requestDeviceOptions() @@ -225,29 +226,37 @@ class TestCommand : Callable { deviceCreationSemaphore.acquire() val deviceId = - deviceIds.getOrNull(shardIndex) // 1. Reuse existing device if deviceId provided - ?: initialActiveDevices.elementAtOrNull(shardIndex) // 2. Reuse existing device if connected device found - ?: run { // 3. Create a new device - val cfg = allDeviceConfigs.first() - allDeviceConfigs.remove(cfg) - val deviceCreated = DeviceCreateUtil.getOrCreateDevice( - cfg.platform, - cfg.osVersion, - null, - null, - true, - shardIndex - ) + // 1. Reuse existing device if deviceId provided + deviceIds.getOrNull(shardIndex) + // 2. Reuse existing device if connected device found + ?: when { + deviceIds.isNotEmpty() -> null + missingDevices >= 0 -> initialActiveDevices.elementAtOrNull(shardIndex) + initialActiveDevices.isNotEmpty() -> PickDeviceInteractor.pickDevice().instanceId + else -> null + } + // 3. Create a new device + ?: run { + val cfg = missingDevicesConfigs.first() + missingDevicesConfigs.remove(cfg) + val deviceCreated = DeviceCreateUtil.getOrCreateDevice( + platform = cfg.platform, + osVersion = cfg.osVersion, + forceCreate = true, + shardIndex = shardIndex + ) - DeviceService.startDevice( - deviceCreated, - driverHostPort, - initialActiveDevices + currentActiveDevices - ).instanceId.also { - currentActiveDevices.add(it) - delay(2.seconds) - } + val device = DeviceService.startDevice( + device = deviceCreated, + driverHostPort = driverHostPort, + connectedDevices = initialActiveDevices + currentActiveDevices + ) + device.instanceId.also { + currentActiveDevices.add(it) + delay(2.seconds) } + } + // Release lock if device ID was obtained from the connected devices deviceCreationSemaphore.release() // Signal that this thread has reached the barrier From b4a56d2bfe8d27e63a410e10b8c9d6ccda4110b4 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Tue, 27 Aug 2024 02:32:48 +0100 Subject: [PATCH 03/15] Add some logs --- maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index f527ee8291..2f8d0463f7 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -60,6 +60,7 @@ import kotlin.io.path.absolutePathString import kotlin.system.exitProcess import kotlin.time.Duration.Companion.seconds import maestro.cli.device.PickDeviceInteractor +import org.slf4j.LoggerFactory @CommandLine.Command( name = "test", @@ -141,6 +142,7 @@ class TestCommand : Callable { private val usedPorts = ConcurrentHashMap() private val initialActiveDevices = ConcurrentSet() private val currentActiveDevices = ConcurrentSet() + private val logger = LoggerFactory.getLogger(TestCommand::class.java) private fun isWebFlow(): Boolean { if (!flowFile.isDirectory) { @@ -215,6 +217,8 @@ class TestCommand : Callable { val barrier = CountDownLatch(effectiveShards) + logger.info("Running $effectiveShards shards on $availableDevices available devices and ${missingDevicesConfigs.size} created devices.") + val results = (0 until effectiveShards).map { shardIndex -> async(Dispatchers.IO) { val driverHostPort = if (!sharded) parent?.port ?: 7001 else @@ -256,6 +260,7 @@ class TestCommand : Callable { delay(2.seconds) } } + logger.info("Selected device $deviceId for shard ${shardIndex + 1} on port $driverHostPort") // Release lock if device ID was obtained from the connected devices deviceCreationSemaphore.release() From e0a167ead06242374b9825d6452f7742dfcde647 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Tue, 27 Aug 2024 02:36:06 +0100 Subject: [PATCH 04/15] Prompt user for device creation. --- .../java/maestro/cli/command/TestCommand.kt | 35 +++++++++++++------ 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index 2f8d0463f7..83fc946d47 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -192,10 +192,32 @@ class TestCommand : Callable { val connectedDevices = DeviceService.listConnectedDevices() initialActiveDevices.addAll(connectedDevices.map { it.instanceId }.toSet()) - val effectiveShards = shards.coerceAtMost(plan.flowsToRun.size) + var effectiveShards = shards.coerceAtMost(plan.flowsToRun.size) + + // Collect device configurations for missing shards, if any + val availableDevices = if (deviceIds.isNotEmpty()) deviceIds.size else initialActiveDevices.size + val missingDevices = effectiveShards - availableDevices + if (missingDevices > 0) { + val message = """ + Found $availableDevices active devices. + Need to create or start $missingDevices more for $effectiveShards shards. Continue? y/n + """.trimIndent() + PrintUtils.message(message) + val str = readlnOrNull()?.lowercase() + val granted = str?.isBlank() == true || str == "y" || str == "yes" + if (!granted) { + PrintUtils.message("Continuing with only $availableDevices shards.") + effectiveShards = availableDevices + } + } + val missingDevicesConfigs = (availableDevices until effectiveShards).mapNotNull { shardIndex -> + PrintUtils.message("Creating device for shard ${shardIndex + 1}:") + PickDeviceView.requestDeviceOptions() + }.toMutableList() + val chunkPlans = plan.flowsToRun .withIndex() - .groupBy { it.index % shards } + .groupBy { it.index % effectiveShards } .map { (shardIndex, files) -> ExecutionPlan( files.map { it.value }, @@ -206,15 +228,6 @@ class TestCommand : Callable { ) } - // Collect device configurations for missing shards, if any - val availableDevices = if (deviceIds.isNotEmpty()) deviceIds.size else initialActiveDevices.size - val missingDevices = effectiveShards - availableDevices - val missingDevicesConfigs = (0 until missingDevices).map { shardIndex -> - PrintUtils.message("------------------ Shard ${shardIndex + 1} ------------------") - // Collect device configurations here, one per shard - PickDeviceView.requestDeviceOptions() - }.toMutableList() - val barrier = CountDownLatch(effectiveShards) logger.info("Running $effectiveShards shards on $availableDevices available devices and ${missingDevicesConfigs.size} created devices.") From 2a4692ae7b1d2fbda8f61bc66e8d2c789831e453 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Tue, 27 Aug 2024 03:35:13 +0100 Subject: [PATCH 05/15] Refactor TestCommand file --- .../java/maestro/cli/command/TestCommand.kt | 400 ++++++++++-------- 1 file changed, 233 insertions(+), 167 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index 83fc946d47..e36ea15462 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -26,13 +26,17 @@ import kotlinx.coroutines.awaitAll import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Semaphore +import maestro.Maestro import maestro.cli.App import maestro.cli.CliError import maestro.cli.DisableAnsiMixin import maestro.cli.ShowHelpMixin +import maestro.cli.device.Device import maestro.cli.device.DeviceCreateUtil import maestro.cli.device.DeviceService +import maestro.cli.device.PickDeviceInteractor import maestro.cli.device.PickDeviceView +import maestro.cli.model.DeviceStartOptions import maestro.cli.model.TestExecutionSummary import maestro.cli.report.ReportFormat import maestro.cli.report.ReporterFactory @@ -43,12 +47,14 @@ import maestro.cli.runner.resultview.AnsiResultView import maestro.cli.runner.resultview.PlainTextResultView import maestro.cli.session.MaestroSessionManager import maestro.cli.util.PrintUtils +import maestro.cli.view.box import maestro.orchestra.error.ValidationError import maestro.orchestra.util.Env.withInjectedShellEnvVars import maestro.orchestra.workspace.WorkspaceExecutionPlanner import maestro.orchestra.workspace.WorkspaceExecutionPlanner.ExecutionPlan import maestro.orchestra.yaml.YamlCommandReader import okio.sink +import org.slf4j.LoggerFactory import picocli.CommandLine import picocli.CommandLine.Option import java.io.File @@ -59,8 +65,6 @@ import java.util.concurrent.CountDownLatch import kotlin.io.path.absolutePathString import kotlin.system.exitProcess import kotlin.time.Duration.Companion.seconds -import maestro.cli.device.PickDeviceInteractor -import org.slf4j.LoggerFactory @CommandLine.Command( name = "test", @@ -177,18 +181,9 @@ class TestCommand : Callable { } private fun handleSessions(debugOutputPath: Path, plan: ExecutionPlan): Int = runBlocking(Dispatchers.IO) { - val sharded = shards > 1 runCatching { - val deviceIds = (if (isWebFlow()) - "chromium".also { - PrintUtils.warn("Web support is an experimental feature and may be removed in future versions.\n") - } - else parent?.deviceId) - .orEmpty() - .split(",") - .map { it.trim() } - .filter { it.isNotBlank() } + val deviceIds = getPassedOptionsDeviceIds() val connectedDevices = DeviceService.listConnectedDevices() initialActiveDevices.addAll(connectedDevices.map { it.instanceId }.toSet()) @@ -197,36 +192,15 @@ class TestCommand : Callable { // Collect device configurations for missing shards, if any val availableDevices = if (deviceIds.isNotEmpty()) deviceIds.size else initialActiveDevices.size val missingDevices = effectiveShards - availableDevices - if (missingDevices > 0) { - val message = """ - Found $availableDevices active devices. - Need to create or start $missingDevices more for $effectiveShards shards. Continue? y/n - """.trimIndent() - PrintUtils.message(message) - val str = readlnOrNull()?.lowercase() - val granted = str?.isBlank() == true || str == "y" || str == "yes" - if (!granted) { - PrintUtils.message("Continuing with only $availableDevices shards.") - effectiveShards = availableDevices - } + + if (!promptForDeviceCreation(availableDevices, effectiveShards)) { + PrintUtils.message("Continuing with only $availableDevices shards.") + effectiveShards = availableDevices } - val missingDevicesConfigs = (availableDevices until effectiveShards).mapNotNull { shardIndex -> - PrintUtils.message("Creating device for shard ${shardIndex + 1}:") - PickDeviceView.requestDeviceOptions() - }.toMutableList() - - val chunkPlans = 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 missingDevicesConfigs = createMissingDevices(availableDevices, effectiveShards).toMutableList() + + val sharded = effectiveShards > 1 + val chunkPlans = makeChunkPlans(plan, effectiveShards, sharded) val barrier = CountDownLatch(effectiveShards) @@ -234,120 +208,16 @@ class TestCommand : Callable { val results = (0 until effectiveShards).map { shardIndex -> async(Dispatchers.IO) { - val driverHostPort = if (!sharded) 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() - - val deviceId = - // 1. Reuse existing device if deviceId provided - deviceIds.getOrNull(shardIndex) - // 2. Reuse existing device if connected device found - ?: when { - deviceIds.isNotEmpty() -> null - missingDevices >= 0 -> initialActiveDevices.elementAtOrNull(shardIndex) - initialActiveDevices.isNotEmpty() -> PickDeviceInteractor.pickDevice().instanceId - else -> null - } - // 3. Create a new device - ?: run { - val cfg = missingDevicesConfigs.first() - missingDevicesConfigs.remove(cfg) - val deviceCreated = DeviceCreateUtil.getOrCreateDevice( - platform = cfg.platform, - osVersion = cfg.osVersion, - forceCreate = true, - shardIndex = shardIndex - ) - - val device = DeviceService.startDevice( - device = deviceCreated, - driverHostPort = driverHostPort, - connectedDevices = initialActiveDevices + currentActiveDevices - ) - device.instanceId.also { - currentActiveDevices.add(it) - delay(2.seconds) - } - } - logger.info("Selected device $deviceId for shard ${shardIndex + 1} on port $driverHostPort") - - // Release lock if device ID was obtained from the connected devices - deviceCreationSemaphore.release() - // Signal that this thread has reached the barrier - barrier.countDown() - // Wait for all threads/shards to complete device creation before proceeding - barrier.await() - - MaestroSessionManager.newSession( - host = parent?.host, - port = parent?.port, - driverHostPort = driverHostPort, - deviceId = deviceId - ) { session -> - val maestro = session.maestro - val device = session.device - - if (flowFile.isDirectory || format != ReportFormat.NOOP) { - // Run multiple flows - if (continuous) { - val error = - if (format != ReportFormat.NOOP) "Format can not be different from NOOP in continuous mode. Passed format is $format." - else "Continuous mode is not supported for directories. $flowFile is a directory" - throw CommandLine.ParameterException(commandSpec.commandLine(), error) - } - - val suiteResult = TestSuiteInteractor( - maestro = maestro, - device = device, - reporter = ReporterFactory.buildReporter(format, testSuiteName), - ).runTestSuite( - executionPlan = chunkPlans[shardIndex], - env = env, - reportOut = null, - debugOutputPath = debugOutputPath - ) - - if (!flattenDebugOutput) { - TestDebugReporter.deleteOldFiles() - } - - return@newSession Triple(suiteResult.passedCount, suiteResult.totalTests, suiteResult) - } else { - // Run a single flow - - if (continuous) { - if (!flattenDebugOutput) { - TestDebugReporter.deleteOldFiles() - } - TestRunner.runContinuous(maestro, device, flowFile, env) - - } else { - val resultView = - if (DisableAnsiMixin.ansiEnabled) AnsiResultView() - else PlainTextResultView() - val resultSingle = TestRunner.runSingle( - maestro, - device, - flowFile, - env, - resultView, - debugOutputPath - ) - if (resultSingle == 1) { - printExitDebugMessage() - } - if (!flattenDebugOutput) { - TestDebugReporter.deleteOldFiles() - } - val result = if (resultSingle == 0) 1 else 0 - return@newSession Triple(result, 1, null) - } - } - } + runShardSuite( + sharded, + deviceIds, + shardIndex, + missingDevices, + missingDevicesConfigs, + barrier, + chunkPlans, + debugOutputPath + ) } }.awaitAll() @@ -367,6 +237,209 @@ class TestCommand : Callable { }.getOrDefault(0) } + private suspend fun runShardSuite( + sharded: Boolean, + deviceIds: List, + shardIndex: Int, + missingDevices: Int, + missingDevicesConfigs: MutableList, + barrier: CountDownLatch, + chunkPlans: List, + debugOutputPath: Path + ): Triple = withContext(Dispatchers.IO) { + val driverHostPort = if (!sharded) 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() + + val deviceId = assignDeviceToShard(deviceIds, shardIndex, missingDevices, missingDevicesConfigs, driverHostPort) + logger.info("Selected device $deviceId for shard ${shardIndex + 1} on port $driverHostPort") + + // Release lock if device ID was obtained from the connected devices + deviceCreationSemaphore.release() + // Signal that this thread has reached the barrier + barrier.countDown() + // Wait for all threads/shards to complete device creation before proceeding + barrier.await() + + return@withContext MaestroSessionManager.newSession( + host = parent?.host, + port = parent?.port, + driverHostPort = driverHostPort, + deviceId = deviceId + ) { session -> + val maestro = session.maestro + val device = session.device + + if (flowFile.isDirectory || format != ReportFormat.NOOP) { + if (continuous) { + throw CommandLine.ParameterException( + commandSpec.commandLine(), + "Continuous mode is not supported for directories. $flowFile is a directory", + ) + } + runMultipleFlows(maestro, device, chunkPlans, shardIndex, debugOutputPath) + } else { + if (continuous) { + if (!flattenDebugOutput) { + TestDebugReporter.deleteOldFiles() + } + TestRunner.runContinuous(maestro, device, flowFile, env) + } else { + runSingleFlow(maestro, device, debugOutputPath) + } + } + } + } + + private fun runSingleFlow( + maestro: Maestro, + device: Device?, + debugOutputPath: Path + ): Triple { + val resultView = + if (DisableAnsiMixin.ansiEnabled) AnsiResultView() + else PlainTextResultView() + val resultSingle = TestRunner.runSingle( + maestro, + device, + flowFile, + env, + resultView, + debugOutputPath + ) + if (resultSingle == 1) { + printExitDebugMessage() + } + if (!flattenDebugOutput) { + TestDebugReporter.deleteOldFiles() + } + val result = if (resultSingle == 0) 1 else 0 + return Triple(result, 1, null) + } + + private fun runMultipleFlows( + maestro: Maestro, + device: Device?, + chunkPlans: List, + shardIndex: Int, + debugOutputPath: Path + ): Triple { + val suiteResult = TestSuiteInteractor( + maestro = maestro, + device = device, + reporter = ReporterFactory.buildReporter(format, testSuiteName), + ).runTestSuite( + executionPlan = chunkPlans[shardIndex], + env = env, + reportOut = null, + debugOutputPath = debugOutputPath + ) + + if (!flattenDebugOutput) { + TestDebugReporter.deleteOldFiles() + } + return Triple(suiteResult.passedCount, suiteResult.totalTests, suiteResult) + } + + private suspend fun assignDeviceToShard( + deviceIds: List, + shardIndex: Int, + missingDevices: Int, + missingDevicesConfigs: MutableList, + driverHostPort: Int + ): String = + useDevicesPassedAsOptions(deviceIds, shardIndex) + ?: useConnectedDevices(deviceIds, missingDevices, shardIndex) + ?: createNewDevice(missingDevicesConfigs, shardIndex, driverHostPort) + + private fun useDevicesPassedAsOptions(deviceIds: List, shardIndex: Int) = + deviceIds.getOrNull(shardIndex) + + private fun useConnectedDevices( + deviceIds: List, + missingDevices: Int, + shardIndex: Int + ) = when { + deviceIds.isNotEmpty() -> null + missingDevices >= 0 -> initialActiveDevices.elementAtOrNull(shardIndex) + initialActiveDevices.isNotEmpty() -> PickDeviceInteractor.pickDevice().instanceId + else -> null + } + + private suspend fun createNewDevice( + missingDevicesConfigs: MutableList, + shardIndex: Int, + driverHostPort: Int + ): String { + val cfg = missingDevicesConfigs.first() + missingDevicesConfigs.remove(cfg) + val deviceCreated = DeviceCreateUtil.getOrCreateDevice( + platform = cfg.platform, + osVersion = cfg.osVersion, + forceCreate = true, + shardIndex = shardIndex + ) + val device = DeviceService.startDevice( + device = deviceCreated, + driverHostPort = driverHostPort, + connectedDevices = initialActiveDevices + currentActiveDevices + ) + currentActiveDevices.add(device.instanceId) + delay(2.seconds) + return device.instanceId + } + + private fun makeChunkPlans( + plan: ExecutionPlan, + effectiveShards: Int, + shared: Boolean, + ) = plan.flowsToRun + .withIndex() + .groupBy { it.index % effectiveShards } + .map { (_, files) -> + val flowsToRun = files.map { it.value } + if (plan.sequence?.flows?.isNotEmpty() == true && shared) + error("Cannot run sharded tests with sequential execution.") + ExecutionPlan(flowsToRun, plan.sequence) + } + + private fun createMissingDevices( + availableDevices: Int, + effectiveShards: Int + ) = (availableDevices until effectiveShards).map { shardIndex -> + PrintUtils.message("Creating device for shard ${shardIndex + 1}:") + PickDeviceView.requestDeviceOptions() + } + + private fun promptForDeviceCreation(availableDevices: Int, effectiveShards: Int): Boolean { + val missingDevices = effectiveShards - availableDevices + if (missingDevices <= 0) return true + val message = """ + Found $availableDevices active devices. + Need to create or start $missingDevices more for $effectiveShards shards. Continue? y/n + """.trimIndent() + PrintUtils.message(message) + val str = readlnOrNull()?.lowercase() + return str?.isBlank() == true || str == "y" || str == "yes" + } + + private fun getPassedOptionsDeviceIds(): List { + val arguments = if (isWebFlow()) { + PrintUtils.warn("Web support is an experimental feature and may be removed in future versions.\n") + "chromium" + } else parent?.deviceId + val deviceIds = arguments + .orEmpty() + .split(",") + .map { it.trim() } + .filter { it.isNotBlank() } + return deviceIds + } + private fun printExitDebugMessage() { println() println("==== Debug output (logs & screenshots) ====") @@ -374,18 +447,11 @@ class TestCommand : Callable { } private fun printShardsMessage(passedTests: Int, totalTests: Int, shardResults: List) { - val box = buildString { - 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") - lines.forEach { append("│${it.padEnd(lineWidth)}│\n") } - append("└${"─".repeat(lineWidth)}┘") - } - PrintUtils.message(box) + val lines = listOf("Passed: $passedTests/$totalTests") + + shardResults.mapIndexed { _, result -> + "[ ${result.suites.first().deviceName} ] - ${result.passedCount ?: 0}/${result.totalTests ?: 0}" + } + PrintUtils.message(lines.joinToString("\n").box()) } private fun TestExecutionSummary.saveReport() { From 592ac48a00d35251990d703df3e1615a72476f27 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Sat, 31 Aug 2024 15:53:27 +0100 Subject: [PATCH 06/15] Fix missing import --- maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index e36ea15462..523ff18c8b 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -65,6 +65,7 @@ import java.util.concurrent.CountDownLatch import kotlin.io.path.absolutePathString import kotlin.system.exitProcess import kotlin.time.Duration.Companion.seconds +import kotlinx.coroutines.withContext @CommandLine.Command( name = "test", From 70c97047cdc5913546e0c989563ad9147366fd41 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Wed, 4 Sep 2024 05:09:22 +0100 Subject: [PATCH 07/15] Fix issues with shard-all and 1 test --- .../main/java/maestro/cli/command/TestCommand.kt | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index 6baa2b14d7..d2675d6169 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -228,12 +228,16 @@ class TestCommand : Callable { initialActiveDevices.addAll(activeDevices) val availableDevices = if (deviceIds.isNotEmpty()) deviceIds.size else initialActiveDevices.size - var effectiveShards = if (onlySequenceFlows) 1 else requestedShards.coerceAtMost(plan.flowsToRun.size) + var effectiveShards = when { + onlySequenceFlows -> 1 + shardAll == null -> requestedShards.coerceAtMost(plan.flowsToRun.size) + else -> requestedShards + } val warning = "Requested $requestedShards shards, " + "but it cannot be higher than the number of flows (${plan.flowsToRun.size}). " + "Will use $effectiveShards shards instead." - if (requestedShards > plan.flowsToRun.size) PrintUtils.warn(warning) + if (shardAll == null && requestedShards > plan.flowsToRun.size) PrintUtils.warn(warning) val chunkPlans = makeChunkPlans(plan, effectiveShards, onlySequenceFlows) @@ -328,7 +332,10 @@ class TestCommand : Callable { val maestro = session.maestro val device = session.device - if (flowFile.isDirectory || format != ReportFormat.NOOP) { + val isReplicatingSingleTest = shardAll != null && effectiveShards > 1 + val isRunningFromFolder = flowFile.isDirectory + val isAskingForReport = format != ReportFormat.NOOP + if (isRunningFromFolder || isAskingForReport || isReplicatingSingleTest) { if (continuous) { throw CommandLine.ParameterException( commandSpec.commandLine(), From 0304ad03d71f3d1cc052085b3935cf4ea790e938 Mon Sep 17 00:00:00 2001 From: Bartek Pacia Date: Wed, 4 Sep 2024 14:04:39 +0200 Subject: [PATCH 08/15] TestCommand: minor formatting improvements, use named args more --- .../java/maestro/cli/command/TestCommand.kt | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index d2675d6169..6cd81e74e4 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -88,8 +88,9 @@ class TestCommand : Callable { private lateinit var flowFile: File @Option( - names = ["--config"], - description = ["Optional YAML configuration file for the workspace. If not provided, Maestro will look for a config.yaml file in the workspace's root directory."]) + names = ["--config"], + description = ["Optional YAML configuration file for the workspace. If not provided, Maestro will look for a config.yaml file in the workspace's root directory."] + ) private var configFile: File? = null @Option( @@ -235,8 +236,8 @@ class TestCommand : Callable { } val warning = "Requested $requestedShards shards, " + - "but it cannot be higher than the number of flows (${plan.flowsToRun.size}). " + - "Will use $effectiveShards shards instead." + "but it cannot be higher than the number of flows (${plan.flowsToRun.size}). " + + "Will use $effectiveShards shards instead." if (shardAll == null && requestedShards > plan.flowsToRun.size) PrintUtils.warn(warning) val chunkPlans = makeChunkPlans(plan, effectiveShards, onlySequenceFlows) @@ -259,6 +260,7 @@ class TestCommand : Callable { val prefix = if (isApprox) "approx. " else "" "Will split $flowCount flows across $effectiveShards shards (${prefix}$flowsPerShard flows per shard)" } + else -> null } message?.let { PrintUtils.info(it) } @@ -267,14 +269,14 @@ class TestCommand : Callable { val results = (0 until effectiveShards).map { shardIndex -> async(Dispatchers.IO) { runShardSuite( - effectiveShards, - deviceIds, - shardIndex, - missingDevices, - missingDevicesConfigs, - barrier, - chunkPlans, - debugOutputPath + effectiveShards = effectiveShards, + deviceIds = deviceIds, + shardIndex = shardIndex, + missingDevices = missingDevices, + missingDevicesConfigs = missingDevicesConfigs, + barrier = barrier, + chunkPlans = chunkPlans, + debugOutputPath = debugOutputPath, ) } }.awaitAll() @@ -417,8 +419,8 @@ class TestCommand : Callable { driverHostPort: Int ): String = useDevicesPassedAsOptions(deviceIds, shardIndex) - ?: useConnectedDevices(deviceIds, missingDevices, shardIndex) - ?: createNewDevice(missingDevicesConfigs, shardIndex, driverHostPort) + ?: useConnectedDevices(deviceIds, missingDevices, shardIndex) + ?: createNewDevice(missingDevicesConfigs, shardIndex, driverHostPort) private fun useDevicesPassedAsOptions(deviceIds: List, shardIndex: Int) = deviceIds.getOrNull(shardIndex) @@ -514,9 +516,9 @@ class TestCommand : Callable { private fun printShardsMessage(passedTests: Int, totalTests: Int, shardResults: List) { val lines = listOf("Passed: $passedTests/$totalTests") + - shardResults.mapIndexed { _, result -> - "[ ${result.suites.first().deviceName} ] - ${result.passedCount ?: 0}/${result.totalTests ?: 0}" - } + shardResults.mapIndexed { _, result -> + "[ ${result.suites.first().deviceName} ] - ${result.passedCount ?: 0}/${result.totalTests ?: 0}" + } PrintUtils.message(lines.joinToString("\n").box()) } From 3280042249e9dfed7f1177b71f1df3d030944623 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Wed, 4 Sep 2024 15:30:13 +0100 Subject: [PATCH 09/15] Fix initial state of param --- maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index 677abad7c8..3803688816 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -86,7 +86,7 @@ class TestCommand : Callable { private val parent: App? = null @CommandLine.Parameters(description = ["One or more flow files or folders containing flow files"]) - private lateinit var flowFiles: Set + private var flowFiles: Set = emptySet() @Option( names = ["--config"], From 7d88ffa4cc1ccb1f06b4f7425bffa5634c3d620f Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Wed, 4 Sep 2024 18:38:41 +0100 Subject: [PATCH 10/15] Add shard info to logs --- .../java/maestro/cli/command/TestCommand.kt | 7 ++-- .../maestro/cli/report/TestDebugReporter.kt | 11 +++--- .../maestro/cli/runner/TestSuiteInteractor.kt | 30 +++++++++------- .../maestro/cli/view/TestSuiteStatusView.kt | 34 +++++++++---------- .../main/java/maestro/debuglog/LogConfig.kt | 2 -- .../java/maestro/utils/ScreenshotUtils.kt | 6 ++-- 6 files changed, 47 insertions(+), 43 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index 3803688816..8be28a3e42 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -314,7 +314,7 @@ class TestCommand : Callable { deviceCreationSemaphore.acquire() val deviceId = assignDeviceToShard(deviceIds, shardIndex, missingDevices, missingDevicesConfigs, driverHostPort) - logger.info("Selected device $deviceId for shard ${shardIndex + 1} on port $driverHostPort") + logger.info("[shard ${shardIndex + 1}] Selected device $deviceId using port $driverHostPort") // Release lock if device ID was obtained from the connected devices deviceCreationSemaphore.release() @@ -333,7 +333,7 @@ class TestCommand : Callable { val maestro = session.maestro val device = session.device - val isReplicatingSingleTest = shardAll != null && effectiveShards > 1 + val isReplicatingSingleTest = shardAll != null && effectiveShards > 1 && flowFiles.isSingleFile val isMultipleFiles = flowFiles.isSingleFile.not() val isAskingForReport = format != ReportFormat.NOOP if (isMultipleFiles || isAskingForReport || isReplicatingSingleTest) { @@ -404,6 +404,7 @@ class TestCommand : Callable { val suiteResult = TestSuiteInteractor( maestro = maestro, device = device, + shardIndex = shardIndex, reporter = ReporterFactory.buildReporter(format, testSuiteName), ).runTestSuite( executionPlan = chunkPlans[shardIndex], @@ -472,7 +473,7 @@ class TestCommand : Callable { onlySequenceFlows: Boolean, ) = when { onlySequenceFlows -> listOf(plan) // We only want to run sequential flows in this case. - shardAll != null -> (0 until effectiveShards).map { plan.copy() } + shardAll != null -> (0 until effectiveShards).reversed().map { plan.copy() } else -> plan.flowsToRun .withIndex() .groupBy { it.index % effectiveShards } diff --git a/maestro-cli/src/main/java/maestro/cli/report/TestDebugReporter.kt b/maestro-cli/src/main/java/maestro/cli/report/TestDebugReporter.kt index e9cc9dede3..ad9f85b118 100644 --- a/maestro-cli/src/main/java/maestro/cli/report/TestDebugReporter.kt +++ b/maestro-cli/src/main/java/maestro/cli/report/TestDebugReporter.kt @@ -67,16 +67,19 @@ object TestDebugReporter { /** * Save debug information about a single flow, after it has finished. */ - fun saveFlow(flowName: String, debugOutput: FlowDebugOutput, path: Path) { + fun saveFlow(flowName: String, debugOutput: FlowDebugOutput, path: Path, shardIndex: Int? = null) { // TODO(bartekpacia): Potentially accept a single "FlowPersistentOutput" object // TODO(bartekpacia: Build output incrementally, instead of single-shot on flow completion // Be aware that this goal somewhat conflicts with including links to other flows in the HTML report. + val shardPrefix = shardIndex?.let { "shard-${it + 1}-" }.orEmpty() + val shardLogPrefix = shardIndex?.let { "[shard ${it + 1}] " }.orEmpty() + // commands try { val commandMetadata = debugOutput.commands if (commandMetadata.isNotEmpty()) { - val commandsFilename = "commands-(${flowName.replace("/", "_")}).json" + val commandsFilename = "commands-$shardPrefix(${flowName.replace("/", "_")}).json" val file = File(path.absolutePathString(), commandsFilename) commandMetadata.map { CommandDebugWrapper(it.key, it.value) @@ -85,7 +88,7 @@ object TestDebugReporter { } } } catch (e: JsonMappingException) { - logger.error("Unable to parse commands", e) + logger.error("${shardLogPrefix}Unable to parse commands", e) } // screenshots @@ -96,7 +99,7 @@ object TestDebugReporter { CommandStatus.WARNED -> "⚠️" else -> "﹖" } - val filename = "screenshot-$status-${it.timestamp}-(${flowName}).png" + val filename = "screenshot-$shardPrefix$status-${it.timestamp}-(${flowName}).png" val file = File(path.absolutePathString(), filename) it.screenshot.copyTo(file) diff --git a/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt b/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt index 968a869caf..4592e12bba 100644 --- a/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt +++ b/maestro-cli/src/main/java/maestro/cli/runner/TestSuiteInteractor.kt @@ -42,9 +42,11 @@ class TestSuiteInteractor( private val maestro: Maestro, private val device: Device? = null, private val reporter: TestSuiteReporter, + private val shardIndex: Int? = null, ) { private val logger = LoggerFactory.getLogger(TestSuiteInteractor::class.java) + private val shardPrefix = shardIndex?.let { "[shard ${it + 1}] " }.orEmpty() fun runTestSuite( executionPlan: WorkspaceExecutionPlanner.ExecutionPlan, @@ -53,13 +55,12 @@ class TestSuiteInteractor( debugOutputPath: Path ): TestExecutionSummary { if (executionPlan.flowsToRun.isEmpty() && executionPlan.sequence.flows.isEmpty()) { - throw CliError("No flows returned from the tag filter used") + throw CliError("${shardPrefix}No flows returned from the tag filter used") } val flowResults = mutableListOf() - PrintUtils.message("Waiting for flows to complete...") - println() + PrintUtils.message("${shardPrefix}Waiting for flows to complete...") var passed = true val aiOutputs = mutableListOf() @@ -78,7 +79,7 @@ class TestSuiteInteractor( if (result.status == FlowStatus.ERROR) { passed = false if (executionPlan.sequence.continueOnFailure != true) { - PrintUtils.message("Flow ${result.name} failed and continueOnFailure is set to false, aborting running sequential Flows") + PrintUtils.message("${shardPrefix}Flow ${result.name} failed and continueOnFailure is set to false, aborting running sequential Flows") println() break } @@ -103,6 +104,7 @@ class TestSuiteInteractor( TestSuiteViewModel( status = if (passed) FlowStatus.SUCCESS else FlowStatus.ERROR, duration = suiteDuration, + shardIndex = shardIndex, flows = flowResults .map { TestSuiteViewModel.FlowResult( @@ -173,21 +175,21 @@ class TestSuiteInteractor( val orchestra = Orchestra( maestro = maestro, onCommandStart = { _, command -> - logger.info("${command.description()} RUNNING") + logger.info("${shardPrefix}${command.description()} RUNNING") debugOutput.commands[command] = CommandDebugMetadata( timestamp = System.currentTimeMillis(), status = CommandStatus.RUNNING ) }, onCommandComplete = { _, command -> - logger.info("${command.description()} COMPLETED") + logger.info("${shardPrefix}${command.description()} COMPLETED") debugOutput.commands[command]?.let { it.status = CommandStatus.COMPLETED it.calculateDuration() } }, onCommandFailed = { _, command, e -> - logger.info("${command.description()} FAILED") + logger.info("${shardPrefix}${command.description()} FAILED") if (e is MaestroException) debugOutput.exception = e debugOutput.commands[command]?.let { it.status = CommandStatus.FAILED @@ -199,25 +201,25 @@ class TestSuiteInteractor( Orchestra.ErrorResolution.FAIL }, onCommandSkipped = { _, command -> - logger.info("${command.description()} SKIPPED") + logger.info("${shardPrefix}${command.description()} SKIPPED") debugOutput.commands[command]?.let { it.status = CommandStatus.SKIPPED } }, onCommandWarned = { _, command -> - logger.info("${command.description()} WARNED") + logger.info("${shardPrefix}${command.description()} WARNED") debugOutput.commands[command]?.apply { status = CommandStatus.WARNED } }, onCommandReset = { command -> - logger.info("${command.description()} PENDING") + logger.info("${shardPrefix}${command.description()} PENDING") debugOutput.commands[command]?.let { it.status = CommandStatus.PENDING } }, onCommandGeneratedOutput = { command, defects, screenshot -> - logger.info("${command.description()} generated output") + logger.info("${shardPrefix}${command.description()} generated output") val screenshotPath = ScreenshotUtils.writeAIscreenshot(screenshot) aiOutput.screenOutputs.add( SingleScreenFlowAIOutput( @@ -231,7 +233,7 @@ class TestSuiteInteractor( val flowSuccess = orchestra.runFlow(commands) flowStatus = if (flowSuccess) FlowStatus.SUCCESS else FlowStatus.ERROR } catch (e: Exception) { - logger.error("Failed to complete flow", e) + logger.error("${shardPrefix}Failed to complete flow", e) flowStatus = FlowStatus.ERROR errorMessage = ErrorViewUtils.exceptionToMessage(e) } @@ -241,6 +243,7 @@ class TestSuiteInteractor( TestDebugReporter.saveFlow( flowName = flowName, debugOutput = debugOutput, + shardIndex = shardIndex, path = debugOutputPath, ) // FIXME(bartekpacia): Save AI output as well @@ -250,6 +253,7 @@ class TestSuiteInteractor( name = flowName, status = flowStatus, duration = flowDuration, + shardIndex = shardIndex, error = debugOutput.exception?.message, ) ) @@ -261,7 +265,7 @@ class TestSuiteInteractor( status = flowStatus, failure = if (flowStatus == FlowStatus.ERROR) { TestExecutionSummary.Failure( - message = errorMessage ?: debugOutput.exception?.message ?: "Unknown error", + message = shardPrefix + (errorMessage ?: debugOutput.exception?.message ?: "Unknown error"), ) } else null, duration = flowDuration, diff --git a/maestro-cli/src/main/java/maestro/cli/view/TestSuiteStatusView.kt b/maestro-cli/src/main/java/maestro/cli/view/TestSuiteStatusView.kt index 2f8ead4217..c8510e3775 100644 --- a/maestro-cli/src/main/java/maestro/cli/view/TestSuiteStatusView.kt +++ b/maestro-cli/src/main/java/maestro/cli/view/TestSuiteStatusView.kt @@ -13,27 +13,22 @@ import kotlin.time.Duration.Companion.seconds object TestSuiteStatusView { fun showFlowCompletion(result: FlowResult) { + val shardPrefix = result.shardIndex?.let { "[shard ${it + 1}] " }.orEmpty() + print(Ansi.ansi().fgCyan().render(shardPrefix).fgDefault()) + printStatus(result.status, result.cancellationReason) val durationString = result.duration?.let { " ($it)" }.orEmpty() print(" ${result.name}$durationString") + if (result.status == FlowStatus.ERROR && result.error != null) { - print( - Ansi.ansi() - .fgRed() - .render(" (${result.error})") - .fgDefault() - ) + val error = " (${result.error})" + print(Ansi.ansi().fgRed().render(error).fgDefault()) } else if (result.status == FlowStatus.WARNING) { - print( - Ansi.ansi() - .fgYellow() - .render(" (Warning)") - .fgDefault() - ) + val warning = " (Warning)" + print(Ansi.ansi().fgYellow().render(warning).fgDefault()) } - println() } @@ -44,18 +39,19 @@ object TestSuiteStatusView { val hasError = suite.flows.find { it.status == FlowStatus.ERROR } != null val canceledFlows = suite.flows .filter { it.status == FlowStatus.CANCELED } + val shardPrefix = suite.shardIndex?.let { "[shard ${it + 1}] " }.orEmpty() if (suite.status == FlowStatus.ERROR || hasError) { val failedFlows = suite.flows .filter { it.status == FlowStatus.ERROR } PrintUtils.err( - "${failedFlows.size}/${suite.flows.size} ${flowWord(failedFlows.size)} Failed", + "${shardPrefix}${failedFlows.size}/${suite.flows.size} ${flowWord(failedFlows.size)} Failed", bold = true, ) if (canceledFlows.isNotEmpty()) { - PrintUtils.warn("${canceledFlows.size} ${flowWord(canceledFlows.size)} Canceled") + PrintUtils.warn("${shardPrefix}${canceledFlows.size} ${flowWord(canceledFlows.size)} Canceled") } } else { @@ -66,16 +62,16 @@ object TestSuiteStatusView { if (passedFlows.isNotEmpty()) { val durationMessage = suite.duration?.let { " in $it" } ?: "" PrintUtils.success( - "${passedFlows.size}/${suite.flows.size} ${flowWord(passedFlows.size)} Passed$durationMessage", + "${shardPrefix}${passedFlows.size}/${suite.flows.size} ${flowWord(passedFlows.size)} Passed$durationMessage", bold = true, ) if (canceledFlows.isNotEmpty()) { - PrintUtils.warn("${canceledFlows.size} ${flowWord(canceledFlows.size)} Canceled") + PrintUtils.warn("${shardPrefix}${canceledFlows.size} ${flowWord(canceledFlows.size)} Canceled") } } else { println() - PrintUtils.err("All flows were canceled") + PrintUtils.err("${shardPrefix}All flows were canceled") } } println() @@ -143,6 +139,7 @@ object TestSuiteStatusView { val status: FlowStatus, val flows: List, val duration: Duration? = null, + val shardIndex: Int? = null, val uploadDetails: UploadDetails? = null, ) { @@ -151,6 +148,7 @@ object TestSuiteStatusView { val status: FlowStatus, val duration: Duration? = null, val error: String? = null, + val shardIndex: Int? = null, val cancellationReason: UploadStatus.CancellationReason? = null ) diff --git a/maestro-client/src/main/java/maestro/debuglog/LogConfig.kt b/maestro-client/src/main/java/maestro/debuglog/LogConfig.kt index 4129f6cce0..d4b1f3a644 100644 --- a/maestro-client/src/main/java/maestro/debuglog/LogConfig.kt +++ b/maestro-client/src/main/java/maestro/debuglog/LogConfig.kt @@ -6,9 +6,7 @@ import ch.qos.logback.classic.encoder.PatternLayoutEncoder import ch.qos.logback.classic.spi.ILoggingEvent import ch.qos.logback.core.FileAppender import ch.qos.logback.core.status.NopStatusListener -import maestro.Driver import org.slf4j.LoggerFactory -import java.util.Properties object LogConfig { private const val LOG_PATTERN = "[%-5level] %logger{36} - %msg%n" diff --git a/maestro-client/src/main/java/maestro/utils/ScreenshotUtils.kt b/maestro-client/src/main/java/maestro/utils/ScreenshotUtils.kt index ed8bb517b7..90128b30e9 100644 --- a/maestro-client/src/main/java/maestro/utils/ScreenshotUtils.kt +++ b/maestro-client/src/main/java/maestro/utils/ScreenshotUtils.kt @@ -14,13 +14,13 @@ class ScreenshotUtils { private val LOGGER = LoggerFactory.getLogger(ScreenshotUtils::class.java) fun takeScreenshot(out: Sink, compressed: Boolean, driver: Driver) { - LOGGER.info("Taking screenshot to output sink") + LOGGER.trace("Taking screenshot to output sink") driver.takeScreenshot(out, compressed) } fun takeScreenshot(compressed: Boolean, driver: Driver): ByteArray { - LOGGER.info("Taking screenshot to byte array") + LOGGER.trace("Taking screenshot to byte array") val buffer = Buffer() takeScreenshot(buffer, compressed, driver) @@ -99,4 +99,4 @@ class ScreenshotUtils { return ViewHierarchy.from(driver, false) } } -} \ No newline at end of file +} From ed1a56a3b8bdd5f7f741049764d157a760fca061 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Wed, 4 Sep 2024 19:01:56 +0100 Subject: [PATCH 11/15] Fix single shard case --- maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index 8be28a3e42..ffa78b2142 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -333,10 +333,10 @@ class TestCommand : Callable { val maestro = session.maestro val device = session.device - val isReplicatingSingleTest = shardAll != null && effectiveShards > 1 && flowFiles.isSingleFile + val isReplicatingSingleFile = shardAll != null && effectiveShards > 1 && flowFiles.isSingleFile val isMultipleFiles = flowFiles.isSingleFile.not() val isAskingForReport = format != ReportFormat.NOOP - if (isMultipleFiles || isAskingForReport || isReplicatingSingleTest) { + if (isMultipleFiles || isAskingForReport || isReplicatingSingleFile) { if (continuous) { throw CommandLine.ParameterException( commandSpec.commandLine(), From be9ab4ebb8a51d96ece1155d307105f49dce5be7 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Wed, 4 Sep 2024 20:05:42 +0100 Subject: [PATCH 12/15] Remove device creation code --- .../java/maestro/cli/command/TestCommand.kt | 129 ++---------------- 1 file changed, 11 insertions(+), 118 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index ffa78b2142..aea8129603 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -19,24 +19,17 @@ package maestro.cli.command -import io.ktor.util.collections.ConcurrentSet import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll -import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.sync.Semaphore import maestro.Maestro import maestro.cli.App import maestro.cli.CliError import maestro.cli.DisableAnsiMixin import maestro.cli.ShowHelpMixin import maestro.cli.device.Device -import maestro.cli.device.DeviceCreateUtil import maestro.cli.device.DeviceService -import maestro.cli.device.PickDeviceInteractor -import maestro.cli.device.PickDeviceView -import maestro.cli.model.DeviceStartOptions import maestro.cli.model.TestExecutionSummary import maestro.cli.report.ReportFormat import maestro.cli.report.ReporterFactory @@ -61,11 +54,9 @@ import java.io.File import java.nio.file.Path import java.util.concurrent.Callable import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.CountDownLatch import kotlin.io.path.absolutePathString import kotlin.math.roundToInt import kotlin.system.exitProcess -import kotlin.time.Duration.Companion.seconds import kotlinx.coroutines.withContext import maestro.utils.isSingleFile import maestro.orchestra.util.Env.withDefaultEnvVars @@ -163,10 +154,7 @@ class TestCommand : Callable { @CommandLine.Spec lateinit var commandSpec: CommandLine.Model.CommandSpec - private val deviceCreationSemaphore = Semaphore(1) private val usedPorts = ConcurrentHashMap() - private val initialActiveDevices = ConcurrentSet() - private val currentActiveDevices = ConcurrentSet() private val logger = LoggerFactory.getLogger(TestCommand::class.java) private fun isWebFlow(): Boolean { @@ -220,20 +208,19 @@ class TestCommand : Callable { 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 = getPassedOptionsDeviceIds() - - val activeDevices = DeviceService.listConnectedDevices().map { it.instanceId } - initialActiveDevices.addAll(activeDevices) - - val availableDevices = if (deviceIds.isNotEmpty()) deviceIds.size else initialActiveDevices.size - var effectiveShards = when { + val deviceIds = getPassedOptionsDeviceIds().ifEmpty { + DeviceService.listConnectedDevices().map { it.instanceId } + } + val effectiveShards = when { onlySequenceFlows -> 1 - shardAll == null -> requestedShards.coerceAtMost(plan.flowsToRun.size) - else -> requestedShards + shardAll == null -> requestedShards + .coerceAtMost(plan.flowsToRun.size) + shardSplit == null -> requestedShards + .coerceAtMost(deviceIds.size) + else -> 1 } val warning = "Requested $requestedShards shards, " + @@ -243,15 +230,6 @@ class TestCommand : Callable { val chunkPlans = makeChunkPlans(plan, effectiveShards, onlySequenceFlows) - val missingDevicesConfigs = mutableListOf() - if (!promptForDeviceCreation(availableDevices, effectiveShards)) { - PrintUtils.message("Continuing with only $availableDevices shards.") - effectiveShards = availableDevices - } else { - missingDevicesConfigs.addAll(createMissingDevices(availableDevices, effectiveShards)) - } - val missingDevices = (effectiveShards - availableDevices).coerceAtLeast(0) - 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" @@ -266,16 +244,12 @@ class TestCommand : Callable { } message?.let { PrintUtils.info(it) } - val barrier = CountDownLatch(effectiveShards) val results = (0 until effectiveShards).map { shardIndex -> async(Dispatchers.IO) { runShardSuite( effectiveShards = effectiveShards, deviceIds = deviceIds, shardIndex = shardIndex, - missingDevices = missingDevices, - missingDevicesConfigs = missingDevicesConfigs, - barrier = barrier, chunkPlans = chunkPlans, debugOutputPath = debugOutputPath, ) @@ -302,27 +276,14 @@ class TestCommand : Callable { effectiveShards: Int, deviceIds: List, shardIndex: Int, - missingDevices: Int, - missingDevicesConfigs: MutableList, - barrier: CountDownLatch, chunkPlans: List, debugOutputPath: Path ): Triple = withContext(Dispatchers.IO) { val driverHostPort = selectPort(effectiveShards) + val deviceId = deviceIds[shardIndex] - // Acquire lock to execute device creation block - deviceCreationSemaphore.acquire() - - val deviceId = assignDeviceToShard(deviceIds, shardIndex, missingDevices, missingDevicesConfigs, driverHostPort) logger.info("[shard ${shardIndex + 1}] Selected device $deviceId using port $driverHostPort") - // Release lock if device ID was obtained from the connected devices - deviceCreationSemaphore.release() - // Signal that this thread has reached the barrier - barrier.countDown() - // Wait for all threads/shards to complete device creation before proceeding - barrier.await() - return@withContext MaestroSessionManager.newSession( host = parent?.host, port = parent?.port, @@ -404,7 +365,7 @@ class TestCommand : Callable { val suiteResult = TestSuiteInteractor( maestro = maestro, device = device, - shardIndex = shardIndex, + shardIndex = if (chunkPlans.size == 1) null else shardIndex, reporter = ReporterFactory.buildReporter(format, testSuiteName), ).runTestSuite( executionPlan = chunkPlans[shardIndex], @@ -419,54 +380,6 @@ class TestCommand : Callable { return Triple(suiteResult.passedCount, suiteResult.totalTests, suiteResult) } - private suspend fun assignDeviceToShard( - deviceIds: List, - shardIndex: Int, - missingDevices: Int, - missingDevicesConfigs: MutableList, - driverHostPort: Int - ): String = - useDevicesPassedAsOptions(deviceIds, shardIndex) - ?: useConnectedDevices(deviceIds, missingDevices, shardIndex) - ?: createNewDevice(missingDevicesConfigs, shardIndex, driverHostPort) - - private fun useDevicesPassedAsOptions(deviceIds: List, shardIndex: Int) = - deviceIds.getOrNull(shardIndex) - - private fun useConnectedDevices( - deviceIds: List, - missingDevices: Int, - shardIndex: Int - ) = when { - deviceIds.isNotEmpty() -> null - missingDevices >= 0 -> initialActiveDevices.elementAtOrNull(shardIndex) - shardAll == null && initialActiveDevices.isNotEmpty() -> PickDeviceInteractor.pickDevice().instanceId - else -> null - } - - private suspend fun createNewDevice( - missingDevicesConfigs: MutableList, - shardIndex: Int, - driverHostPort: Int - ): String { - val cfg = missingDevicesConfigs.first() - missingDevicesConfigs.remove(cfg) - val deviceCreated = DeviceCreateUtil.getOrCreateDevice( - platform = cfg.platform, - osVersion = cfg.osVersion, - forceCreate = true, - shardIndex = shardIndex - ) - val device = DeviceService.startDevice( - device = deviceCreated, - driverHostPort = driverHostPort, - connectedDevices = initialActiveDevices + currentActiveDevices - ) - currentActiveDevices.add(device.instanceId) - delay(2.seconds) - return device.instanceId - } - private fun makeChunkPlans( plan: ExecutionPlan, effectiveShards: Int, @@ -483,26 +396,6 @@ class TestCommand : Callable { } } - private fun createMissingDevices( - availableDevices: Int, - effectiveShards: Int - ) = (availableDevices until effectiveShards).map { shardIndex -> - PrintUtils.message("Creating device for shard ${shardIndex + 1}:") - PickDeviceView.requestDeviceOptions() - } - - private fun promptForDeviceCreation(availableDevices: Int, effectiveShards: Int): Boolean { - val missingDevices = effectiveShards - availableDevices - if (missingDevices <= 0) return true - val message = """ - Found $availableDevices active devices. - Need to create or start $missingDevices more for $effectiveShards shards. Continue? y/n - """.trimIndent() - PrintUtils.message(message) - val str = readlnOrNull()?.lowercase() - return str?.isBlank() == true || str == "y" || str == "yes" - } - private fun getPassedOptionsDeviceIds(): List { val arguments = if (isWebFlow()) { PrintUtils.warn("Web support is an experimental feature and may be removed in future versions.\n") From 9238f97b2a707afde00b5551eb0cd7824c206b6d Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Wed, 4 Sep 2024 20:16:10 +0100 Subject: [PATCH 13/15] Remove count from --shard-all --- .../java/maestro/cli/command/TestCommand.kt | 49 +++++++++++-------- 1 file changed, 29 insertions(+), 20 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index aea8129603..6860eb0bdf 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -100,9 +100,9 @@ class TestCommand : Callable { @Option( names = ["--shard-all"], - description = ["Run all the tests across N connected devices"], + description = ["Run all the tests across all the connected devices"], ) - private var shardAll: Int? = null + private var shardAll: Boolean = false @Option(names = ["-c", "--continuous"]) private var continuous: Boolean = false @@ -173,7 +173,7 @@ class TestCommand : Callable { printToConsole = parent?.verbose == true, ) - if (shardSplit != null && shardAll != null) { + if (shardSplit != null && shardAll) { throw CliError("Options --shard-split and --shard-all are mutually exclusive.") } @@ -204,42 +204,51 @@ class TestCommand : Callable { } private fun handleSessions(debugOutputPath: Path, plan: ExecutionPlan): Int = runBlocking(Dispatchers.IO) { - val requestedShards = shardSplit ?: shardAll ?: 1 + val deviceIds = getPassedOptionsDeviceIds().ifEmpty { + DeviceService.listConnectedDevices().map { it.instanceId } + } + + val deviceCount = deviceIds.size + val flowsToRun = plan.flowsToRun.size + val requestedShards = if (shardAll) deviceCount else shardSplit ?: 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 = getPassedOptionsDeviceIds().ifEmpty { - DeviceService.listConnectedDevices().map { it.instanceId } - } val effectiveShards = when { onlySequenceFlows -> 1 - shardAll == null -> requestedShards - .coerceAtMost(plan.flowsToRun.size) - shardSplit == null -> requestedShards - .coerceAtMost(deviceIds.size) + shardAll -> requestedShards + .coerceAtMost(deviceCount) + shardSplit != null -> requestedShards + .coerceAtMost(flowsToRun) + .coerceAtMost(deviceCount) else -> 1 } - val warning = "Requested $requestedShards shards, " + - "but it cannot be higher than the number of flows (${plan.flowsToRun.size}). " + - "Will use $effectiveShards shards instead." - if (shardAll == null && requestedShards > plan.flowsToRun.size) PrintUtils.warn(warning) + val notEnoughFlows = requestedShards > flowsToRun + val notEnoughDevices = requestedShards > deviceCount + val reason = + if (notEnoughFlows) "but it cannot be higher than the number of flows ($flowsToRun)" + else if (notEnoughDevices) "but it cannot be higher than the number of connected or specified devices ($deviceCount)" + else "" + val warning = "Requested $requestedShards shards, $reason. Will use $effectiveShards shards instead." + if (notEnoughFlows || notEnoughDevices) PrintUtils.warn(warning) val chunkPlans = makeChunkPlans(plan, effectiveShards, onlySequenceFlows) - val flowCount = if (onlySequenceFlows) plan.sequence.flows.size else plan.flowsToRun.size + val flowCount = if (onlySequenceFlows) plan.sequence.flows.size else flowsToRun val message = when { - shardAll != null -> "Will run $effectiveShards shards, with all $flowCount flows in each shard" + shardAll -> "Will run $effectiveShards shards, with all $flowCount flows in each shard" shardSplit != null -> { val flowsPerShard = (flowCount.toFloat() / effectiveShards).roundToInt() val isApprox = flowCount % effectiveShards != 0 val prefix = if (isApprox) "approx. " else "" "Will split $flowCount flows across $effectiveShards shards (${prefix}$flowsPerShard flows per shard)" } - else -> null } message?.let { PrintUtils.info(it) } @@ -294,7 +303,7 @@ class TestCommand : Callable { val maestro = session.maestro val device = session.device - val isReplicatingSingleFile = shardAll != null && effectiveShards > 1 && flowFiles.isSingleFile + val isReplicatingSingleFile = shardAll && effectiveShards > 1 && flowFiles.isSingleFile val isMultipleFiles = flowFiles.isSingleFile.not() val isAskingForReport = format != ReportFormat.NOOP if (isMultipleFiles || isAskingForReport || isReplicatingSingleFile) { @@ -386,7 +395,7 @@ class TestCommand : Callable { onlySequenceFlows: Boolean, ) = when { onlySequenceFlows -> listOf(plan) // We only want to run sequential flows in this case. - shardAll != null -> (0 until effectiveShards).reversed().map { plan.copy() } + shardAll -> (0 until effectiveShards).reversed().map { plan.copy() } else -> plan.flowsToRun .withIndex() .groupBy { it.index % effectiveShards } From 11742907366e324741f897ea58e46c953571f4d5 Mon Sep 17 00:00:00 2001 From: Tarek Belkahia Date: Wed, 4 Sep 2024 21:53:15 +0100 Subject: [PATCH 14/15] Revert "Remove count from --shard-all" This reverts commit 9238f97b2a707afde00b5551eb0cd7824c206b6d. --- .../java/maestro/cli/command/TestCommand.kt | 49 ++++++++----------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index 6860eb0bdf..aea8129603 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -100,9 +100,9 @@ class TestCommand : Callable { @Option( names = ["--shard-all"], - description = ["Run all the tests across all the connected devices"], + description = ["Run all the tests across N connected devices"], ) - private var shardAll: Boolean = false + private var shardAll: Int? = null @Option(names = ["-c", "--continuous"]) private var continuous: Boolean = false @@ -173,7 +173,7 @@ class TestCommand : Callable { printToConsole = parent?.verbose == true, ) - if (shardSplit != null && shardAll) { + if (shardSplit != null && shardAll != null) { throw CliError("Options --shard-split and --shard-all are mutually exclusive.") } @@ -204,51 +204,42 @@ class TestCommand : Callable { } private fun handleSessions(debugOutputPath: Path, plan: ExecutionPlan): Int = runBlocking(Dispatchers.IO) { - val deviceIds = getPassedOptionsDeviceIds().ifEmpty { - DeviceService.listConnectedDevices().map { it.instanceId } - } - - val deviceCount = deviceIds.size - val flowsToRun = plan.flowsToRun.size - val requestedShards = if (shardAll) deviceCount else shardSplit ?: 1 - + 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 = getPassedOptionsDeviceIds().ifEmpty { + DeviceService.listConnectedDevices().map { it.instanceId } + } val effectiveShards = when { onlySequenceFlows -> 1 - shardAll -> requestedShards - .coerceAtMost(deviceCount) - shardSplit != null -> requestedShards - .coerceAtMost(flowsToRun) - .coerceAtMost(deviceCount) + shardAll == null -> requestedShards + .coerceAtMost(plan.flowsToRun.size) + shardSplit == null -> requestedShards + .coerceAtMost(deviceIds.size) else -> 1 } - val notEnoughFlows = requestedShards > flowsToRun - val notEnoughDevices = requestedShards > deviceCount - val reason = - if (notEnoughFlows) "but it cannot be higher than the number of flows ($flowsToRun)" - else if (notEnoughDevices) "but it cannot be higher than the number of connected or specified devices ($deviceCount)" - else "" - val warning = "Requested $requestedShards shards, $reason. Will use $effectiveShards shards instead." - if (notEnoughFlows || notEnoughDevices) PrintUtils.warn(warning) + val warning = "Requested $requestedShards shards, " + + "but it cannot be higher than the number of flows (${plan.flowsToRun.size}). " + + "Will use $effectiveShards shards instead." + if (shardAll == null && requestedShards > plan.flowsToRun.size) PrintUtils.warn(warning) val chunkPlans = makeChunkPlans(plan, effectiveShards, onlySequenceFlows) - val flowCount = if (onlySequenceFlows) plan.sequence.flows.size else flowsToRun + val flowCount = if (onlySequenceFlows) plan.sequence.flows.size else plan.flowsToRun.size val message = when { - shardAll -> "Will run $effectiveShards shards, with all $flowCount flows in each shard" + shardAll != null -> "Will run $effectiveShards shards, with all $flowCount flows in each shard" shardSplit != null -> { val flowsPerShard = (flowCount.toFloat() / effectiveShards).roundToInt() val isApprox = flowCount % effectiveShards != 0 val prefix = if (isApprox) "approx. " else "" "Will split $flowCount flows across $effectiveShards shards (${prefix}$flowsPerShard flows per shard)" } + else -> null } message?.let { PrintUtils.info(it) } @@ -303,7 +294,7 @@ class TestCommand : Callable { val maestro = session.maestro val device = session.device - val isReplicatingSingleFile = shardAll && effectiveShards > 1 && flowFiles.isSingleFile + val isReplicatingSingleFile = shardAll != null && effectiveShards > 1 && flowFiles.isSingleFile val isMultipleFiles = flowFiles.isSingleFile.not() val isAskingForReport = format != ReportFormat.NOOP if (isMultipleFiles || isAskingForReport || isReplicatingSingleFile) { @@ -395,7 +386,7 @@ class TestCommand : Callable { onlySequenceFlows: Boolean, ) = when { onlySequenceFlows -> listOf(plan) // We only want to run sequential flows in this case. - shardAll -> (0 until effectiveShards).reversed().map { plan.copy() } + shardAll != null -> (0 until effectiveShards).reversed().map { plan.copy() } else -> plan.flowsToRun .withIndex() .groupBy { it.index % effectiveShards } From cbda9fa170a9e7cfee369c4f4812c4544f80c77b Mon Sep 17 00:00:00 2001 From: Bartek Pacia Date: Thu, 5 Sep 2024 14:33:57 +0200 Subject: [PATCH 15/15] handle more edge cases --- .../java/maestro/cli/command/TestCommand.kt | 49 ++++++++++++++----- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt index aea8129603..5b41714f42 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -208,18 +208,36 @@ class TestCommand : Callable { 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 = getPassedOptionsDeviceIds().ifEmpty { - DeviceService.listConnectedDevices().map { it.instanceId } + val availableDevices = DeviceService.listConnectedDevices().map { it.instanceId }.toSet() + val deviceIds = getPassedOptionsDeviceIds() + .filter { device -> + if (device !in availableDevices) { + throw CliError("Device $device was requested, but it is not connected.") + } else { + true + } + } + .ifEmpty { availableDevices } + .toList() + + val missingDevices = requestedShards - deviceIds.size + if (missingDevices > 0) { + PrintUtils.warn("Want to use ${deviceIds.size} devices, which is not enough to run $requestedShards shards. Missing $missingDevices device(s).") + throw CliError("Not enough devices connected ($missingDevices) to run the requested number of shards ($requestedShards).") } + val effectiveShards = when { + onlySequenceFlows -> 1 - shardAll == null -> requestedShards - .coerceAtMost(plan.flowsToRun.size) - shardSplit == null -> requestedShards - .coerceAtMost(deviceIds.size) + + shardAll == null -> requestedShards.coerceAtMost(plan.flowsToRun.size) + + shardSplit == null -> requestedShards.coerceAtMost(deviceIds.size) + else -> 1 } @@ -277,7 +295,7 @@ class TestCommand : Callable { deviceIds: List, shardIndex: Int, chunkPlans: List, - debugOutputPath: Path + debugOutputPath: Path, ): Triple = withContext(Dispatchers.IO) { val driverHostPort = selectPort(effectiveShards) val deviceId = deviceIds[shardIndex] @@ -334,23 +352,28 @@ class TestCommand : Callable { val resultView = if (DisableAnsiMixin.ansiEnabled) AnsiResultView() else PlainTextResultView() + env = env .withInjectedShellEnvVars() .withDefaultEnvVars(flowFile) + val resultSingle = TestRunner.runSingle( - maestro, - device, - flowFile, - env, - resultView, - debugOutputPath + maestro = maestro, + device = device, + flowFile = flowFile, + env = env, + resultView = resultView, + debugOutputPath = debugOutputPath, ) + if (resultSingle == 1) { printExitDebugMessage() } + if (!flattenDebugOutput) { TestDebugReporter.deleteOldFiles() } + val result = if (resultSingle == 0) 1 else 0 return Triple(result, 1, null) }