diff --git a/maestro-cli/build.gradle.kts b/maestro-cli/build.gradle.kts index b5df3afaa9..a880cecd5b 100644 --- a/maestro-cli/build.gradle.kts +++ b/maestro-cli/build.gradle.kts @@ -225,6 +225,10 @@ tasks.named("compileKotlin", KotlinCompilationTask::class.java) { } } +tasks.named("test") { + useJUnitPlatform() +} + tasks.create("createProperties") { dependsOn("processResources") diff --git a/maestro-cli/src/main/java/maestro/cli/App.kt b/maestro-cli/src/main/java/maestro/cli/App.kt index 722a83bf99..5393e38a68 100644 --- a/maestro-cli/src/main/java/maestro/cli/App.kt +++ b/maestro-cli/src/main/java/maestro/cli/App.kt @@ -94,6 +94,9 @@ class App { @Option(names = ["--port"], hidden = true) var port: Int? = null + @Option(names = ["--driver-host-port"], hidden = true) + var driverHostPort: Int? = null + @Option( names = ["--device", "--udid"], description = ["(Optional) Device ID to run on explicitly, can be a comma separated list of IDs: --device \"Emulator_1,Emulator_2\" "], diff --git a/maestro-cli/src/main/java/maestro/cli/command/PrintHierarchyCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/PrintHierarchyCommand.kt index 797dc0b1e8..5c81c22812 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/PrintHierarchyCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/PrintHierarchyCommand.kt @@ -64,12 +64,12 @@ class PrintHierarchyCommand : Runnable { @CommandLine.Option( names = ["--reinstall-driver"], - description = ["Reinstalls driver before running the test. On iOS, reinstalls xctestrunner driver. On Android, reinstalls both driver and server apps. Set to false to skip reinstallation."], + description = ["Force reinstall of the driver before running the test. On iOS, reinstalls xctestrunner driver. On Android, reinstalls both driver and server apps. By default, reuses an existing healthy driver."], negatable = true, - defaultValue = "true", - fallbackValue = "true" + defaultValue = "false", + fallbackValue = "true" ) - private var reinstallDriver: Boolean = true + private var reinstallDriver: Boolean = false @Option( names = ["--apple-team-id"], @@ -108,7 +108,7 @@ class PrintHierarchyCommand : Runnable { MaestroSessionManager.newSession( host = parent?.host, port = parent?.port, - driverHostPort = null, + driverHostPort = parent?.driverHostPort, teamId = appleTeamId, deviceId = parent?.deviceId, platform = parent?.platform, diff --git a/maestro-cli/src/main/java/maestro/cli/command/QueryCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/QueryCommand.kt index 2c6d016f1c..af1c47207b 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/QueryCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/QueryCommand.kt @@ -73,7 +73,7 @@ class QueryCommand : Runnable { MaestroSessionManager.newSession( host = parent?.host, port = parent?.port, - driverHostPort = null, + driverHostPort = parent?.driverHostPort, deviceId = parent?.deviceId, platform = parent?.platform, teamId = appleTeamId, diff --git a/maestro-cli/src/main/java/maestro/cli/command/RecordCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/RecordCommand.kt index dd721e8110..2405bfb94e 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/RecordCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/RecordCommand.kt @@ -130,7 +130,7 @@ class RecordCommand : Callable { return MaestroSessionManager.newSession( host = parent?.host, port = parent?.port, - driverHostPort = null, + driverHostPort = parent?.driverHostPort, deviceId = deviceId, teamId = appleTeamId, platform = parent?.platform, diff --git a/maestro-cli/src/main/java/maestro/cli/command/StudioCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/StudioCommand.kt index 6797cc75b6..310f6ae0d7 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/StudioCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/StudioCommand.kt @@ -75,7 +75,7 @@ class StudioCommand : Callable { MaestroSessionManager.newSession( host = parent?.host, port = parent?.port, - driverHostPort = null, + driverHostPort = parent?.driverHostPort, teamId = appleTeamId, deviceId = parent?.deviceId, platform = parent?.platform, 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 f55271fba4..15424583eb 100644 --- a/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt +++ b/maestro-cli/src/main/java/maestro/cli/command/TestCommand.kt @@ -48,6 +48,7 @@ import maestro.cli.runner.resultview.AnsiResultView import maestro.cli.runner.resultview.PlainTextResultView import maestro.cli.session.MaestroSessionManager import maestro.cli.util.CiUtils +import maestro.cli.util.isPortAvailable import maestro.cli.util.EnvUtils import maestro.cli.util.FileUtils.isWebFlow import maestro.cli.util.PrintUtils @@ -204,12 +205,12 @@ class TestCommand : Callable { @Option( names = ["--reinstall-driver"], - description = ["Reinstalls driver before running the test. On iOS, reinstalls xctestrunner driver. On Android, reinstalls both driver and server apps. Set to false to skip reinstallation."], + description = ["Force reinstall of the driver before running the test. On iOS, reinstalls xctestrunner driver. On Android, reinstalls both driver and server apps. By default, reuses an existing healthy driver."], negatable = true, - defaultValue = "true", + defaultValue = "false", fallbackValue = "true" ) - private var reinstallDriver: Boolean = true + private var reinstallDriver: Boolean = false @Option( names = ["--apple-team-id"], @@ -226,7 +227,10 @@ class TestCommand : Callable { description = ["Device ID to run on explicitly, can be a comma separated list of IDs: --device \"Emulator_1,Emulator_2\" "], ) var deviceId: String? = null - + + @Option(names = ["--driver-host-port"], hidden = true) + var driverHostPort: Int? = null + @CommandLine.Spec lateinit var commandSpec: CommandLine.Model.CommandSpec @@ -532,11 +536,18 @@ class TestCommand : Callable { } } - private fun selectPort(effectiveShards: Int): Int = - if (effectiveShards == 1) 7001 - else (7001..7128).shuffled().find { port -> - usedPorts.putIfAbsent(port, true) == null + private fun selectPort(effectiveShards: Int): Int { + val userPort = driverHostPort ?: parent?.driverHostPort + if (userPort != null) { + if (!isPortAvailable(userPort)) { + throw CliError("Requested driver host port $userPort is not available") + } + return userPort + } + return (7001..7128).shuffled().find { port -> + isPortAvailable(port) && usedPorts.putIfAbsent(port, true) == null } ?: error("No available ports found") + } private fun runSingleFlow( maestro: Maestro, diff --git a/maestro-cli/src/main/java/maestro/cli/db/KeyValueStore.kt b/maestro-cli/src/main/java/maestro/cli/db/KeyValueStore.kt index 4998d499d8..8ab10e0d44 100644 --- a/maestro-cli/src/main/java/maestro/cli/db/KeyValueStore.kt +++ b/maestro-cli/src/main/java/maestro/cli/db/KeyValueStore.kt @@ -1,6 +1,8 @@ package maestro.cli.db import java.io.File +import java.io.RandomAccessFile +import java.nio.channels.FileLock import java.util.concurrent.locks.ReentrantReadWriteLock import kotlin.concurrent.read import kotlin.concurrent.write @@ -12,25 +14,31 @@ class KeyValueStore(private val dbFile: File) { dbFile.createNewFile() } - fun get(key: String): String? = lock.read { getCurrentDB()[key] } + fun get(key: String): String? = lock.read { withFileLock { getCurrentDB()[key] } } fun set(key: String, value: String) = lock.write { - val db = getCurrentDB() - db[key] = value - commit(db) + withFileLock { + val db = getCurrentDB() + db[key] = value + commit(db) + } } fun delete(key: String) = lock.write { - val db = getCurrentDB() - db.remove(key) - commit(db) + withFileLock { + val db = getCurrentDB() + db.remove(key) + commit(db) + } } - fun keys(): List = lock.read { getCurrentDB().keys.toList() } + fun keys(): List = lock.read { withFileLock { getCurrentDB().keys.toList() } } private fun getCurrentDB(): MutableMap { + if (dbFile.length() == 0L) return mutableMapOf() return dbFile .readLines() + .filter { it.contains("=") } .associate { line -> val (key, value) = line.split("=", limit = 2) key to value @@ -44,4 +52,19 @@ class KeyValueStore(private val dbFile: File) { .joinToString("\n") ) } + + private fun withFileLock(block: () -> T): T { + val raf = RandomAccessFile(dbFile, "rw") + return try { + val channel = raf.channel + val fileLock: FileLock = channel.lock() + try { + block() + } finally { + fileLock.release() + } + } finally { + raf.close() + } + } } diff --git a/maestro-cli/src/main/java/maestro/cli/session/MaestroSessionManager.kt b/maestro-cli/src/main/java/maestro/cli/session/MaestroSessionManager.kt index d62d063409..229fc877f7 100644 --- a/maestro-cli/src/main/java/maestro/cli/session/MaestroSessionManager.kt +++ b/maestro-cli/src/main/java/maestro/cli/session/MaestroSessionManager.kt @@ -56,7 +56,9 @@ import kotlin.io.path.pathString object MaestroSessionManager { private const val defaultHost = "localhost" - private const val defaultXctestHost = "127.0.0.1" + // Use localhost (not 127.0.0.1) so name resolution can fall back to IPv6 + // when the XCTest runner happens to bind to ::1 only. See #1299. + private const val defaultXctestHost = "localhost" private const val defaultXcTestPort = 22087 private val executor = Executors.newScheduledThreadPool(1) @@ -73,7 +75,7 @@ object MaestroSessionManager { isStudio: Boolean = false, isHeadless: Boolean = false, screenSize: String? = null, - reinstallDriver: Boolean = true, + reinstallDriver: Boolean = false, deviceIndex: Int? = null, executionPlan: WorkspaceExecutionPlanner.ExecutionPlan? = null, block: (MaestroSession) -> T, @@ -88,12 +90,12 @@ object MaestroSessionManager { deviceIndex = deviceIndex, ) val sessionId = UUID.randomUUID().toString() + val effectiveDeviceId = selectedDevice.device?.instanceId ?: selectedDevice.deviceId val heartbeatFuture = executor.scheduleAtFixedRate( { try { - Thread.sleep(1000) // Add a 1-second delay here for fixing race condition - SessionStore.heartbeat(sessionId, selectedDevice.platform) + SessionStore.default.heartbeat(sessionId, selectedDevice.platform, effectiveDeviceId) } catch (e: Exception) { logger.error("Failed to record heartbeat", e) } @@ -108,9 +110,10 @@ object MaestroSessionManager { connectToExistingSession = if (isStudio) { false } else { - SessionStore.hasActiveSessions( + SessionStore.default.hasActiveSessionForDevice( sessionId, - selectedDevice.platform + selectedDevice.platform, + effectiveDeviceId ) }, isStudio = isStudio, @@ -122,9 +125,9 @@ object MaestroSessionManager { ) Runtime.getRuntime().addShutdownHook(thread(start = false) { heartbeatFuture.cancel(true) - SessionStore.delete(sessionId, selectedDevice.platform) + SessionStore.default.delete(sessionId, selectedDevice.platform, effectiveDeviceId) runCatching { ScreenReporter.reportMaxDepth() } - if (SessionStore.activeSessions().isEmpty()) { + if (SessionStore.default.shouldCloseSession(selectedDevice.platform, effectiveDeviceId)) { session.close() } }) diff --git a/maestro-cli/src/main/java/maestro/cli/session/SessionStore.kt b/maestro-cli/src/main/java/maestro/cli/session/SessionStore.kt index de28a4a097..f667ac142c 100644 --- a/maestro-cli/src/main/java/maestro/cli/session/SessionStore.kt +++ b/maestro-cli/src/main/java/maestro/cli/session/SessionStore.kt @@ -5,21 +5,12 @@ import maestro.device.Platform import java.nio.file.Paths import java.util.concurrent.TimeUnit -object SessionStore { +class SessionStore(private val keyValueStore: KeyValueStore) { - private val keyValueStore by lazy { - KeyValueStore( - dbFile = Paths - .get(System.getProperty("user.home"), ".maestro", "sessions") - .toFile() - .also { it.parentFile.mkdirs() } - ) - } - - fun heartbeat(sessionId: String, platform: Platform) { + fun heartbeat(sessionId: String, platform: Platform, deviceId: String? = null) { synchronized(keyValueStore) { keyValueStore.set( - key = key(sessionId, platform), + key = key(sessionId, platform, deviceId), value = System.currentTimeMillis().toString(), ) @@ -37,10 +28,10 @@ object SessionStore { } } - fun delete(sessionId: String, platform: Platform) { + fun delete(sessionId: String, platform: Platform, deviceId: String? = null) { synchronized(keyValueStore) { keyValueStore.delete( - key(sessionId, platform) + key(sessionId, platform, deviceId) ) } } @@ -56,18 +47,44 @@ object SessionStore { } } - fun hasActiveSessions( + fun shouldCloseSession(platform: Platform, deviceId: String? = null): Boolean { + return activeSessionsForDevice(platform, deviceId).isEmpty() + } + + fun activeSessionsForDevice(platform: Platform, deviceId: String? = null): List { + val devicePrefix = "${platform}_${deviceId ?: "unknown"}_" + synchronized(keyValueStore) { + return activeSessions().filter { it.startsWith(devicePrefix) } + } + } + + fun hasActiveSessionForDevice( sessionId: String, - platform: Platform + platform: Platform, + deviceId: String? = null ): Boolean { + val currentKey = key(sessionId, platform, deviceId) + val devicePrefix = "${platform}_${deviceId ?: "unknown"}_" synchronized(keyValueStore) { return activeSessions() - .any { it != key(sessionId, platform) } + .any { it.startsWith(devicePrefix) && it != currentKey } } } - private fun key(sessionId: String, platform: Platform): String { - return "${platform}_$sessionId" + private fun key(sessionId: String, platform: Platform, deviceId: String? = null): String { + return "${platform}_${deviceId ?: "unknown"}_$sessionId" } -} \ No newline at end of file + companion object { + val default by lazy { + SessionStore( + KeyValueStore( + dbFile = Paths + .get(System.getProperty("user.home"), ".maestro", "sessions") + .toFile() + .also { it.parentFile.mkdirs() } + ) + ) + } + } +} diff --git a/maestro-cli/src/main/java/maestro/cli/util/SocketUtils.kt b/maestro-cli/src/main/java/maestro/cli/util/SocketUtils.kt index fcbab81721..8afbc52ec5 100644 --- a/maestro-cli/src/main/java/maestro/cli/util/SocketUtils.kt +++ b/maestro-cli/src/main/java/maestro/cli/util/SocketUtils.kt @@ -2,6 +2,14 @@ package maestro.cli.util import java.net.ServerSocket +fun isPortAvailable(port: Int): Boolean { + return try { + ServerSocket(port).use { true } + } catch (e: Exception) { + false + } +} + fun getFreePort(): Int { (9999..11000).forEach { port -> try { diff --git a/maestro-cli/src/test/kotlin/maestro/cli/session/SessionStoreTest.kt b/maestro-cli/src/test/kotlin/maestro/cli/session/SessionStoreTest.kt new file mode 100644 index 0000000000..4ae53d8511 --- /dev/null +++ b/maestro-cli/src/test/kotlin/maestro/cli/session/SessionStoreTest.kt @@ -0,0 +1,124 @@ +package maestro.cli.session + +import com.google.common.truth.Truth.assertThat +import maestro.cli.db.KeyValueStore +import maestro.device.Platform +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.nio.file.Path + +class SessionStoreTest { + + @TempDir + lateinit var tempDir: Path + + private lateinit var sessionStore: SessionStore + + @BeforeEach + fun setUp() { + val sessionsFile = tempDir.resolve("sessions").toFile() + sessionStore = SessionStore(KeyValueStore(sessionsFile)) + } + + // --- Basic behavior --- + + @Test + fun `heartbeat creates a session that appears in activeSessions`() { + sessionStore.heartbeat("session-1", Platform.IOS, "device-A") + + assertThat(sessionStore.activeSessions()).hasSize(1) + } + + @Test + fun `delete removes a session from activeSessions`() { + sessionStore.heartbeat("session-1", Platform.IOS, "device-A") + sessionStore.delete("session-1", Platform.IOS, "device-A") + + assertThat(sessionStore.activeSessions()).isEmpty() + } + + // --- Task #16: Session key format migration --- + + @Test + fun `old format keys should not match hasActiveSessionForDevice`() { + // Manually write an old-format key (pre-upgrade) + val kvStore = KeyValueStore(tempDir.resolve("sessions-migration").toFile()) + kvStore.set("ANDROID_old-session-uuid", System.currentTimeMillis().toString()) + val store = SessionStore(kvStore) + + // Looking for sessions on a specific device should not match old-format keys + val hasSession = store.hasActiveSessionForDevice( + "new-session", Platform.ANDROID, "emulator-5554" + ) + + assertThat(hasSession).isFalse() + } + + // --- Per-device session isolation --- + + @Test + fun `hasActiveSessionForDevice excludes current session`() { + sessionStore.heartbeat("session-1", Platform.IOS, "device-A") + + val hasOther = sessionStore.hasActiveSessionForDevice( + "session-1", Platform.IOS, "device-A" + ) + + // Should not find itself + assertThat(hasOther).isFalse() + } + + @Test + fun `hasActiveSessionForDevice detects another session on same device`() { + sessionStore.heartbeat("session-1", Platform.IOS, "device-A") + sessionStore.heartbeat("session-2", Platform.IOS, "device-A") + + val hasOther = sessionStore.hasActiveSessionForDevice( + "session-1", Platform.IOS, "device-A" + ) + + assertThat(hasOther).isTrue() + } + + @Test + fun `hasActiveSessionForDevice ignores sessions on different devices`() { + sessionStore.heartbeat("session-1", Platform.IOS, "device-A") + sessionStore.heartbeat("session-2", Platform.IOS, "device-B") + + val hasOther = sessionStore.hasActiveSessionForDevice( + "session-1", Platform.IOS, "device-A" + ) + + // session-2 is on device-B, should not count + assertThat(hasOther).isFalse() + } + + // --- Task #17: Per-device session close --- + + @Test + fun `activeSessionsForDevice only returns sessions for that device`() { + sessionStore.heartbeat("s1", Platform.IOS, "device-A") + sessionStore.heartbeat("s2", Platform.ANDROID, "emulator-5554") + sessionStore.heartbeat("s3", Platform.IOS, "device-B") + + val deviceASessions = sessionStore.activeSessionsForDevice(Platform.IOS, "device-A") + + assertThat(deviceASessions).hasSize(1) + } + + @Test + fun `shouldCloseSession returns true when no sessions remain for device even if other devices have sessions`() { + // iOS session on device-A + sessionStore.heartbeat("ios-session", Platform.IOS, "device-A") + // Android session on emulator + sessionStore.heartbeat("android-session", Platform.ANDROID, "emulator-5554") + + // Android session shuts down + sessionStore.delete("android-session", Platform.ANDROID, "emulator-5554") + + // DESIRED: should close because no Android sessions remain on this device + val shouldClose = sessionStore.shouldCloseSession(Platform.ANDROID, "emulator-5554") + assertThat(shouldClose).isTrue() + } +} diff --git a/maestro-client/src/main/java/maestro/debuglog/DebugLogStore.kt b/maestro-client/src/main/java/maestro/debuglog/DebugLogStore.kt index 97a2cf77f7..370dce5a0e 100644 --- a/maestro-client/src/main/java/maestro/debuglog/DebugLogStore.kt +++ b/maestro-client/src/main/java/maestro/debuglog/DebugLogStore.kt @@ -30,8 +30,9 @@ object DebugLogStore { init { val dateFormatter = DateTimeFormatter.ofPattern(LOG_DIR_DATE_FORMAT) val date = dateFormatter.format(LocalDateTime.now()) + val pid = java.lang.management.ManagementFactory.getRuntimeMXBean().name.split("@")[0] - currentRunLogDirectory = File(logDirectory, date) + currentRunLogDirectory = File(logDirectory, "${date}_$pid") currentRunLogDirectory.mkdirs() removeOldLogs(logDirectory) diff --git a/maestro-client/src/main/java/maestro/drivers/AndroidDriver.kt b/maestro-client/src/main/java/maestro/drivers/AndroidDriver.kt index 155744e60d..b3051ded75 100644 --- a/maestro-client/src/main/java/maestro/drivers/AndroidDriver.kt +++ b/maestro-client/src/main/java/maestro/drivers/AndroidDriver.kt @@ -67,7 +67,7 @@ class AndroidDriver( private val dadb: Dadb, hostPort: Int? = null, private var emulatorName: String = "", - private val reinstallDriver: Boolean = true, + private val reinstallDriver: Boolean = false, private val metricsProvider: Metrics = MetricsProvider.getInstance(), ) : Driver { private var portForwarder: AutoCloseable? = null @@ -1114,7 +1114,7 @@ class AndroidDriver( metrics.measured("operation", mapOf("command" to "installMaestroDriverApp")) { if (reinstallDriver) { uninstallMaestroDriverApp() - } else if (isPackageInstalled("dev.mobile.maestro")) { + } else if (isPackageInstalled("dev.mobile.maestro") && isDriverVersionCurrent()) { return@measured } @@ -1137,7 +1137,7 @@ class AndroidDriver( private fun installMaestroServerApp() { if (reinstallDriver) { uninstallMaestroServerApp() - } else if (isPackageInstalled("dev.mobile.maestro.test")) { + } else if (isPackageInstalled("dev.mobile.maestro.test") && isDriverVersionCurrent()) { return } @@ -1159,6 +1159,43 @@ class AndroidDriver( private fun installMaestroApks() { installMaestroDriverApp() installMaestroServerApp() + saveDriverVersionHash() + } + + private val driverHashFile = File(System.getProperty("user.home"), ".maestro/android-driver-hash") + + private fun isDriverVersionCurrent(): Boolean { + return try { + if (!driverHashFile.exists()) return false + val savedHash = driverHashFile.readText() + val currentHash = computeDriverHash() + savedHash == currentHash + } catch (e: Exception) { + false + } + } + + private fun saveDriverVersionHash() { + try { + driverHashFile.parentFile.mkdirs() + driverHashFile.writeText(computeDriverHash()) + } catch (e: Exception) { + LOGGER.warn("Failed to save driver version hash", e) + } + } + + private fun computeDriverHash(): String { + val digest = java.security.MessageDigest.getInstance("SHA-256") + listOf("/maestro-app.apk", "/maestro-server.apk").forEach { resource -> + Maestro::class.java.getResourceAsStream(resource)?.use { stream -> + val buffer = ByteArray(8192) + var bytesRead: Int + while (stream.read(buffer).also { bytesRead = it } != -1) { + digest.update(buffer, 0, bytesRead) + } + } + } + return digest.digest().joinToString("") { "%02x".format(it) } } fun uninstallMaestroDriverApp() { diff --git a/maestro-client/src/main/java/maestro/drivers/IOSDriver.kt b/maestro-client/src/main/java/maestro/drivers/IOSDriver.kt index 85b3a83c80..55cf2c9719 100644 --- a/maestro-client/src/main/java/maestro/drivers/IOSDriver.kt +++ b/maestro-client/src/main/java/maestro/drivers/IOSDriver.kt @@ -194,7 +194,16 @@ class IOSDriver( attributes["accessibilityText"] = element.label attributes["title"] = element.title ?: "" attributes["value"] = element.value ?: "" - attributes["text"] = element.title?.ifEmpty { element.value } ?: "" + // Fall back to accessibility label when title and value are both empty. + // This is the common case for React Native Pressable with accessibilityLabel — + // iOS collapses child Text into the parent's accessibility label, leaving + // title and value empty. Without this fallback, `tapOn: ""` cannot + // find buttons that are clearly visible to users. See #1409. + attributes["text"] = element.title + ?.takeIf { it.isNotEmpty() } + ?: element.value + ?.takeIf { it.isNotEmpty() } + ?: element.label attributes["hintText"] = element.placeholderValue ?: "" attributes["resource-id"] = element.identifier attributes["bounds"] = element.frame.boundsString @@ -485,10 +494,23 @@ class IOSDriver( override fun waitForAppToSettle(initialHierarchy: ViewHierarchy?, appId: String?, timeoutMs: Int?): ViewHierarchy? { return metrics.measured("operation", mapOf("command" to "waitForAppToSettle", "appId" to appId.toString(), "timeoutMs" to timeoutMs.toString())) { - LOGGER.info("Waiting for animation to end with timeout $SCREEN_SETTLE_TIMEOUT_MS") - val didFinishOnTime = waitUntilScreenIsStatic(SCREEN_SETTLE_TIMEOUT_MS) - - if (didFinishOnTime) null else ScreenshotUtils.waitForAppToSettle(initialHierarchy, this, timeoutMs) + val totalTimeout = timeoutMs?.toLong() ?: SCREEN_SETTLE_TIMEOUT_MS + val startTime = System.currentTimeMillis() + LOGGER.info("Waiting for animation to end with timeout ${totalTimeout}ms") + val didFinishOnTime = waitUntilScreenIsStatic(totalTimeout) + + if (didFinishOnTime) { + null + } else { + // Use remaining time budget for hierarchy-based fallback + val elapsed = System.currentTimeMillis() - startTime + val remainingMs = (totalTimeout - elapsed).coerceAtLeast(0) + if (remainingMs > 0) { + ScreenshotUtils.waitForAppToSettle(initialHierarchy, this, remainingMs.toInt()) + } else { + initialHierarchy ?: ScreenshotUtils.waitForAppToSettle(initialHierarchy, this, 0) + } + } } } diff --git a/maestro-ios-driver/src/main/kotlin/util/LocalSimulatorUtils.kt b/maestro-ios-driver/src/main/kotlin/util/LocalSimulatorUtils.kt index 7dab5364fb..ede5fdf6f9 100644 --- a/maestro-ios-driver/src/main/kotlin/util/LocalSimulatorUtils.kt +++ b/maestro-ios-driver/src/main/kotlin/util/LocalSimulatorUtils.kt @@ -252,28 +252,54 @@ class LocalSimulatorUtils(private val tempFileHandler: TempFileHandler) { } private fun reinstallApp(deviceId: String, bundleId: String) { + val cachedBinaryPath = getCachedAppBinary(deviceId, bundleId) + + logger.info("Reinstalling $bundleId from $cachedBinaryPath") + uninstall(deviceId, bundleId) + install(deviceId, cachedBinaryPath) + logger.info("App $bundleId reinstalled") + } + + private fun getCachedAppBinary(deviceId: String, bundleId: String): Path { + val cacheDir = File(System.getProperty("user.home"), ".maestro/app-cache/$deviceId").also { it.mkdirs() } + val cachedAppPath = Path(cacheDir.absolutePath, "$bundleId.app") + val appBinaryPath = getAppBinaryDirectory(deviceId, bundleId) if (appBinaryPath.isEmpty()) { - throw SimctlError("Could not find app binary for bundle $bundleId at $appBinaryPath") + throw SimctlError("Could not find app binary for bundle $bundleId") } val pathToBinary = Path(appBinaryPath) - if (Files.isDirectory(pathToBinary)) { - val tmpDir = tempFileHandler.createTempDirectory().toPath() - val tmpBundlePath = tmpDir.resolve("$bundleId-${System.currentTimeMillis()}.app") - - logger.info("Copying app binary from $pathToBinary to $tmpBundlePath") - Files.copy(pathToBinary, tmpBundlePath) - copyDirectoryRecursively(pathToBinary, tmpBundlePath) - - logger.info("Reinstalling and launching $bundleId") - uninstall(deviceId, bundleId) - install(deviceId, tmpBundlePath) - deleteFolderRecursively(tmpBundlePath.toFile()) - logger.info("App $bundleId reinstalled and launched") - } else { + if (!Files.isDirectory(pathToBinary)) { throw SimctlError("Could not find app binary for bundle $bundleId at $pathToBinary") } + + if (Files.isDirectory(cachedAppPath) && !isAppCacheStale(cachedAppPath, pathToBinary)) { + logger.info("Using cached app binary for $bundleId on device $deviceId") + return cachedAppPath + } + + // Cache miss or stale — copy from device container + if (Files.isDirectory(cachedAppPath)) { + logger.info("Cached app binary is stale, re-caching") + deleteFolderRecursively(cachedAppPath.toFile()) + } + + logger.info("Caching app binary from $pathToBinary to $cachedAppPath") + Files.copy(pathToBinary, cachedAppPath) + copyDirectoryRecursively(pathToBinary, cachedAppPath) + + return cachedAppPath + } + + private fun isAppCacheStale(cachedPath: Path, installedPath: Path): Boolean { + // Compare Info.plist as a fast proxy for "is this the same app version?" + val cachedPlist = cachedPath.resolve("Info.plist").toFile() + val installedPlist = installedPath.resolve("Info.plist").toFile() + + if (!cachedPlist.exists() || !installedPlist.exists()) return true + + return !cachedPlist.readBytes().contentEquals(installedPlist.readBytes()) } fun clearAppState(deviceId: String, bundleId: String) { diff --git a/maestro-ios-driver/src/main/kotlin/xcuitest/XCTestDriverClient.kt b/maestro-ios-driver/src/main/kotlin/xcuitest/XCTestDriverClient.kt index bf7228b1fd..b18265257f 100644 --- a/maestro-ios-driver/src/main/kotlin/xcuitest/XCTestDriverClient.kt +++ b/maestro-ios-driver/src/main/kotlin/xcuitest/XCTestDriverClient.kt @@ -21,23 +21,30 @@ class XCTestDriverClient( connectTimeout = 1.seconds, callTimeout = 200.seconds ), - private val reinstallDriver: Boolean = true, + private val reinstallDriver: Boolean = false, ) { private val logger = LoggerFactory.getLogger(XCTestDriverClient::class.java) private lateinit var client: XCTestClient - constructor(installer: XCTestInstaller, client: XCTestClient, reinstallDriver: Boolean = true): this(installer, reinstallDriver = reinstallDriver) { + constructor(installer: XCTestInstaller, client: XCTestClient, reinstallDriver: Boolean = false): this(installer, reinstallDriver = reinstallDriver) { this.client = client } fun restartXCTestRunner() { - if(reinstallDriver) { - logger.trace("Restarting XCTest Runner (uninstalling, installing and starting)") + if (reinstallDriver) { + logger.trace("Reinstall requested — uninstalling, installing and starting XCTest Runner") installer.uninstall() - logger.trace("XCTest Runner uninstalled, will install and start it") + client = installer.start() + return } + if (installer.isChannelAlive() && installer.isVersionMatch()) { + logger.trace("XCTest Runner already running and version matches, reusing existing session") + return + } + + logger.trace("XCTest Runner not running, starting fresh") client = installer.start() } diff --git a/maestro-ios-driver/src/main/kotlin/xcuitest/installer/IOSBuildProductsExtractor.kt b/maestro-ios-driver/src/main/kotlin/xcuitest/installer/IOSBuildProductsExtractor.kt index ee6e9755cb..b881300561 100644 --- a/maestro-ios-driver/src/main/kotlin/xcuitest/installer/IOSBuildProductsExtractor.kt +++ b/maestro-ios-driver/src/main/kotlin/xcuitest/installer/IOSBuildProductsExtractor.kt @@ -15,6 +15,7 @@ import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths import java.nio.file.StandardCopyOption +import java.security.MessageDigest import kotlin.io.path.isRegularFile data class BuildProducts(val xctestRunPath: File, val uiRunnerPath: File) @@ -34,7 +35,22 @@ class IOSBuildProductsExtractor( private val LOGGER = LoggerFactory.getLogger(IOSBuildProductsExtractor::class.java) } + fun sourceHash(sourceDirectory: String): String? = computeSourceHash(sourceDirectory) + fun extract(sourceDirectory: String): BuildProducts { + val targetFile = target.toFile() + val hashFile = File(targetFile, ".source-hash") + val sourceHash = computeSourceHash(sourceDirectory) + + if (sourceHash != null && hashFile.exists() && hashFile.readText() == sourceHash) { + val xctestRun = targetFile.walkTopDown().firstOrNull { it.extension == "xctestrun" } + val uiRunner = targetFile.walkTopDown().firstOrNull { it.name == "maestro-driver-iosUITests-Runner.app" } + if (xctestRun != null && uiRunner != null) { + LOGGER.info("Build products cached and up to date, skipping extraction") + return BuildProducts(xctestRunPath = xctestRun, uiRunnerPath = uiRunner) + } + } + LOGGER.info("[Start] Writing build products") writeBuildProducts(sourceDirectory) LOGGER.info("[Done] Writing build products") @@ -47,7 +63,10 @@ class IOSBuildProductsExtractor( extractZipToApp("maestro-driver-ios.zip") LOGGER.info("[Done] Writing maestro-driver-ios app") - val targetFile = target.toFile() + if (sourceHash != null) { + hashFile.writeText(sourceHash) + } + val xctestRun = targetFile.walkTopDown().firstOrNull { it.extension == "xctestrun" } ?: throw FileNotFoundException("xctestrun config does not exist") val uiRunner = targetFile.walkTopDown().firstOrNull { it.name == "maestro-driver-iosUITests-Runner.app" } @@ -59,6 +78,42 @@ class IOSBuildProductsExtractor( ) } + private fun computeSourceHash(sourceDirectory: String): String? { + return try { + val uri = LocalXCTestInstaller::class.java.classLoader.getResource(sourceDirectory)?.toURI() + ?: return null + + val sourcePath = if (uri.scheme == "jar") { + val fs = try { + FileSystems.getFileSystem(uri) + } catch (e: FileSystemNotFoundException) { + uri.getOrCreateFileSystem() + } + fs.getPath(sourceDirectory) + } else { + Paths.get(uri) + } + + val digest = MessageDigest.getInstance("SHA-256") + Files.walk(sourcePath).use { paths -> + paths.filter { it.isRegularFile() }.sorted().forEach { file -> + digest.update(file.fileName.toString().toByteArray()) + Files.newInputStream(file).use { input -> + val buffer = ByteArray(8192) + var bytesRead: Int + while (input.read(buffer).also { bytesRead = it } != -1) { + digest.update(buffer, 0, bytesRead) + } + } + } + } + digest.digest().joinToString("") { "%02x".format(it) } + } catch (e: Exception) { + LOGGER.info("Could not compute source hash, will extract fresh: ${e.message}") + null + } + } + private fun extractZipToApp(appFileName: String) { val appZip = target.toFile().walk().firstOrNull { it.name == appFileName && it.extension == "zip" } ?: run { @@ -126,4 +181,4 @@ class IOSBuildProductsExtractor( FileSystems.getFileSystem(this) } } -} \ No newline at end of file +} diff --git a/maestro-ios-driver/src/main/kotlin/xcuitest/installer/LocalXCTestInstaller.kt b/maestro-ios-driver/src/main/kotlin/xcuitest/installer/LocalXCTestInstaller.kt index 8c0a3d0acb..c7430bbff9 100644 --- a/maestro-ios-driver/src/main/kotlin/xcuitest/installer/LocalXCTestInstaller.kt +++ b/maestro-ios-driver/src/main/kotlin/xcuitest/installer/LocalXCTestInstaller.kt @@ -24,16 +24,16 @@ import kotlin.time.Duration.Companion.seconds class LocalXCTestInstaller( private val deviceId: String, - private val host: String = "127.0.0.1", + private val host: String = "localhost", private val deviceType: IOSDeviceType, private val defaultPort: Int, private val metricsProvider: Metrics = MetricsProvider.getInstance(), private val httpClient: OkHttpClient = HttpClient.build( name = "XCUITestDriverStatusCheck", connectTimeout = 1.seconds, - readTimeout = 100.seconds, + readTimeout = 3.seconds, ), - val reinstallDriver: Boolean = true, + val reinstallDriver: Boolean = false, private val iOSDriverConfig: IOSDriverConfig, private val deviceController: IOSDevice, private val tempFileHandler: TempFileHandler = TempFileHandler() @@ -49,16 +49,17 @@ class LocalXCTestInstaller( * Make sure to launch the xctest runner from Xcode whenever maestro needs it. */ private val useXcodeTestRunner = !System.getenv("USE_XCODE_TEST_RUNNER").isNullOrEmpty() - private val tempDir = tempFileHandler.createTempDirectory(deviceId) + private val buildProductsDir = File(System.getProperty("user.home"), ".maestro/build-products/$deviceId").also { it.mkdirs() } private val localSimulatorUtils = LocalSimulatorUtils(tempFileHandler) private val iosBuildProductsExtractor = IOSBuildProductsExtractor( - target = tempDir.toPath(), + target = buildProductsDir.toPath(), context = iOSDriverConfig.context, deviceType = deviceType, ) private val xcRunnerCLIUtils = XCRunnerCLIUtils(tempFileHandler) private var xcTestProcess: Process? = null + private val runningHashFile = File(buildProductsDir, ".running-hash") override fun uninstall(): Boolean { return metrics.measured("operation", mapOf("command" to "uninstall")) { @@ -125,7 +126,13 @@ class LocalXCTestInstaller( while (System.currentTimeMillis() - startTime < getStartupTimeout()) { runCatching { - if (isChannelAlive()) return@measured XCTestClient(host, defaultPort) + if (isChannelAlive()) { + // Record which version is running so isVersionMatch() can check later + iosBuildProductsExtractor.sourceHash(iOSDriverConfig.sourceDirectory)?.let { + runningHashFile.writeText(it) + } + return@measured XCTestClient(host, defaultPort) + } } Thread.sleep(500) } @@ -146,6 +153,12 @@ class LocalXCTestInstaller( } } + override fun isVersionMatch(): Boolean { + val expectedHash = iosBuildProductsExtractor.sourceHash(iOSDriverConfig.sourceDirectory) ?: return false + val runningHash = if (runningHashFile.exists()) runningHashFile.readText() else return false + return expectedHash == runningHash + } + private fun ensureOpen(): Boolean { val timeout = 120_000L logger.info("ensureOpen(): Will spend $timeout ms waiting for the channel to become alive") @@ -161,7 +174,7 @@ class LocalXCTestInstaller( fun xctestAPIBuilder(pathSegment: String): HttpUrl.Builder { return HttpUrl.Builder() .scheme("http") - .host("127.0.0.1") + .host(host) .addPathSegment(pathSegment) .port(defaultPort) } diff --git a/maestro-ios-driver/src/main/kotlin/xcuitest/installer/XCTestInstaller.kt b/maestro-ios-driver/src/main/kotlin/xcuitest/installer/XCTestInstaller.kt index ec7029882d..2f30c2a9bb 100644 --- a/maestro-ios-driver/src/main/kotlin/xcuitest/installer/XCTestInstaller.kt +++ b/maestro-ios-driver/src/main/kotlin/xcuitest/installer/XCTestInstaller.kt @@ -13,4 +13,11 @@ interface XCTestInstaller: AutoCloseable { fun uninstall(): Boolean fun isChannelAlive(): Boolean + + /** + * Returns true if the currently running XCTest Runner matches the + * expected version (e.g., same build product hash). Returns false + * if the version cannot be determined or does not match. + */ + fun isVersionMatch(): Boolean = true } diff --git a/maestro-ios-driver/src/test/kotlin/LocalSimulatorUtilsTest.kt b/maestro-ios-driver/src/test/kotlin/LocalSimulatorUtilsTest.kt new file mode 100644 index 0000000000..f6a32ac4bb --- /dev/null +++ b/maestro-ios-driver/src/test/kotlin/LocalSimulatorUtilsTest.kt @@ -0,0 +1,103 @@ +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.io.File +import java.nio.file.Path + +class LocalSimulatorUtilsTest { + + @TempDir + lateinit var tempDir: Path + + private lateinit var cacheDir: File + + @BeforeEach + fun setUp() { + cacheDir = tempDir.resolve("app-cache").resolve("test-device-id").toFile() + cacheDir.mkdirs() + } + + // --- Task #13: App binary cache staleness --- + + @Test + fun `cached binary should be updated when installed app has different content`() { + val bundleId = "com.example.app" + + // Simulate cached app (v1) + val cachedAppDir = File(cacheDir, "$bundleId.app") + cachedAppDir.mkdirs() + File(cachedAppDir, "Info.plist").writeText("v1-plist") + File(cachedAppDir, "AppBinary").writeText("v1-binary") + + // Simulate installed app updated to v2 (different binary) + val installedAppDir = tempDir.resolve("installed").resolve("$bundleId.app").toFile() + installedAppDir.mkdirs() + File(installedAppDir, "Info.plist").writeText("v2-plist") + File(installedAppDir, "AppBinary").writeText("v2-binary-updated") + + // getCachedAppBinary should detect the mismatch and re-cache. + // For now, simulate what the fix should do: + // compare installed vs cached, update cache if different. + val isCacheStale = isBinaryCacheStale(cachedAppDir, installedAppDir) + + // DESIRED: cache should be detected as stale + assertThat(isCacheStale).isTrue() + } + + @Test + fun `cached binary should be reused when installed app matches`() { + val bundleId = "com.example.app" + + // Same content in cache and installed + val cachedAppDir = File(cacheDir, "$bundleId.app") + cachedAppDir.mkdirs() + File(cachedAppDir, "AppBinary").writeText("same-binary") + + val installedAppDir = tempDir.resolve("installed").resolve("$bundleId.app").toFile() + installedAppDir.mkdirs() + File(installedAppDir, "AppBinary").writeText("same-binary") + + val isCacheStale = isBinaryCacheStale(cachedAppDir, installedAppDir) + + // Cache matches — not stale + assertThat(isCacheStale).isFalse() + } + + @Test + fun `per-device cache dirs should be isolated`() { + val bundleId = "com.example.app" + + val deviceACache = tempDir.resolve("app-cache/device-A/$bundleId.app").toFile() + deviceACache.mkdirs() + File(deviceACache, "data").writeText("device-A-data") + + val deviceBCache = tempDir.resolve("app-cache/device-B/$bundleId.app").toFile() + deviceBCache.mkdirs() + File(deviceBCache, "data").writeText("device-B-data") + + // Modifying device A should not affect device B + File(deviceACache, "data").writeText("device-A-modified") + assertThat(File(deviceBCache, "data").readText()).isEqualTo("device-B-data") + } + + // --- Helper: this is the logic getCachedAppBinary SHOULD implement --- + + private fun isBinaryCacheStale(cachedDir: File, installedDir: File): Boolean { + if (!cachedDir.exists()) return true + if (!installedDir.exists()) return false + + // Compare file contents to detect staleness + val cachedFiles = cachedDir.walkTopDown().filter { it.isFile }.sortedBy { it.name } + val installedFiles = installedDir.walkTopDown().filter { it.isFile }.sortedBy { it.name } + + val cachedList = cachedFiles.toList() + val installedList = installedFiles.toList() + + if (cachedList.size != installedList.size) return true + + return cachedList.zip(installedList).any { (cached, installed) -> + cached.name != installed.name || cached.readBytes().contentEquals(installed.readBytes()).not() + } + } +} diff --git a/maestro-ios-driver/src/test/kotlin/XCTestDriverClientTest.kt b/maestro-ios-driver/src/test/kotlin/XCTestDriverClientTest.kt new file mode 100644 index 0000000000..e5011f1dc2 --- /dev/null +++ b/maestro-ios-driver/src/test/kotlin/XCTestDriverClientTest.kt @@ -0,0 +1,92 @@ +import com.google.common.truth.Truth.assertThat +import org.junit.jupiter.api.Test +import xcuitest.XCTestDriverClient +import xcuitest.installer.XCTestInstaller +import xcuitest.XCTestClient + +class XCTestDriverClientTest { + + class FakeXCTestInstaller( + var channelAlive: Boolean = false, + var versionMatch: Boolean = true, + ) : XCTestInstaller { + + var startCount = 0 + private set + var uninstallCount = 0 + private set + + override fun start(): XCTestClient { + startCount++ + return XCTestClient("localhost", 22087) + } + + override fun uninstall(): Boolean { + uninstallCount++ + return true + } + + override fun isChannelAlive(): Boolean = channelAlive + override fun isVersionMatch(): Boolean = versionMatch + override fun close() {} + } + + @Test + fun `restartXCTestRunner should reinstall when channel is alive but version mismatches`() { + val installer = FakeXCTestInstaller(channelAlive = true, versionMatch = false) + val driverClient = XCTestDriverClient( + installer = installer, + client = XCTestClient("localhost", 22087), + reinstallDriver = false + ) + + driverClient.restartXCTestRunner() + + assertThat(installer.startCount).isEqualTo(1) + } + + @Test + fun `restartXCTestRunner should reuse session when channel is alive and version matches`() { + val installer = FakeXCTestInstaller(channelAlive = true, versionMatch = true) + val driverClient = XCTestDriverClient( + installer = installer, + client = XCTestClient("localhost", 22087), + reinstallDriver = false + ) + + driverClient.restartXCTestRunner() + + assertThat(installer.startCount).isEqualTo(0) + assertThat(installer.uninstallCount).isEqualTo(0) + } + + @Test + fun `restartXCTestRunner should honor explicit reinstall even when channel is alive and version matches`() { + val installer = FakeXCTestInstaller(channelAlive = true, versionMatch = true) + val driverClient = XCTestDriverClient( + installer = installer, + client = XCTestClient("localhost", 22087), + reinstallDriver = true + ) + + driverClient.restartXCTestRunner() + + assertThat(installer.uninstallCount).isEqualTo(1) + assertThat(installer.startCount).isEqualTo(1) + } + + @Test + fun `restartXCTestRunner should start fresh when channel is not alive`() { + val installer = FakeXCTestInstaller(channelAlive = false) + val driverClient = XCTestDriverClient( + installer = installer, + client = XCTestClient("localhost", 22087), + reinstallDriver = false + ) + + driverClient.restartXCTestRunner() + + assertThat(installer.uninstallCount).isEqualTo(0) + assertThat(installer.startCount).isEqualTo(1) + } +}