Merge pull request #802 from MarathonLabs/fix/md5-detection

fix(android): findBinary should make decision on exit code, not output

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" }
     }

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>): String?
-    suspend fun installSplitPackages(absolutePaths: List<String>, reinstall: Boolean, optionalParams: List<String>): 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<String>): ShellCommandResult?
+    suspend fun installSplitPackages(absolutePaths: List<String>, reinstall: Boolean, optionalParams: List<String>): String
+    suspend fun safeUninstallPackage(appPackage: String, keepData: Boolean = false): ShellCommandResult?
+    suspend fun safeClearPackage(packageName: String): ShellCommandResult?
 
     suspend fun safeStartScreenRecorder(remoteFilePath: String, options: VideoConfiguration)
 }

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

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,29 +120,29 @@ class AdamAndroidDevice(
 
     private var props: Map<String, String> = 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)
             null
         }
     }
 
-    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>): String? {
+    override suspend fun installPackage(absolutePath: String, reinstall: Boolean, optionalParams: List<String>): 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<String>, reinstall: Boolean, optionalParams: List<String>): String? {
+    override suspend fun installSplitPackages(absolutePaths: List<String>, reinstall: Boolean, optionalParams: List<String>): String {
         return withTimeoutOrNull(androidConfiguration.timeoutConfiguration.install) {
             val files = absolutePaths.map { File(it) }
             client.execute(

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()}" }
                     }
                 }
             }

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)
+}

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()) {

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
 }

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)