From 5705c948cfbe485d4db1cafad7807a2fa737f377 Mon Sep 17 00:00:00 2001 From: Anton Malinskiy Date: Thu, 1 Jun 2023 15:22:13 +1000 Subject: [PATCH] fix(android): findBinary should make decision on exit code, not output note: shame Android doesn't include `which` --- .../marathon/android/AndroidAppInstaller.kt | 6 ++--- .../marathon/android/AndroidDevice.kt | 15 ++++++------ .../marathon/android/BaseAndroidDevice.kt | 13 +++++----- .../android/adam/AdamAndroidDevice.kt | 24 ++++++++++--------- .../android/adam/AndroidDeviceTestRunner.kt | 8 +++---- .../adam/extension/ShellCommandResult.kt | 7 ++++++ .../listeners/video/ScreenRecorderStopper.kt | 4 ++-- .../video/ScreenRecorderTestBatchListener.kt | 2 +- .../android/model/ShellCommandResult.kt | 3 +++ 9 files changed, 47 insertions(+), 35 deletions(-) create mode 100644 vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/extension/ShellCommandResult.kt create mode 100644 vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/model/ShellCommandResult.kt diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/AndroidAppInstaller.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/AndroidAppInstaller.kt index bbc00a36f..96387d3d9 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/AndroidAppInstaller.kt +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/AndroidAppInstaller.kt @@ -67,13 +67,13 @@ class AndroidAppInstaller(configuration: Configuration) { try { if (installed(device, appPackage)) { logger.info("Uninstalling $appPackage from ${device.serialNumber}") - val uninstallMessage = device.safeUninstallPackage(appPackage)?.trim() + val uninstallMessage = device.safeUninstallPackage(appPackage)?.output?.trim() uninstallMessage?.let { logger.debug { it } } } logger.info("Installing $appPackage, ${absolutePaths.joinToString()} to ${device.serialNumber}") val installMessage = when { - splitApks.isEmpty() -> device.installPackage(applicationApk.absolutePath, true, optionalParams(device))?.trim() + splitApks.isEmpty() -> device.installPackage(applicationApk.absolutePath, true, optionalParams(device))?.output?.trim() else -> device.installSplitPackages(absolutePaths, true, optionalParams(device)) } installMessage?.let { logger.debug { it } } @@ -91,7 +91,7 @@ class AndroidAppInstaller(configuration: Configuration) { * @throws DeviceSetupException if unable to verify */ private suspend fun installed(device: AndroidDevice, appPackage: String): Boolean { - val lines = device.safeExecuteShellCommand("pm list packages")?.lines() + val lines = device.safeExecuteShellCommand("pm list packages")?.output?.lines() ?: throw DeviceSetupException("Unable to verify that package $appPackage is installed") return lines.any { it == "package:$appPackage" } } diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/AndroidDevice.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/AndroidDevice.kt index f091b67a7..c1aee22af 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/AndroidDevice.kt +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/AndroidDevice.kt @@ -1,6 +1,7 @@ package com.malinskiy.marathon.android import com.android.sdklib.AndroidVersion +import com.malinskiy.marathon.android.model.ShellCommandResult import com.malinskiy.marathon.device.screenshot.Rotation import com.malinskiy.marathon.config.vendor.android.VideoConfiguration import com.malinskiy.marathon.device.Device @@ -24,17 +25,17 @@ interface AndroidDevice : Device, LogProducer, Screenshottable { /** * @return null if command did not complete successfully, otherwise cmd output */ - suspend fun executeShellCommand(command: String, errorMessage: String = ""): String? + suspend fun executeShellCommand(command: String, errorMessage: String = ""): ShellCommandResult? /** * @return null if command did not complete successfully, otherwise cmd output */ - suspend fun safeExecuteShellCommand(command: String, errorMessage: String = ""): String? + suspend fun safeExecuteShellCommand(command: String, errorMessage: String = ""): ShellCommandResult? /** * @throws com.malinskiy.marathon.android.exception.CommandRejectedException in case the command fails */ - suspend fun criticalExecuteShellCommand(command: String, errorMessage: String = ""): String + suspend fun criticalExecuteShellCommand(command: String, errorMessage: String = ""): ShellCommandResult /** * @throws com.malinskiy.marathon.android.exception.TransferException @@ -60,10 +61,10 @@ interface AndroidDevice : Device, LogProducer, Screenshottable { /** * @throws com.malinskiy.marathon.android.exception.InstallException in case of failure to push the apk */ - suspend fun installPackage(absolutePath: String, reinstall: Boolean, optionalParams: List): String? - suspend fun installSplitPackages(absolutePaths: List, reinstall: Boolean, optionalParams: List): String? - suspend fun safeUninstallPackage(appPackage: String, keepData: Boolean = false): String? - suspend fun safeClearPackage(packageName: String): String? + suspend fun installPackage(absolutePath: String, reinstall: Boolean, optionalParams: List): ShellCommandResult? + suspend fun installSplitPackages(absolutePaths: List, reinstall: Boolean, optionalParams: List): String + suspend fun safeUninstallPackage(appPackage: String, keepData: Boolean = false): ShellCommandResult? + suspend fun safeClearPackage(packageName: String): ShellCommandResult? suspend fun safeStartScreenRecorder(remoteFilePath: String, options: VideoConfiguration) } diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/BaseAndroidDevice.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/BaseAndroidDevice.kt index dfddd79b6..397345cc0 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/BaseAndroidDevice.kt +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/BaseAndroidDevice.kt @@ -13,6 +13,7 @@ import com.malinskiy.marathon.android.executor.listeners.filesync.FileSyncTestRu import com.malinskiy.marathon.android.executor.listeners.screenshot.AdamScreenCaptureTestRunListener import com.malinskiy.marathon.android.executor.listeners.screenshot.ScreenCapturerTestRunListener import com.malinskiy.marathon.android.executor.listeners.video.ScreenRecorderTestBatchListener +import com.malinskiy.marathon.android.model.ShellCommandResult import com.malinskiy.marathon.device.screenshot.Rotation import com.malinskiy.marathon.config.Configuration import com.malinskiy.marathon.config.ScreenRecordingPolicy @@ -96,7 +97,7 @@ abstract class BaseAndroidDevice( manufacturer = getProperty("ro.product.manufacturer") ?: "Unknown" initialRotation = fetchRotation() - externalStorageMount = safeExecuteShellCommand("echo \$EXTERNAL_STORAGE")?.trim { + externalStorageMount = safeExecuteShellCommand("echo \$EXTERNAL_STORAGE")?.output?.trim { when (it) { '\n' -> true '\r' -> true @@ -188,7 +189,7 @@ abstract class BaseAndroidDevice( } - override suspend fun safeClearPackage(packageName: String): String? = + override suspend fun safeClearPackage(packageName: String): ShellCommandResult? = safeExecuteShellCommand("pm clear $packageName", "Could not clear package $packageName on device: $serialNumber") protected suspend fun clearLogcat() = safeExecuteShellCommand("logcat -c", "Could not clear logcat on device: $serialNumber") @@ -200,7 +201,7 @@ abstract class BaseAndroidDevice( if (md5.isNotEmpty() && md5cmd.isNotEmpty()) { val syncTimeMillis = measureTimeMillis { do { - val remoteMd5 = safeExecuteShellCommand("$md5cmd $remotePath") ?: "" + val remoteMd5 = safeExecuteShellCommand("$md5cmd $remotePath")?.output ?: "" delay(10) } while (!remoteMd5.contains(md5)) } @@ -211,9 +212,7 @@ abstract class BaseAndroidDevice( } private suspend fun hasBinary(path: String): Boolean { - val output = safeExecuteShellCommand("ls $path") - val value: String = output?.trim { it <= ' ' } ?: return false - return !value.endsWith("No such file or directory") + return safeExecuteShellCommand("ls $path")?.exitCode == 0 } private suspend fun waitForBoot(): Boolean { @@ -328,7 +327,7 @@ abstract class BaseAndroidDevice( } private suspend fun fetchRotation() = - safeExecuteShellCommand("dumpsys input")?.let { dumpsysOutput -> + safeExecuteShellCommand("dumpsys input")?.output?.let { dumpsysOutput -> val start = dumpsysOutput.indexOf("SurfaceOrientation") if (start == -1) { return@let null diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AdamAndroidDevice.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AdamAndroidDevice.kt index 7bdb184bb..baa4d6568 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AdamAndroidDevice.kt +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AdamAndroidDevice.kt @@ -38,6 +38,7 @@ import com.malinskiy.marathon.android.AndroidAppInstaller import com.malinskiy.marathon.android.AndroidTestBundleIdentifier import com.malinskiy.marathon.android.BaseAndroidDevice import com.malinskiy.marathon.android.RemoteFileManager +import com.malinskiy.marathon.android.adam.extension.toShellResult import com.malinskiy.marathon.android.exception.CommandRejectedException import com.malinskiy.marathon.android.exception.InstallException import com.malinskiy.marathon.exceptions.TransferException @@ -70,6 +71,7 @@ import java.awt.image.BufferedImage import java.io.File import java.time.Duration import kotlin.coroutines.CoroutineContext +import com.malinskiy.marathon.android.model.ShellCommandResult as MarathonShellCommandResult class AdamAndroidDevice( internal val client: AndroidDebugBridgeClient, @@ -118,19 +120,19 @@ class AdamAndroidDevice( private var props: Map = emptyMap() - override suspend fun executeShellCommand(command: String, errorMessage: String): String? { + override suspend fun executeShellCommand(command: String, errorMessage: String): MarathonShellCommandResult? { return try { - return client.execute(ShellCommandRequest(command), serial = adbSerial).output + return client.execute(ShellCommandRequest(command), serial = adbSerial).toShellResult() } catch (e: Exception) { logger.error(errorMessage, e) null } } - override suspend fun safeExecuteShellCommand(command: String, errorMessage: String): String? { + override suspend fun safeExecuteShellCommand(command: String, errorMessage: String): MarathonShellCommandResult? { return try { withTimeoutOrNull(androidConfiguration.timeoutConfiguration.shell) { - client.execute(ShellCommandRequest(command), serial = adbSerial).output + client.execute(ShellCommandRequest(command), serial = adbSerial).toShellResult() } } catch (e: Exception) { logger.error(errorMessage, e) @@ -138,9 +140,9 @@ class AdamAndroidDevice( } } - override suspend fun criticalExecuteShellCommand(command: String, errorMessage: String): String { + override suspend fun criticalExecuteShellCommand(command: String, errorMessage: String): MarathonShellCommandResult { return withTimeoutOrNull(androidConfiguration.timeoutConfiguration.shell) { - client.execute(ShellCommandRequest(command), serial = adbSerial).output + client.execute(ShellCommandRequest(command), serial = adbSerial).toShellResult() } ?: throw CommandRejectedException(errorMessage) } @@ -271,13 +273,13 @@ class AdamAndroidDevice( } ?: logger.warn { "Pushing $localFolderPath timed out. Ignoring" } } - override suspend fun safeUninstallPackage(appPackage: String, keepData: Boolean): String? { + override suspend fun safeUninstallPackage(appPackage: String, keepData: Boolean): MarathonShellCommandResult? { return withTimeoutOrNull(androidConfiguration.timeoutConfiguration.uninstall) { - client.execute(UninstallRemotePackageRequest(appPackage, keepData = keepData), serial = adbSerial).output + client.execute(UninstallRemotePackageRequest(appPackage, keepData = keepData), serial = adbSerial).toShellResult() } } - override suspend fun installPackage(absolutePath: String, reinstall: Boolean, optionalParams: List): String? { + override suspend fun installPackage(absolutePath: String, reinstall: Boolean, optionalParams: List): MarathonShellCommandResult { val file = File(absolutePath) //Very simple escaping for the name of the file val fileName = file.name.escape() @@ -302,10 +304,10 @@ class AdamAndroidDevice( } ?: throw InstallException("Timeout transferring $absolutePath") safeExecuteShellCommand("rm $remotePath") - return result.output.trim() + return com.malinskiy.marathon.android.model.ShellCommandResult(result.output.trim(), result.exitCode) } - override suspend fun installSplitPackages(absolutePaths: List, reinstall: Boolean, optionalParams: List): String? { + override suspend fun installSplitPackages(absolutePaths: List, reinstall: Boolean, optionalParams: List): String { return withTimeoutOrNull(androidConfiguration.timeoutConfiguration.install) { val files = absolutePaths.map { File(it) } client.execute( diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt index 1a9074480..f2c83e1d5 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/AndroidDeviceTestRunner.kt @@ -135,12 +135,12 @@ class AndroidDeviceTestRunner(private val device: AdamAndroidDevice, private val info: InstrumentationInfo ) { if (androidConfiguration.applicationPmClear) { - device.safeClearPackage(info.applicationPackage)?.trim()?.also { + device.safeClearPackage(info.applicationPackage)?.output?.trim()?.also { logger.debug { "Package ${info.applicationPackage} cleared: $it" } } } if (androidConfiguration.testApplicationPmClear) { - device.safeClearPackage(info.instrumentationPackage)?.trim()?.also { + device.safeClearPackage(info.instrumentationPackage)?.output?.trim()?.also { logger.debug { "Package ${info.instrumentationPackage} cleared: $it" } } } @@ -149,14 +149,14 @@ class AndroidDeviceTestRunner(private val device: AdamAndroidDevice, private val device.version.isGreaterOrEqualThan(30) -> { val command = "appops set --uid ${info.applicationPackage} MANAGE_EXTERNAL_STORAGE allow" device.criticalExecuteShellCommand(command).also { - logger.debug { "File pull requested. Granted MANAGE_EXTERNAL_STORAGE to ${info.applicationPackage}: ${it.trim()}" } + logger.debug { "File pull requested. Granted MANAGE_EXTERNAL_STORAGE to ${info.applicationPackage}: ${it.output.trim()}" } } } device.version.equals(29) -> { //API 29 doesn't have MANAGE_EXTERNAL_STORAGE, force legacy storage val command = "appops set --uid ${info.applicationPackage} LEGACY_STORAGE allow" device.criticalExecuteShellCommand(command).also { - logger.debug { "File pull requested. Granted LEGACY_STORAGE to ${info.applicationPackage}: ${it.trim()}" } + logger.debug { "File pull requested. Granted LEGACY_STORAGE to ${info.applicationPackage}: ${it.output.trim()}" } } } } diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/extension/ShellCommandResult.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/extension/ShellCommandResult.kt new file mode 100644 index 000000000..b9ab55417 --- /dev/null +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/adam/extension/ShellCommandResult.kt @@ -0,0 +1,7 @@ +package com.malinskiy.marathon.android.adam.extension + +import com.malinskiy.adam.request.shell.v1.ShellCommandResult + +fun ShellCommandResult.toShellResult(): com.malinskiy.marathon.android.model.ShellCommandResult { + return com.malinskiy.marathon.android.model.ShellCommandResult(output, exitCode) +} diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/video/ScreenRecorderStopper.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/video/ScreenRecorderStopper.kt index fd3d59304..187453209 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/video/ScreenRecorderStopper.kt +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/video/ScreenRecorderStopper.kt @@ -15,9 +15,9 @@ internal class ScreenRecorderStopper(private val device: AndroidDevice) { private suspend fun grepPid(): String { val output = if (device.version.isGreaterOrEqualThan(26)) { - device.safeExecuteShellCommand("ps -A | grep screenrecord") ?: "" + device.safeExecuteShellCommand("ps -A | grep screenrecord")?.output ?: "" } else { - device.safeExecuteShellCommand("ps | grep screenrecord") ?: "" + device.safeExecuteShellCommand("ps | grep screenrecord")?.output ?: "" } if (output.isBlank()) { diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/video/ScreenRecorderTestBatchListener.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/video/ScreenRecorderTestBatchListener.kt index a149f5a41..c7c78aa8b 100644 --- a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/video/ScreenRecorderTestBatchListener.kt +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/executor/listeners/video/ScreenRecorderTestBatchListener.kt @@ -152,6 +152,6 @@ class ScreenRecorderTestBatchListener( private suspend fun AndroidDevice.verifyHealthy(): Boolean { return withTimeoutOrNull(Duration.ofSeconds(10)) { - executeShellCommand("echo quickhealthcheck")?.let { it.contains("quickhealthcheck") } ?: false + executeShellCommand("echo quickhealthcheck")?.output?.let { it.contains("quickhealthcheck") } ?: false } ?: false } diff --git a/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/model/ShellCommandResult.kt b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/model/ShellCommandResult.kt new file mode 100644 index 000000000..6667ec95b --- /dev/null +++ b/vendor/vendor-android/src/main/kotlin/com/malinskiy/marathon/android/model/ShellCommandResult.kt @@ -0,0 +1,3 @@ +package com.malinskiy.marathon.android.model + +data class ShellCommandResult(val output: String, val exitCode: Int)