fix: use noop insights by default on maestro (#2131)

mobile-dev-inc · Nov 14, 2024 · 17e67dc · 17e67dc
1 parent 759d954
commit 17e67dc
Show file tree

Hide file tree

Showing 10 changed files with 84 additions and 43 deletions.
diff --git a/maestro-cli/src/main/java/maestro/cli/command/PrintHierarchyCommand.kt b/maestro-cli/src/main/java/maestro/cli/command/PrintHierarchyCommand.kt
@@ -26,9 +26,9 @@ import maestro.cli.DisableAnsiMixin
 import maestro.cli.ShowHelpMixin
 import maestro.cli.report.TestDebugReporter
 import maestro.cli.session.MaestroSessionManager
-import maestro.cli.view.green
 import maestro.cli.view.yellow
-import maestro.utils.Insights
+import maestro.utils.CliInsights
+import maestro.utils.Insight
 import maestro.utils.chunkStringByWordCount
 import picocli.CommandLine
 import java.lang.StringBuilder
@@ -65,7 +65,7 @@ class PrintHierarchyCommand : Runnable {
             deviceId = parent?.deviceId,
             platform = parent?.platform,
         ) { session ->
-            Insights.onInsightsUpdated {
+            val callback: (Insight) -> Unit = {
                 val message = StringBuilder()
                 val level = it.level.toString().lowercase().replaceFirstChar(Char::uppercase)
                 message.append(level.yellow() + ": ")
@@ -74,11 +74,17 @@ class PrintHierarchyCommand : Runnable {
                 }
                 println(message.toString())
             }
+            val insights = CliInsights
+
+            insights.onInsightsUpdated(callback)
+
             val hierarchy = jacksonObjectMapper()
                 .setSerializationInclusion(JsonInclude.Include.NON_NULL)
                 .writerWithDefaultPrettyPrinter()
                 .writeValueAsString(session.maestro.viewHierarchy().root)
 
+            insights.unregisterListener(callback)
+
             println(hierarchy)
         }
     }

diff --git a/maestro-cli/src/main/java/maestro/cli/runner/MaestroCommandRunner.kt b/maestro-cli/src/main/java/maestro/cli/runner/MaestroCommandRunner.kt
@@ -33,10 +33,11 @@ import maestro.orchestra.CompositeCommand
 import maestro.orchestra.MaestroCommand
 import maestro.orchestra.Orchestra
 import maestro.orchestra.yaml.YamlCommandReader
-import maestro.utils.Insight
+import maestro.utils.CliInsights
 import org.slf4j.LoggerFactory
 import java.util.IdentityHashMap
 import maestro.cli.util.ScreenshotUtils
+import maestro.utils.Insight
 
 /**
  * Knows how to run a list of Maestro commands and update the UI.
@@ -91,6 +92,7 @@ object MaestroCommandRunner {
 
         val orchestra = Orchestra(
             maestro = maestro,
+            insights = CliInsights,
             onCommandStart = { _, command ->
                 logger.info("${command.description()} RUNNING")
                 commandStatuses[command] = CommandStatus.RUNNING

diff --git a/maestro-cli/src/main/java/maestro/cli/session/MaestroSessionManager.kt b/maestro-cli/src/main/java/maestro/cli/session/MaestroSessionManager.kt
@@ -28,6 +28,7 @@ import maestro.Maestro
 import maestro.cli.device.Device
 import maestro.cli.device.PickDeviceInteractor
 import maestro.cli.device.Platform
+import maestro.utils.CliInsights
 import maestro.cli.util.ScreenReporter
 import maestro.drivers.AndroidDriver
 import maestro.drivers.IOSDriver
@@ -304,7 +305,9 @@ object MaestroSessionManager {
                 deviceId = deviceId,
                 xcTestDevice = xcTestDevice,
                 simctlIOSDevice = simctlIOSDevice,
-            )
+                insights = CliInsights
+            ),
+            insights = CliInsights
         )
 
         return Maestro.ios(

diff --git a/maestro-client/src/main/java/maestro/drivers/IOSDriver.kt b/maestro-client/src/main/java/maestro/drivers/IOSDriver.kt
@@ -40,6 +40,7 @@ import kotlin.collections.set
 
 class IOSDriver(
     private val iosDevice: IOSDevice,
+    private val insights: Insights = NoopInsights
 ) : Driver {
 
     private var appId: String? = null
@@ -138,9 +139,9 @@ class IOSDriver(
                     "If you are using React native, consider migrating to the new " +
                     "architecture where view flattening is available. For more information on the " +
                     "migration process, please visit: https://reactnative.dev/docs/new-architecture-intro"
-            Insights.report(Insight(message, Insight.Level.INFO))
+            insights.report(Insight(message, Insight.Level.INFO))
         } else {
-            Insights.report(Insight("", Insight.Level.NONE))
+            insights.report(Insight("", Insight.Level.NONE))
         }
         val hierarchy = hierarchyResult.axElement
         return mapViewHierarchy(hierarchy)

diff --git a/maestro-ios/src/main/java/ios/LocalIOSDevice.kt b/maestro-ios/src/main/java/ios/LocalIOSDevice.kt
@@ -10,13 +10,15 @@ import java.util.UUID
 import hierarchy.ViewHierarchy
 import maestro.utils.Insight
 import maestro.utils.Insights
+import maestro.utils.NoopInsights
 import java.util.concurrent.Executors
 import java.util.concurrent.TimeUnit
 
 class LocalIOSDevice(
     override val deviceId: String?,
     private val xcTestDevice: XCTestIOSDevice,
     private val simctlIOSDevice: SimctlIOSDevice,
+    private val insights: Insights = NoopInsights
 ) : IOSDevice {
 
     private val executor by lazy { Executors.newSingleThreadScheduledExecutor() }
@@ -34,7 +36,7 @@ class LocalIOSDevice(
         val future = executor.schedule(
             {
                 if (isViewHierarchyInProgress) {
-                    Insights.report(
+                    insights.report(
                         Insight(
                             message = "Retrieving the hierarchy is taking longer than usual. This might be due to a " +
                                     "deep hierarchy in the current view. Please wait a bit more to complete the operation.",

diff --git a/maestro-orchestra/src/main/java/maestro/orchestra/Orchestra.kt b/maestro-orchestra/src/main/java/maestro/orchestra/Orchestra.kt
@@ -40,6 +40,7 @@ import maestro.orchestra.yaml.YamlCommandReader
 import maestro.utils.Insight
 import maestro.utils.Insights
 import maestro.utils.MaestroTimer
+import maestro.utils.NoopInsights
 import maestro.utils.StringUtils.toRegexSafe
 import okhttp3.OkHttpClient
 import okio.Buffer
@@ -76,11 +77,12 @@ class Orchestra(
     private val lookupTimeoutMs: Long = 17000L,
     private val optionalLookupTimeoutMs: Long = 7000L,
     private val httpClient: OkHttpClient? = null,
+    private val insights: Insights = NoopInsights,
     private val onFlowStart: (List<MaestroCommand>) -> Unit = {},
     private val onCommandStart: (Int, MaestroCommand) -> Unit = { _, _ -> },
     private val onCommandComplete: (Int, MaestroCommand) -> Unit = { _, _ -> },
     private val onCommandFailed: (Int, MaestroCommand, Throwable) -> ErrorResolution = { _, _, e -> throw e },
-    private val onCommandWarned: (Int, MaestroCommand) -> Unit = { _, _ -> },
+    private val onCommandWarned: (Int, MaestroCommand) -> Unit = { _,  _ -> },
     private val onCommandSkipped: (Int, MaestroCommand) -> Unit = { _, _ -> },
     private val onCommandReset: (MaestroCommand) -> Unit = {},
     private val onCommandMetadataUpdate: (MaestroCommand, CommandMetadata) -> Unit = { _, _ -> },
@@ -186,7 +188,7 @@ class Orchestra(
                         )
                     )
                 }
-                Insights.onInsightsUpdated(callback)
+                insights.onInsightsUpdated(callback)
 
                 try {
                     try {
@@ -199,7 +201,7 @@ class Orchestra(
                     }
                 } catch (ignored: CommandWarned) {
                     // Swallow exception, but add a warning as an insight
-                    Insights.report(Insight(message = ignored.message, level = Insight.Level.WARNING))
+                    insights.report(Insight(message = ignored.message, level = Insight.Level.WARNING))
                     onCommandWarned(index, command)
                 } catch (ignored: CommandSkipped) {
                     // Swallow exception
@@ -210,8 +212,9 @@ class Orchestra(
                         ErrorResolution.FAIL -> return false
                         ErrorResolution.CONTINUE -> {} // Do nothing
                     }
+                } finally {
+                    insights.unregisterListener(callback)
                 }
-                Insights.unregisterListener(callback)
             }
         return true
     }
@@ -690,7 +693,7 @@ class Orchestra(
                         }
                     } catch (ignored: CommandWarned) {
                         // Swallow exception, but add a warning as an insight
-                        Insights.report(Insight(message = ignored.message, level = Insight.Level.WARNING))
+                        insights.report(Insight(message = ignored.message, level = Insight.Level.WARNING))
                         onCommandWarned(index, command)
                         false
                     } catch (ignored: CommandSkipped) {

diff --git a/maestro-studio/server/src/main/java/maestro/studio/InsightService.kt b/maestro-studio/server/src/main/java/maestro/studio/InsightService.kt
@@ -5,12 +5,9 @@ import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import io.ktor.server.application.*
 import io.ktor.server.response.*
 import io.ktor.server.routing.*
-import kotlinx.coroutines.*
 import maestro.studio.BannerMessage.*
 import maestro.utils.Insight
-import maestro.utils.Insights
-import kotlin.coroutines.resume
-import kotlin.coroutines.suspendCoroutine
+import maestro.utils.CliInsights
 
 object InsightService {
 
@@ -41,7 +38,7 @@ object InsightService {
     }
 
     private fun registerInsightUpdateCallback() {
-        Insights.onInsightsUpdated {
+        CliInsights.onInsightsUpdated {
             currentInsight = it
         }
     }

diff --git a/maestro-utils/src/main/kotlin/Insight.kt b/maestro-utils/src/main/kotlin/Insight.kt
@@ -1,31 +1,20 @@
 package maestro.utils
 
-object Insights {
+object CliInsights: Insights {
 
     private var insight: Insight = Insight("", Insight.Level.NONE)
     private val listeners = mutableListOf<(Insight) -> Unit>()
 
-    fun report(insight: Insight) {
-        this.insight = insight
+    override fun report(insight: Insight) {
+        CliInsights.insight = insight
         listeners.forEach { it.invoke(insight) }
     }
 
-    fun onInsightsUpdated(callback: (Insight) -> Unit) {
+    override fun onInsightsUpdated(callback: (Insight) -> Unit) {
         listeners.add(callback)
     }
 
-    fun unregisterListener(callback: (Insight) -> Unit) {
+    override fun unregisterListener(callback: (Insight) -> Unit) {
         listeners.remove(callback)
     }
 }
-
-data class Insight(
-    val message: String,
-    val level: Level
-) {
-    enum class Level {
-        WARNING,
-        INFO,
-        NONE
-    }
-}
diff --git a/maestro-utils/src/main/kotlin/Insights.kt b/maestro-utils/src/main/kotlin/Insights.kt
@@ -0,0 +1,38 @@
+package maestro.utils
+
+interface Insights {
+
+    fun report(insight: Insight)
+
+    fun onInsightsUpdated(callback: (Insight) -> Unit)
+
+    fun unregisterListener(callback: (Insight) -> Unit)
+}
+
+object NoopInsights: Insights {
+
+    override fun report(insight: Insight) {
+        /* no-op */
+    }
+
+    override fun onInsightsUpdated(callback: (Insight) -> Unit) {
+        /* no-op */
+    }
+
+    override fun unregisterListener(callback: (Insight) -> Unit) {
+        /* no-op */
+    }
+
+}
+
+
+data class Insight(
+    val message: String,
+    val level: Level
+) {
+    enum class Level {
+        WARNING,
+        INFO,
+        NONE
+    }
+}
diff --git a/maestro-utils/src/test/kotlin/InsightTest.kt b/maestro-utils/src/test/kotlin/InsightTest.kt
@@ -1,18 +1,18 @@
 import maestro.utils.Insight
-import maestro.utils.Insights
+import maestro.utils.CliInsights
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test
 
-class InsightsTest {
+class CliInsightsTest {
 
     @Test
     fun `report should update insight and notify listeners`() {
         val insight = Insight("Test message", Insight.Level.INFO)
         var notifiedInsight: Insight? = null
 
-        Insights.onInsightsUpdated { notifiedInsight = it }
-        Insights.report(insight)
+        CliInsights.onInsightsUpdated { notifiedInsight = it }
+        CliInsights.report(insight)
 
         assertEquals(insight, notifiedInsight)
     }
@@ -22,8 +22,8 @@ class InsightsTest {
         val insight = Insight("Test message", Insight.Level.INFO)
         var notified = false
 
-        Insights.onInsightsUpdated { notified = true }
-        Insights.report(insight)
+        CliInsights.onInsightsUpdated { notified = true }
+        CliInsights.report(insight)
 
         assertTrue(notified)
     }
@@ -34,9 +34,9 @@ class InsightsTest {
         var notified = false
         val listener: (Insight) -> Unit = { notified = true }
 
-        Insights.onInsightsUpdated(listener)
-        Insights.unregisterListener(listener)
-        Insights.report(insight)
+        CliInsights.onInsightsUpdated(listener)
+        CliInsights.unregisterListener(listener)
+        CliInsights.report(insight)
 
         assertTrue(!notified)
     }