Repository: wix/Detox
Pull request #1823 (2020-01-02 12:37:48)

fix(instruments): should not crash if the app was terminated inside a test
- [x] This is a small change 

---

**Description:**

If we terminate a tested app inside a test, then the test next to it won't pass if you have `--record-performance all` flag turned on. :disappointed:  Detox tries to send `setRecordingState` via `Client.js` right inside `beforeEach` phase, which is a bad idea in the case when the app is not running.


Patch:
diff --git a/detox/src/artifacts/ArtifactsManager.js b/detox/src/artifacts/ArtifactsManager.js
index d9b249ba5..f64627670 100644
--- a/detox/src/artifacts/ArtifactsManager.js
+++ b/detox/src/artifacts/ArtifactsManager.js
@@ -96,6 +96,7 @@ class ArtifactsManager {
     deviceEmitter.on('launchApp', this.onLaunchApp.bind(this));
     deviceEmitter.on('beforeUninstallApp', this.onBeforeUninstallApp.bind(this));
     deviceEmitter.on('beforeTerminateApp', this.onBeforeTerminateApp.bind(this));
+    deviceEmitter.on('terminateApp', this.onTerminateApp.bind(this));
     deviceEmitter.on('createExternalArtifact', this.onCreateExternalArtifact.bind(this));
   }
 
@@ -107,6 +108,10 @@ class ArtifactsManager {
     await this._callPlugins('plain', 'onBeforeTerminateApp', appInfo);
   }
 
+  async onTerminateApp(appInfo) {
+    await this._callPlugins('plain', 'onTerminateApp', appInfo);
+  }
+
   async onBeforeUninstallApp(appInfo) {
     await this._callPlugins('plain', 'onBeforeUninstallApp', appInfo);
   }
diff --git a/detox/src/artifacts/instruments/SimulatorInstrumentsPlugin.js b/detox/src/artifacts/instruments/SimulatorInstrumentsPlugin.js
index 94bcaa245..bd35f0e76 100644
--- a/detox/src/artifacts/instruments/SimulatorInstrumentsPlugin.js
+++ b/detox/src/artifacts/instruments/SimulatorInstrumentsPlugin.js
@@ -51,6 +51,7 @@ class SimulatorInstrumentsPlugin extends WholeTestRecorderPlugin {
 
   createTestRecording() {
     return new SimulatorInstrumentsRecording({
+      pluginContext: this.context,
       client: this.client,
       temporaryRecordingPath: temporaryPath.for.dtxrec(),
     });
diff --git a/detox/src/artifacts/instruments/SimulatorInstrumentsRecording.js b/detox/src/artifacts/instruments/SimulatorInstrumentsRecording.js
index ec1e2bf9d..db11ce133 100644
--- a/detox/src/artifacts/instruments/SimulatorInstrumentsRecording.js
+++ b/detox/src/artifacts/instruments/SimulatorInstrumentsRecording.js
@@ -5,11 +5,13 @@ const FileArtifact = require('../templates/artifact/FileArtifact');
 
 class SimulatorInstrumentsRecording extends Artifact {
   constructor({
+    pluginContext,
     client,
     temporaryRecordingPath,
   }) {
     super();
 
+    this._pluginContext = pluginContext;
     this._client = client;
     this.temporaryRecordingPath = temporaryRecordingPath;
   }
@@ -46,7 +48,10 @@ class SimulatorInstrumentsRecording extends Artifact {
   }
 
   _isClientConnected() {
-    return this._client.isConnected && !this._client.pandingAppCrash;
+    const isConnectedToDetoxServer = this._client.isConnected && !this._client.pandingAppCrash;
+    const isAppRunning = this._pluginContext.bundleId;
+
+    return Boolean(isConnectedToDetoxServer && isAppRunning);
   }
 }
 
diff --git a/detox/src/artifacts/log/ios/SimulatorLogPlugin.js b/detox/src/artifacts/log/ios/SimulatorLogPlugin.js
index 25f1a438a..eb48170f9 100644
--- a/detox/src/artifacts/log/ios/SimulatorLogPlugin.js
+++ b/detox/src/artifacts/log/ios/SimulatorLogPlugin.js
@@ -17,8 +17,8 @@ class SimulatorLogPlugin extends LogArtifactPlugin {
 
     if (this.currentRecording) {
       await this.currentRecording.start({
-        udid: this.context.deviceId,
-        bundleId: this.context.bundleId,
+        udid: event.deviceId,
+        bundleId: event.bundleId,
       });
     }
   }
diff --git a/detox/src/artifacts/templates/plugin/ArtifactPlugin.js b/detox/src/artifacts/templates/plugin/ArtifactPlugin.js
index 0914ecbf2..678342fa5 100644
--- a/detox/src/artifacts/templates/plugin/ArtifactPlugin.js
+++ b/detox/src/artifacts/templates/plugin/ArtifactPlugin.js
@@ -46,14 +46,7 @@ class ArtifactPlugin {
    * @param {Object} event.launchArgs - Mutable key-value pairs of args before the launch
    * @return {Promise<void>} - when done
    */
-  async onBeforeLaunchApp(event) {
-    Object.assign(this.context, {
-      bundleId: event.bundleId,
-      deviceId: event.deviceId,
-      launchArgs: event.launchArgs,
-      pid: NaN,
-    });
-  }
+  async onBeforeLaunchApp(event) {}
 
   /**
    * Hook that is called inside device.launchApp() and
@@ -72,7 +65,6 @@ class ArtifactPlugin {
   async onLaunchApp(event) {
     Object.assign(this.context, {
       bundleId: event.bundleId,
-      deviceId: event.deviceId,
       launchArgs: event.launchArgs,
       pid: event.pid,
    });
@@ -91,8 +83,6 @@ class ArtifactPlugin {
   async onBootDevice(event) {
     Object.assign(this.context, {
       deviceId: event.deviceId,
-      bundleId: '',
-      pid: NaN,
     });
   }
 
@@ -106,10 +96,23 @@ class ArtifactPlugin {
    * @param {string} event.bundleId - Current bundleId
    * @return {Promise<void>} - when done
    */
-  async onBeforeTerminateApp(event) {
+  async onBeforeTerminateApp(event) {}
+
+  /**
+   * Hook that is supposed to be called after app has been terminated
+   *
+   * @protected
+   * @async
+   * @param {Object} event - App termination event object
+   * @param {string} event.deviceId - Current deviceId
+   * @param {string} event.bundleId - Terminated bundleId
+   * @return {Promise<void>} - when done
+   */
+  async onTerminateApp(event) {
     Object.assign(this.context, {
-      deviceId: event.deviceId,
-      bundleId: event.bundleId,
+      bundleId: '',
+      launchArgs: null,
+      pid: NaN,
     });
   }
 
@@ -123,12 +126,7 @@ class ArtifactPlugin {
    * @param {string} event.bundleId - Current bundleId
    * @return {Promise<void>} - when done
    */
-  async onBeforeUninstallApp(event) {
-    Object.assign(this.context, {
-      deviceId: event.deviceId,
-      bundleId: event.bundleId,
-    });
-  }
+  async onBeforeUninstallApp(event) {}
 
   /**
    * Hook that is supposed to be called before device.shutdown() happens
@@ -139,11 +137,7 @@ class ArtifactPlugin {
    * @param {string} event.deviceId - Current deviceId
    * @return {Promise<void>} - when done
    */
-  async onBeforeShutdownDevice(event) {
-    Object.assign(this.context, {
-      deviceId: event.deviceId,
-    });
-  }
+  async onBeforeShutdownDevice(event) {}
 
   /**
    * Hook that is supposed to be called from device.shutdown()
@@ -156,8 +150,9 @@ class ArtifactPlugin {
    */
   async onShutdownDevice(event) {
     Object.assign(this.context, {
-      deviceId: event.deviceId,
+      deviceId: '',
       bundleId: '',
+      launchArgs: null,
       pid: NaN,
     });
   }
@@ -242,6 +237,7 @@ class ArtifactPlugin {
     this.onBeforeShutdownDevice = _.noop;
     this.onShutdownDevice = _.noop;
     this.onBeforeTerminateApp = _.noop;
+    this.onTerminateApp = _.noop;
     this.onBeforeLaunchApp = _.noop;
     this.onLaunchApp = _.noop;
     this.onUserAction = _.noop;
diff --git a/detox/src/devices/drivers/AndroidDriver.js b/detox/src/devices/drivers/AndroidDriver.js
index 36042aa1e..29d82b555 100644
--- a/detox/src/devices/drivers/AndroidDriver.js
+++ b/detox/src/devices/drivers/AndroidDriver.js
@@ -131,6 +131,7 @@ class AndroidDriver extends DeviceDriverBase {
     await this.emitter.emit('beforeTerminateApp', { deviceId, bundleId });
     await this._terminateInstrumentation();
     await this.adb.terminate(deviceId, bundleId);
+    await this.emitter.emit('terminateApp', { deviceId, bundleId });
   }
 
   async _terminateInstrumentation() {
diff --git a/detox/src/devices/drivers/DeviceDriverBase.js b/detox/src/devices/drivers/DeviceDriverBase.js
index 7e5886354..db7951e4c 100644
--- a/detox/src/devices/drivers/DeviceDriverBase.js
+++ b/detox/src/devices/drivers/DeviceDriverBase.js
@@ -15,6 +15,7 @@ class DeviceDriverBase {
         'beforeShutdownDevice',
         'shutdownDevice',
         'beforeTerminateApp',
+        'terminateApp',
         'beforeUninstallApp',
         'beforeLaunchApp',
         'launchApp',
diff --git a/detox/src/devices/drivers/SimulatorDriver.js b/detox/src/devices/drivers/SimulatorDriver.js
index 0c705e6f2..9022c907e 100644
--- a/detox/src/devices/drivers/SimulatorDriver.js
+++ b/detox/src/devices/drivers/SimulatorDriver.js
@@ -94,6 +94,7 @@ class SimulatorDriver extends IosDriver {
   async terminate(deviceId, bundleId) {
     await this.emitter.emit('beforeTerminateApp', { deviceId, bundleId });
     await this.applesimutils.terminate(deviceId, bundleId);
+    await this.emitter.emit('terminateApp', { deviceId, bundleId });
   }
 
   async setBiometricEnrollment(deviceId, yesOrNo) {


Test patch:
diff --git a/detox/src/artifacts/ArtifactsManager.test.js b/detox/src/artifacts/ArtifactsManager.test.js
index 996bf9d8f..53fef3a3f 100644
--- a/detox/src/artifacts/ArtifactsManager.test.js
+++ b/detox/src/artifacts/ArtifactsManager.test.js
@@ -117,6 +117,7 @@ describe('ArtifactsManager', () => {
           onShutdownDevice: jest.fn(),
           onBeforeUninstallApp: jest.fn(),
           onBeforeTerminateApp: jest.fn(),
+          onTerminateApp: jest.fn(),
           onBeforeLaunchApp: jest.fn(),
           onLaunchApp: jest.fn(),
           onCreateExternalArtifact: jest.fn(),
@@ -326,6 +327,11 @@ describe('ArtifactsManager', () => {
           deviceId: 'testDeviceId',
         }));
 
+        itShouldCatchErrorsOnPhase('onTerminateApp', () => ({
+          bundleId: 'testBundleId',
+          deviceId: 'testDeviceId',
+        }));
+
         itShouldCatchErrorsOnPhase('onBeforeUninstallApp', () => ({
           bundleId: 'testBundleId',
           deviceId: 'testDeviceId',
@@ -402,6 +408,19 @@ describe('ArtifactsManager', () => {
         });
       });
 
+      describe('onTerminateApp', () => {
+        it('should call onTerminateApp in plugins', async () => {
+          const terminateInfo = {
+            deviceId: 'testDeviceId',
+            bundleId: 'testBundleId',
+          };
+
+          expect(testPlugin.onBeforeTerminateApp).not.toHaveBeenCalled();
+          await artifactsManager.onBeforeTerminateApp(terminateInfo);
+          expect(testPlugin.onBeforeTerminateApp).toHaveBeenCalledWith(terminateInfo);
+        });
+      });
+
       describe('onBeforeUninstallApp', () => {
         it('should call onBeforeUninstallApp in plugins', async () => {
           const uninstallInfo = {
diff --git a/detox/src/artifacts/templates/plugin/ArtifactPlugin.test.js b/detox/src/artifacts/templates/plugin/ArtifactPlugin.test.js
index c78678492..f6d4d31ee 100644
--- a/detox/src/artifacts/templates/plugin/ArtifactPlugin.test.js
+++ b/detox/src/artifacts/templates/plugin/ArtifactPlugin.test.js
@@ -73,12 +73,12 @@ describe('ArtifactPlugin', () => {
   describe('lifecycle hooks', () => {
     beforeEach(() => {
       plugin.context = {
-        deviceId: 'shouldNotBeInExpectSnapshots',
+        deviceId: 'someOriginalDeviceId',
         shouldNotBeDeletedFromContext: 'extraProperty'
       };
     });
 
-    it('should update context on .onBeforeLaunchApp', async () => {
+    it('should not update context on .onBeforeLaunchApp', async () => {
       await expect(plugin.onBeforeLaunchApp({
         deviceId: 'testDeviceId',
         bundleId: 'testBundleId',
@@ -88,17 +88,12 @@ describe('ArtifactPlugin', () => {
       }));
 
       expect(plugin.context).toEqual({
-        bundleId: 'testBundleId',
-        deviceId: 'testDeviceId',
-        launchArgs: {
-          detoxSessionId: 'test',
-        },
-        pid: NaN,
-        shouldNotBeDeletedFromContext: 'extraProperty',
+        deviceId: 'someOriginalDeviceId',
+        shouldNotBeDeletedFromContext: 'extraProperty'
       });
     });
 
-    it('should have .onLaunchApp', async () => {
+    it('should update context .onLaunchApp', async () => {
       await expect(plugin.onLaunchApp({
         deviceId: 'testDeviceId',
         bundleId: 'testBundleId',
@@ -110,7 +105,7 @@ describe('ArtifactPlugin', () => {
 
       expect(plugin.context).toEqual({
         bundleId: 'testBundleId',
-        deviceId: 'testDeviceId',
+        deviceId: 'someOriginalDeviceId',
         launchArgs: {
           detoxSessionId: 'test',
         },
@@ -119,53 +114,64 @@ describe('ArtifactPlugin', () => {
       });
     });
 
-    it('should update context on .onBeforeUninstallApp', async () => {
+    it('should not update context on .onBeforeUninstallApp', async () => {
       await expect(plugin.onBeforeUninstallApp({
         deviceId: 'testDeviceId',
         bundleId: 'testBundleId',
       }));
 
       expect(plugin.context).toEqual({
-        bundleId: 'testBundleId',
-        deviceId: 'testDeviceId',
-        shouldNotBeDeletedFromContext: 'extraProperty',
+        deviceId: 'someOriginalDeviceId',
+        shouldNotBeDeletedFromContext: 'extraProperty'
       });
     });
 
-    it('should update context on .onBeforeTerminateApp', async () => {
+    it('should not update context on .onBeforeTerminateApp', async () => {
       await expect(plugin.onBeforeTerminateApp({
         deviceId: 'testDeviceId',
         bundleId: 'testBundleId',
       }));
 
       expect(plugin.context).toEqual({
-        bundleId: 'testBundleId',
+        deviceId: 'someOriginalDeviceId',
+        shouldNotBeDeletedFromContext: 'extraProperty'
+      });
+    });
+
+    it('should update context on .onTerminateApp', async () => {
+      await expect(plugin.onTerminateApp({
         deviceId: 'testDeviceId',
+        bundleId: 'testBundleId',
+      }));
+
+      expect(plugin.context).toEqual({
+        bundleId: '',
+        launchArgs: null,
+        pid: NaN,
+        deviceId: 'someOriginalDeviceId',
         shouldNotBeDeletedFromContext: 'extraProperty',
       });
     });
 
-    it('should have .onBootDevice', async () => {
+    it('should update context on .onBootDevice', async () => {
       await expect(plugin.onBootDevice({
         deviceId: 'testDeviceId',
         coldBoot: true
       }));
 
       expect(plugin.context).toEqual({
-        bundleId: '',
         deviceId: 'testDeviceId',
-        pid: NaN,
         shouldNotBeDeletedFromContext: 'extraProperty',
       });
     });
 
-    it('should have .onBeforeShutdownDevice', async () => {
+    it('should not update context on .onBeforeShutdownDevice', async () => {
       await expect(plugin.onBeforeShutdownDevice({
         deviceId: 'testDeviceId'
       }));
 
       expect(plugin.context).toEqual({
-        deviceId: 'testDeviceId',
+        deviceId: 'someOriginalDeviceId',
         shouldNotBeDeletedFromContext: 'extraProperty',
       });
     });
@@ -176,8 +182,9 @@ describe('ArtifactPlugin', () => {
       }));
 
       expect(plugin.context).toEqual({
+        deviceId: '',
         bundleId: '',
-        deviceId: 'testDeviceId',
+        launchArgs: null,
         pid: NaN,
         shouldNotBeDeletedFromContext: 'extraProperty',
       });
@@ -232,6 +239,7 @@ describe('ArtifactPlugin', () => {
         expect(plugin.onBeforeLaunchApp).toBe(plugin.onTerminate);
         expect(plugin.onLaunchApp).toBe(plugin.onTerminate);
         expect(plugin.onBeforeTerminateApp).toBe(plugin.onTerminate);
+        expect(plugin.onTerminateApp).toBe(plugin.onTerminate);
         expect(plugin.onBeforeAll).toBe(plugin.onTerminate);
         expect(plugin.onBeforeEach).toBe(plugin.onTerminate);
         expect(plugin.onAfterEach).toBe(plugin.onTerminate);
diff --git a/detox/test/e2e/23.flows.test.js b/detox/test/e2e/23.flows.test.js
index 48e22f929..42f5986f7 100644
--- a/detox/test/e2e/23.flows.test.js
+++ b/detox/test/e2e/23.flows.test.js
@@ -4,4 +4,8 @@ describe('Flows', () => {
     await device.launchApp({newInstance: true});
     await device.terminateApp();
   });
+
+  it('should be able to start the next test with the terminated app', async () => {
+    await device.launchApp({newInstance: true});
+  });
 });