Fix device selection (#1953)

* Fix coding style * Fix device picking logic * Add some logs * Prompt user for device creation. * Refactor TestCommand file * Fix missing import * Fix issues with shard-all and 1 test * TestCommand: minor formatting improvements, use named args more * Fix initial state of param * Add shard info to logs * Fix single shard case * Remove device creation code * Remove count from --shard-all * Revert "Remove count from --shard-all" This reverts commit 9238f97. * handle more edge cases --------- Co-authored-by: Bartek Pacia <[email protected]>
mobile-dev-inc · Sep 5, 2024 · 2d9e106 · 2d9e106
1 parent 9d81a85
commit 2d9e106
Show file tree

Hide file tree

Showing 9 changed files with 348 additions and 364 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
diff --git 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,
         )
     }
-}
+}
diff --git 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/MaestroCommandRunner.kt b/maestro-cli/src/main/java/maestro/cli/runner/MaestroCommandRunner.kt
@@ -34,11 +34,9 @@ import maestro.orchestra.MaestroCommand
 import maestro.orchestra.Orchestra
 import maestro.orchestra.yaml.YamlCommandReader
 import maestro.utils.Insight
-import okio.Buffer
-import okio.sink
 import org.slf4j.LoggerFactory
-import java.io.File
 import java.util.IdentityHashMap
+import maestro.cli.util.ScreenshotUtils
 
 /**
  * Knows how to run a list of Maestro commands and update the UI.
@@ -64,40 +62,6 @@ object MaestroCommandRunner {
         val commandStatuses = IdentityHashMap<MaestroCommand, CommandStatus>()
         val commandMetadata = IdentityHashMap<MaestroCommand, Orchestra.CommandMetadata>()
 
-        fun takeDebugScreenshot(status: CommandStatus): File? {
-            val containsFailed = debugOutput.screenshots.any { it.status == CommandStatus.FAILED }
-
-            // Avoids duplicate failed images from parent commands
-            if (containsFailed && status == CommandStatus.FAILED) {
-                return null
-            }
-
-            val result = kotlin.runCatching {
-                val out = File
-                    .createTempFile("screenshot-${System.currentTimeMillis()}", ".png")
-                    .also { it.deleteOnExit() } // save to another dir before exiting
-                maestro.takeScreenshot(out.sink(), false)
-                debugOutput.screenshots.add(
-                    FlowDebugOutput.Screenshot(
-                        screenshot = out,
-                        timestamp = System.currentTimeMillis(),
-                        status = status
-                    )
-                )
-                out
-            }
-
-            return result.getOrNull()
-        }
-
-        fun writeAIscreenshot(buffer: Buffer): File {
-            val out = File
-                .createTempFile("ai-screenshot-${System.currentTimeMillis()}", ".png")
-                .also { it.deleteOnExit() }
-            out.outputStream().use { it.write(buffer.readByteArray()) }
-            return out
-        }
-
         fun refreshUi() {
             view.setState(
                 UiState.Running(
@@ -151,7 +115,7 @@ object MaestroCommandRunner {
                     error = e
                 }
 
-                takeDebugScreenshot(CommandStatus.FAILED)
+                ScreenshotUtils.takeDebugScreenshot(maestro, debugOutput, CommandStatus.FAILED)
 
                 if (e !is MaestroException) {
                     throw e
@@ -179,7 +143,7 @@ object MaestroCommandRunner {
                     status = CommandStatus.WARNED
                 }
 
-                takeDebugScreenshot(CommandStatus.WARNED)
+                ScreenshotUtils.takeDebugScreenshot(maestro, debugOutput, CommandStatus.WARNED)
 
                 refreshUi()
             },
@@ -198,7 +162,7 @@ object MaestroCommandRunner {
             },
             onCommandGeneratedOutput = { command, defects, screenshot ->
                 logger.info("${command.description()} generated output")
-                val screenshotPath = writeAIscreenshot(screenshot)
+                val screenshotPath = ScreenshotUtils.writeAIscreenshot(screenshot)
                 aiOutput.screenOutputs.add(
                     SingleScreenFlowAIOutput(
                         screenshotPath = screenshotPath,