Fix supervised android launch

detox/android/detox/src/full/java/com/wix/detox/ActivityLaunchHelper.kt

@@ -0,0 +1,78 @@
+package com.wix.detox
+
+import android.app.Instrumentation.ActivityMonitor
+import android.content.Context
+import android.content.Intent
+import androidx.test.platform.app.InstrumentationRegistry
+import androidx.test.rule.ActivityTestRule
+
+class ActivityLaunchHelper
+    @JvmOverloads constructor(
+        private val activityTestRule: ActivityTestRule<*>,
+        private val launchArgs: LaunchArgs = LaunchArgs(),
+        private val intentsFactory: LaunchIntentsFactory = LaunchIntentsFactory(),
+        private val notificationDataParserGen: (String) -> NotificationDataParser = { path -> NotificationDataParser(path) }
+) {
+    fun launchActivityUnderTest() {
+        val intent = extractInitialIntent()
+        activityTestRule.launchActivity(intent)
+    }
+
+    fun launchMainActivity() {
+        val activity = activityTestRule.activity
+        launchActivitySync(intentsFactory.activityLaunchIntent(activity))
+    }
+
+    fun startActivityFromUrl(url: String) {
+        launchActivitySync(intentsFactory.intentWithUrl(url, false))
+    }
+
+    fun startActivityFromNotification(dataFilePath: String) {
+        val notificationData = notificationDataParserGen(dataFilePath).toBundle()
+        val intent = intentsFactory.intentWithNotificationData(appContext, notificationData, false)
+        launchActivitySync(intent)
+    }
+
+    private fun extractInitialIntent(): Intent =
+        (if (launchArgs.hasUrlOverride()) {
+            intentsFactory.intentWithUrl(launchArgs.urlOverride, true)
+        } else if (launchArgs.hasNotificationPath()) {
+            val notificationData = notificationDataParserGen(launchArgs.notificationPath).toBundle()
+            intentsFactory.intentWithNotificationData(appContext, notificationData, true)
+        } else {
+            intentsFactory.cleanIntent()
+        }).also {
+            it.putExtra(INTENT_LAUNCH_ARGS_KEY, launchArgs.asIntentBundle())
+        }
+
+    private fun launchActivitySync(intent: Intent) {
+        // Ideally, we would just call sActivityTestRule.launchActivity(intent) and get it over with.
+        // BUT!!! as it turns out, Espresso has an issue where doing this for an activity running in the background
+        // would have Espresso set up an ActivityMonitor which will spend its time waiting for the activity to load, *without
+        // ever being released*. It will finally fail after a 45 seconds timeout.
+        // Without going into full details, it seems that activity test rules were not meant to be used this way. However,
+        // the all-new ActivityScenario implementation introduced in androidx could probably support this (e.g. by using
+        // dedicated methods such as moveToState(), which give better control over the lifecycle).
+        // In any case, this is the core reason for this issue: https://github.com/wix/Detox/issues/1125
+        // What it forces us to do, then, is this -
+        // 1. Launch the activity by "ourselves" from the OS (i.e. using context.startActivity()).
+        // 2. Set up an activity monitor by ourselves -- such that it would block until the activity is ready.
+        // ^ Hence the code below.
+        val activity = activityTestRule.activity
+        val activityMonitor = ActivityMonitor(activity.javaClass.name, null, true)
+        activity.startActivity(intent)
+
+        InstrumentationRegistry.getInstrumentation().run {
+            addMonitor(activityMonitor)
+            waitForMonitorWithTimeout(activityMonitor, ACTIVITY_LAUNCH_TIMEOUT)
+        }
+    }
+
+    private val appContext: Context
+        get() = InstrumentationRegistry.getInstrumentation().targetContext.applicationContext
+
+    companion object {
+        private const val INTENT_LAUNCH_ARGS_KEY = "launchArgs"
+        private const val ACTIVITY_LAUNCH_TIMEOUT = 10000L
+    }
+}

detox/android/detox/src/full/java/com/wix/detox/Detox.java

@@ -1,18 +1,13 @@
 package com.wix.detox;
 
-import android.app.Activity;
-import android.app.Instrumentation;
 import android.content.Context;
-import android.content.Intent;
-import android.os.Bundle;
-
-import com.wix.detox.config.DetoxConfig;
-import com.wix.detox.espresso.UiControllerSpy;
 
 import androidx.annotation.NonNull;
 import androidx.test.platform.app.InstrumentationRegistry;
 import androidx.test.rule.ActivityTestRule;
 
+import com.wix.detox.config.DetoxConfig;
+
 /**
  * <p>Static class.</p>
  *
@@ -67,12 +62,7 @@
  * <p>If not set, then Detox tests are no ops. So it's safe to mix it with other tests.</p>
  */
 public final class Detox {
-    private static final String INTENT_LAUNCH_ARGS_KEY = "launchArgs";
-    private static final long ACTIVITY_LAUNCH_TIMEOUT = 10000L;
-
-    private static final LaunchArgs sLaunchArgs = new LaunchArgs();
-    private static final LaunchIntentsFactory sIntentsFactory = new LaunchIntentsFactory();
-    private static ActivityTestRule sActivityTestRule;
+    private static ActivityLaunchHelper sActivityLaunchHelper;
 
     private Detox() {
     }
@@ -132,72 +122,20 @@ public static void runTests(ActivityTestRule activityTestRule, @NonNull final Co
         DetoxConfig.CONFIG = detoxConfig;
         DetoxConfig.CONFIG.apply();
 
-        sActivityTestRule = activityTestRule;
-
-        UiControllerSpy.attachThroughProxy();
-
-        Intent intent = extractInitialIntent();
-        sActivityTestRule.launchActivity(intent);
-
-        try {
-            DetoxMain.run(context);
-        } catch (Exception e) {
-            Thread.currentThread().interrupt();
-            throw new RuntimeException("Detox got interrupted prematurely", e);
-        }
+        sActivityLaunchHelper = new ActivityLaunchHelper(activityTestRule);
+        DetoxMain.run(context, sActivityLaunchHelper);
     }
 
     public static void launchMainActivity() {
-        final Activity activity = sActivityTestRule.getActivity();
-        launchActivitySync(sIntentsFactory.activityLaunchIntent(activity));
+        sActivityLaunchHelper.launchMainActivity();
     }
 
     public static void startActivityFromUrl(String url) {
-        launchActivitySync(sIntentsFactory.intentWithUrl(url, false));
+        sActivityLaunchHelper.startActivityFromUrl(url);
     }
 
     public static void startActivityFromNotification(String dataFilePath) {
-        Bundle notificationData = new NotificationDataParser(dataFilePath).toBundle();
-        Intent intent = sIntentsFactory.intentWithNotificationData(getAppContext(), notificationData, false);
-        launchActivitySync(intent);
-    }
-
-    private static Intent extractInitialIntent() {
-        Intent intent;
-
-        if (sLaunchArgs.hasUrlOverride()) {
-            intent = sIntentsFactory.intentWithUrl(sLaunchArgs.getUrlOverride(), true);
-        } else if (sLaunchArgs.hasNotificationPath()) {
-            Bundle notificationData = new NotificationDataParser(sLaunchArgs.getNotificationPath()).toBundle();
-            intent = sIntentsFactory.intentWithNotificationData(getAppContext(), notificationData, true);
-        } else {
-            intent = sIntentsFactory.cleanIntent();
-        }
-        intent.putExtra(INTENT_LAUNCH_ARGS_KEY, sLaunchArgs.asIntentBundle());
-        return intent;
-    }
-
-    private static void launchActivitySync(Intent intent) {
-        // Ideally, we would just call sActivityTestRule.launchActivity(intent) and get it over with.
-        // BUT!!! as it turns out, Espresso has an issue where doing this for an activity running in the background
-        // would have Espresso set up an ActivityMonitor which will spend its time waiting for the activity to load, *without
-        // ever being released*. It will finally fail after a 45 seconds timeout.
-        // Without going into full details, it seems that activity test rules were not meant to be used this way. However,
-        // the all-new ActivityScenario implementation introduced in androidx could probably support this (e.g. by using
-        // dedicated methods such as moveToState(), which give better control over the lifecycle).
-        // In any case, this is the core reason for this issue: https://github.com/wix/Detox/issues/1125
-        // What it forces us to do, then, is this -
-        // 1. Launch the activity by "ourselves" from the OS (i.e. using context.startActivity()).
-        // 2. Set up an activity monitor by ourselves -- such that it would block until the activity is ready.
-        // ^ Hence the code below.
-
-        final Instrumentation instrumentation = InstrumentationRegistry.getInstrumentation();
-        final Activity activity = sActivityTestRule.getActivity();
-        final Instrumentation.ActivityMonitor activityMonitor = new Instrumentation.ActivityMonitor(activity.getClass().getName(), null, true);
-
-        activity.startActivity(intent);
-        instrumentation.addMonitor(activityMonitor);
-        instrumentation.waitForMonitorWithTimeout(activityMonitor, ACTIVITY_LAUNCH_TIMEOUT);
+        sActivityLaunchHelper.startActivityFromNotification(dataFilePath);
     }
 
     private static Context getAppContext() {

detox/android/detox/src/full/java/com/wix/detox/DetoxMain.kt

@@ -3,35 +3,62 @@ package com.wix.detox
 import android.content.Context
 import android.util.Log
 import com.wix.detox.adapters.server.*
-import com.wix.detox.common.DetoxLog.Companion.LOG_TAG
+import com.wix.detox.common.DetoxLog
+import com.wix.detox.espresso.UiControllerSpy
 import com.wix.detox.instruments.DetoxInstrumentsManager
 import com.wix.detox.reactnative.ReactNativeExtension
 import com.wix.invoke.MethodInvocation
+import java.util.concurrent.CountDownLatch
 
-private const val INIT_ACTION = "_init"
-private const val IS_READY_ACTION = "isReady"
 private const val TERMINATION_ACTION = "_terminate"
 
 object DetoxMain {
+    private val handshakeLock = CountDownLatch(1)
+
     @JvmStatic
-    fun run(rnHostHolder: Context) {
+    fun run(rnHostHolder: Context, activityLaunchHelper: ActivityLaunchHelper) {
         val detoxServerInfo = DetoxServerInfo()
-        Log.i(LOG_TAG, "Detox server connection details: $detoxServerInfo")
-
         val testEngineFacade = TestEngineFacade()
         val actionsDispatcher = DetoxActionsDispatcher()
-        val externalAdapter = DetoxServerAdapter(actionsDispatcher, detoxServerInfo, IS_READY_ACTION, TERMINATION_ACTION)
-        initActionHandlers(actionsDispatcher, externalAdapter, testEngineFacade, rnHostHolder)
-        actionsDispatcher.dispatchAction(INIT_ACTION, "", 0)
+        val serverAdapter = DetoxServerAdapter(actionsDispatcher, detoxServerInfo, TERMINATION_ACTION)
+
+        initCrashHandler(serverAdapter)
+        initANRListener(serverAdapter)
+        initEspresso()
+        initReactNative()
+
+        setupActionHandlers(actionsDispatcher, serverAdapter, testEngineFacade, rnHostHolder)
+        serverAdapter.connect()
+
+        launchActivityOnCue(rnHostHolder, activityLaunchHelper)
         actionsDispatcher.join()
     }
 
-    private fun doInit(externalAdapter: DetoxServerAdapter, rnHostHolder: Context) {
-        externalAdapter.connect()
+    /**
+     * Launch the tested activity "on cue", namely, right after a connection is established and the handshake
+     * completes successfully.
+     *
+     * This has to be synchronized so that an `isReady` isn't handled *before* the activity is launched (albeit not fully
+     * initialized - all native modules and everything) and a react context is available.
+     *
+     * As a better alternative, it would make sense to execute this as a simple action from within the actions
+     * dispatcher (i.e. handler of `loginSuccess`), in which case, no inter-thread locking would be required
+     * thanks to the usage of Handlers. However, in this type of a solution, errors / crashes would be reported
+     * not by instrumentation itself, but based on the `AppWillTerminateWithError` message; In it's own, it is a good
+     * thing, but for a reason we're not sure of yet, it is ignored by the test runner at this point in the flow.
+     */
+    @Synchronized
+    private fun launchActivityOnCue(rnHostHolder: Context, activityLaunchHelper: ActivityLaunchHelper) {
+        awaitHandshake()
+        launchActivity(rnHostHolder, activityLaunchHelper)
+    }
+
+    private fun awaitHandshake() {
+        handshakeLock.await()
+    }
 
-        initCrashHandler(externalAdapter)
-        initANRListener(externalAdapter)
-        initReactNativeIfNeeded(rnHostHolder)
+    private fun onLoginSuccess() {
+        handshakeLock.countDown()
     }
 
     private fun doTeardown(serverAdapter: DetoxServerAdapter, actionsDispatcher: DetoxActionsDispatcher, testEngineFacade: TestEngineFacade) {
@@ -41,35 +68,28 @@ object DetoxMain {
         actionsDispatcher.teardown()
     }
 
-    private fun initActionHandlers(actionsDispatcher: DetoxActionsDispatcher, serverAdapter: DetoxServerAdapter, testEngineFacade: TestEngineFacade, rnHostHolder: Context) {
+    private fun setupActionHandlers(actionsDispatcher: DetoxActionsDispatcher, serverAdapter: DetoxServerAdapter, testEngineFacade: TestEngineFacade, rnHostHolder: Context) {
+        class SynchronizedActionHandler(private val actionHandler: DetoxActionHandler): DetoxActionHandler {
+            override fun handle(params: String, messageId: Long) {
+                synchronized(this@DetoxMain) {
+                    actionHandler.handle(params, messageId)
+                }
+            }
+        }
+
         // Primary actions
         with(actionsDispatcher) {
-            val rnReloadHandler = ReactNativeReloadActionHandler(rnHostHolder, serverAdapter, testEngineFacade)
+            val readyHandler = SynchronizedActionHandler( ReadyActionHandler(serverAdapter, testEngineFacade) )
+            val rnReloadHandler = SynchronizedActionHandler( ReactNativeReloadActionHandler(rnHostHolder, serverAdapter, testEngineFacade) )
 
-            associateActionHandler(INIT_ACTION, object : DetoxActionHandler {
-                override fun handle(params: String, messageId: Long) =
-                    synchronized(this@DetoxMain) {
-                        [email protected](serverAdapter, rnHostHolder)
-                    }
-            })
-            associateActionHandler(IS_READY_ACTION, ReadyActionHandler(serverAdapter, testEngineFacade))
-
-            associateActionHandler("loginSuccess", ScarceActionHandler())
-            associateActionHandler("reactNativeReload", object: DetoxActionHandler {
-                override fun handle(params: String, messageId: Long) =
-                    synchronized(this@DetoxMain) {
-                        rnReloadHandler.handle(params, messageId)
-                    }
-            })
+            associateActionHandler("loginSuccess", ::onLoginSuccess)
+            associateActionHandler("isReady", readyHandler)
+            associateActionHandler("reactNativeReload", rnReloadHandler)
             associateActionHandler("invoke", InvokeActionHandler(MethodInvocation(), serverAdapter))
             associateActionHandler("cleanup", CleanupActionHandler(serverAdapter, testEngineFacade) {
                 dispatchAction(TERMINATION_ACTION, "", 0)
             })
-            associateActionHandler(TERMINATION_ACTION, object: DetoxActionHandler {
-                override fun handle(params: String, messageId: Long) {
-                    [email protected](serverAdapter, actionsDispatcher, testEngineFacade)
-                }
-            })
+            associateActionHandler(TERMINATION_ACTION) { -> doTeardown(serverAdapter, actionsDispatcher, testEngineFacade) }
 
             if (DetoxInstrumentsManager.supports()) {
                 val instrumentsManager = DetoxInstrumentsManager(rnHostHolder)
@@ -80,13 +100,8 @@ object DetoxMain {
 
         // Secondary actions
         with(actionsDispatcher) {
-            val queryStatusHandler = QueryStatusActionHandler(serverAdapter, testEngineFacade)
-            associateActionHandler("currentStatus", object: DetoxActionHandler {
-                override fun handle(params: String, messageId: Long) =
-                    synchronized(this@DetoxMain) {
-                        queryStatusHandler.handle(params, messageId)
-                    }
-            }, false)
+            val queryStatusHandler = SynchronizedActionHandler( QueryStatusActionHandler(serverAdapter, testEngineFacade) )
+            associateSecondaryActionHandler("currentStatus", queryStatusHandler)
         }
     }
 
@@ -98,7 +113,17 @@ object DetoxMain {
         DetoxANRHandler(outboundServerAdapter).attach()
     }
 
-    private fun initReactNativeIfNeeded(rnHostHolder: Context) {
+    private fun initEspresso() {
+        UiControllerSpy.attachThroughProxy()
+    }
+
+    private fun initReactNative() {
+        ReactNativeExtension.initIfNeeded()
+    }
+
+    private fun launchActivity(rnHostHolder: Context, activityLaunchHelper: ActivityLaunchHelper) {
+        Log.i(DetoxLog.LOG_TAG, "Launching the tested activity!")
+        activityLaunchHelper.launchActivityUnderTest()
         ReactNativeExtension.waitForRNBootstrap(rnHostHolder)
     }
 }

detox/android/detox/src/full/java/com/wix/detox/LaunchIntentsFactory.kt

@@ -6,7 +6,7 @@ import android.content.Intent
 import android.net.Uri
 import android.os.Bundle
 
-internal class LaunchIntentsFactory {
+class LaunchIntentsFactory {
 
     /**
      * Constructs an intent tightly associated with a specific activity.

detox/android/detox/src/full/java/com/wix/detox/NotificationDataParser.kt

@@ -5,7 +5,7 @@ import com.wix.detox.common.JsonConverter
 import com.wix.detox.common.TextFileReader
 import org.json.JSONObject
 
-internal class NotificationDataParser(private val notificationPath: String) {
+class NotificationDataParser(private val notificationPath: String) {
     fun toBundle(): Bundle {
         val rawData = readNotificationFromFile()
         val json = JSONObject(rawData)

detox/android/detox/src/full/java/com/wix/detox/adapters/server/DetoxActionHandlers.kt

@@ -152,7 +152,3 @@ class InstrumentsEventsActionsHandler(
         outboundServerAdapter.sendMessage("eventDone", emptyMap<String, Any>(), messageId)
     }
 }
-
-class ScarceActionHandler: DetoxActionHandler {
-    override fun handle(params: String, messageId: Long) {}
-}