Clean-up waiting for a device logic

Author: Sergey Chelombitko

core/src/main/kotlin/com/malinskiy/marathon/device/DeviceProvider.kt

@@ -3,9 +3,8 @@ package com.malinskiy.marathon.device
 import kotlinx.coroutines.flow.Flow
 
 interface DeviceProvider : AutoCloseable {
-    val deviceInitializationTimeoutMillis: Long
+    val deviceEvents: Flow<DeviceEvent>
+
     suspend fun initialize()
     suspend fun terminate()
-
-    val deviceEvents: Flow<DeviceEvent>
 }

core/src/main/kotlin/com/malinskiy/marathon/exceptions/NoDevicesException.kt

@@ -1,3 +1,3 @@
 package com.malinskiy.marathon.exceptions
 
-class NoDevicesException(message: String = "No devices found") : RuntimeException(message)
+class NoDevicesException(cause: Throwable) : RuntimeException("No devices found", cause)

core/src/main/kotlin/com/malinskiy/marathon/execution/Scheduler.kt

@@ -24,7 +24,6 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.DelicateCoroutinesApi
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.TimeoutCancellationException
-import kotlinx.coroutines.cancelAndJoin
 import kotlinx.coroutines.channels.SendChannel
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.filter
@@ -66,16 +65,14 @@ class Scheduler(
         initializeCache(scope)
 
         try {
-            withTimeout(deviceProvider.deviceInitializationTimeoutMillis) {
+            withTimeout(configuration.noDevicesTimeoutMillis) {
                 while (pools.isEmpty()) {
-                    delay(100)
+                    logger.debug("Waiting for a device...")
+                    delay(500L)
                 }
             }
         } catch (e: TimeoutCancellationException) {
-            logger.warn("Timeout waiting for non-empty pools", e)
-
-            job.cancelAndJoin()
-            throw NoDevicesException()
+            throw NoDevicesException(e)
         }
     }
 

vendor/vendor-android/ddmlib/src/main/kotlin/com/malinskiy/marathon/android/ddmlib/DdmlibDeviceProvider.kt

@@ -3,7 +3,6 @@ package com.malinskiy.marathon.android.ddmlib
 import com.android.ddmlib.AndroidDebugBridge
 import com.android.ddmlib.DdmPreferences
 import com.android.ddmlib.IDevice
-import com.android.ddmlib.TimeoutException
 import com.malinskiy.marathon.actor.unboundedChannel
 import com.malinskiy.marathon.analytics.internal.pub.Track
 import com.malinskiy.marathon.android.AndroidAppInstaller
@@ -24,12 +23,15 @@ import kotlinx.coroutines.CompletableJob
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.TimeoutCancellationException
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.channels.Channel
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.consumeAsFlow
+import kotlinx.coroutines.isActive
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.time.withTimeout
 import java.time.Duration
 import java.util.concurrent.ConcurrentHashMap
 import java.util.concurrent.ConcurrentMap
@@ -54,8 +56,6 @@ class DdmlibDeviceProvider(
     private val job = SupervisorJob()
     private val coroutineScope = CoroutineScope(job + dispatcher)
 
-    override val deviceInitializationTimeoutMillis: Long = 180_000
-
     override val deviceEvents: Flow<DeviceEvent>
         get() = channel.consumeAsFlow()
 
@@ -74,31 +74,14 @@ class DdmlibDeviceProvider(
             logger.debug("Reusing existing ADB bridge")
         }
 
-        var getDevicesCountdown = config.noDevicesTimeoutMillis
-        val sleepTime = DEFAULT_DDM_LIB_SLEEP_TIME
-        while (!adb.hasInitialDeviceList() || !adb.hasDevices() && getDevicesCountdown >= 0) {
-            logger.debug("No devices, waiting...")
-
-            try {
-                delay(sleepTime)
-            } catch (e: InterruptedException) {
-                throw TimeoutException("Timeout getting device list", e)
-            }
-            getDevicesCountdown -= sleepTime
-        }
-
-        logger.debug("Finished waiting for a device")
+        adb.ensureInitialized()
 
         if (!newAdbCreated && adb.devices.isNotEmpty()) {
             logger.debug("Initial connected devices: {}", adb.devices.joinToString(", "))
             adb.devices.forEach {
                 deviceConnected(it)
             }
         }
-
-        if (!adb.hasInitialDeviceList() || !adb.hasDevices()) {
-            throw NoDevicesException()
-        }
     }
 
     private fun getDeviceOrPut(androidDevice: DdmlibAndroidDevice): DdmlibAndroidDevice {
@@ -122,8 +105,6 @@ class DdmlibDeviceProvider(
         }
     }
 
-    private fun AndroidDebugBridge.hasDevices(): Boolean = devices.isNotEmpty()
-
     override suspend fun terminate() {
         job.completeRecursively()
         job.join()
@@ -216,6 +197,19 @@ class DdmlibDeviceProvider(
             parentJob = job
         )
 
+    private suspend fun AndroidDebugBridge.ensureInitialized() {
+        try {
+            withTimeout(ADB_INIT_TIMEOUT) {
+                while (isActive && !hasInitialDeviceList()) {
+                    logger.debug("Waiting for ADB initialization...")
+                    delay(500L)
+                }
+            }
+        } catch (e: TimeoutCancellationException) {
+            throw NoDevicesException(e)
+        }
+    }
+
     private val vendorConfiguration: AndroidConfiguration
         get() = config.vendorConfiguration as AndroidConfiguration
 
@@ -229,6 +223,5 @@ class DdmlibDeviceProvider(
     companion object {
         private val ADB_INIT_TIMEOUT = Duration.ofSeconds(60)
         private const val DEFAULT_DDM_LIB_TIMEOUT = 30000
-        private const val DEFAULT_DDM_LIB_SLEEP_TIME = 500L
     }
 }

vendor/vendor-test/src/main/kotlin/com/malinskiy/marathon/test/StubDeviceProvider.kt

@@ -15,9 +15,6 @@ class StubDeviceProvider : DeviceProvider {
     private val channel: Channel<DeviceEvent> = unboundedChannel()
     var providingLogic: (suspend (Channel<DeviceEvent>) -> Unit)? = null
 
-    override val deviceInitializationTimeoutMillis: Long = 180_000
-    override suspend fun initialize() = Unit
-
     override val deviceEvents: Flow<DeviceEvent>
         get() {
             providingLogic?.let {
@@ -29,6 +26,8 @@ class StubDeviceProvider : DeviceProvider {
             return channel.consumeAsFlow()
         }
 
+    override suspend fun initialize() = Unit
+
     override suspend fun terminate() {
         channel.close()
     }