feat: take device screenshots on-demand (#904)

Author: noomorph

detox/src/artifacts/ArtifactsManager.js

@@ -81,8 +81,9 @@ class ArtifactsManager {
     deviceEmitter.on('shutdownDevice', this.onShutdownDevice.bind(this));
     deviceEmitter.on('beforeLaunchApp', this.onBeforeLaunchApp.bind(this));
     deviceEmitter.on('launchApp', this.onLaunchApp.bind(this));
-    deviceEmitter.on('beforeTerminateApp', this.onBeforeTerminateApp.bind(this));
     deviceEmitter.on('beforeUninstallApp', this.onBeforeUninstallApp.bind(this));
+    deviceEmitter.on('beforeTerminateApp', this.onBeforeTerminateApp.bind(this));
+    deviceEmitter.on('userAction', this.onUserAction.bind(this));
   }
 
   async onBootDevice(deviceInfo) {
@@ -113,6 +114,10 @@ class ArtifactsManager {
     await this._callPlugins('onLaunchApp', appLaunchInfo);
   }
 
+  async onUserAction(actionInfo) {
+    await this._callPlugins('onUserAction', actionInfo);
+  }
+
   async onBeforeAll() {
     await this._callPlugins('onBeforeAll');
   }

detox/src/artifacts/ArtifactsManager.test.js

@@ -91,9 +91,10 @@ describe('ArtifactsManager', () => {
           onBeforeShutdownDevice: jest.fn(),
           onShutdownDevice: jest.fn(),
           onBeforeUninstallApp: jest.fn(),
+          onBeforeTerminateApp: jest.fn(),
           onBeforeLaunchApp: jest.fn(),
           onLaunchApp: jest.fn(),
-          onBeforeTerminateApp: jest.fn(),
+          onUserAction: jest.fn(),
           onBeforeAll: jest.fn(),
           onBeforeEach: jest.fn(),
           onAfterEach: jest.fn(),
@@ -252,6 +253,13 @@ describe('ArtifactsManager', () => {
           deviceId: 'testDeviceId',
         }));
 
+        itShouldCatchErrorsOnPhase('onUserAction', () => ({
+          type: 'takeScreenshot',
+          options: {
+            name: 'open app',
+          }
+        }));
+
         itShouldCatchErrorsOnPhase('onBeforeShutdownDevice', () => ({
           deviceId: 'testDeviceId'
         }));
@@ -390,6 +398,20 @@ describe('ArtifactsManager', () => {
       });
     });
 
+    describe('onUserAction', () => {
+      it('should call onUserAction in plugins', async () => {
+        const actionInfo = {
+          type: 'takeScreenshot',
+          options: {
+            name: 'open app',
+          },
+        };
+
+        await artifactsManager.onUserAction(actionInfo);
+        expect(testPlugin.onUserAction).toHaveBeenCalledWith(actionInfo);
+      });
+    });
+
     describe('onBeforeLaunchApp', () => {
       it('should call onBeforeLaunchApp in plugins', async () => {
         const launchInfo = {

detox/src/artifacts/__snapshots__/ArtifactsManager.test.js.snap

@@ -169,3 +169,17 @@ Array [
   ],
 ]
 `;
+
+exports[`ArtifactsManager .artifactsApi hooks error handling should catch .onUserAction errors 1`] = `
+Array [
+  Array [
+    Object {
+      "err": [Error: test onUserAction error],
+      "event": "SUPPRESS_PLUGIN_ERROR",
+      "methodName": "onUserAction",
+      "plugin": "testPlugin",
+    },
+    "Suppressed error inside function call: testPlugin.onUserAction({ type: 'takeScreenshot', options: { name: 'open app' } })",
+  ],
+]
+`;

detox/src/artifacts/log/ios/SimulatorLogPlugin.test.js

@@ -2,9 +2,11 @@ jest.mock('../../../utils/argparse');
 jest.mock('../../../utils/logger');
 
 const _ = require('lodash');
+const os = require('os');
 const tempfile = require('tempfile');
 const fs = require('fs-extra');
 const path = require('path');
+const sleep = require('../../../utils/sleep');
 
 describe('SimulatorLogPlugin', () => {
   async function majorWorkflow() {
@@ -88,6 +90,10 @@ describe('SimulatorLogPlugin', () => {
     await artifactsManager.onBeforeAll();
     await logToDeviceLogs('inside before all');
 
+    if (os.platform() !== 'darwin') {
+      await sleep(1000); // HACK: till we replace `tail` with something less flaky
+    }
+
     await artifactsManager.onBeforeEach({ title: 'test', fullName: 'some test', status: 'running'});
     await logToDeviceLogs('inside before each');
 

detox/src/artifacts/screenshot/ScreenshotArtifactPlugin.js

@@ -1,4 +1,6 @@
+const _ = require('lodash');
 const argparse = require('../../utils/argparse');
+const log = require('../../utils/logger').child({ __filename });
 const TwoSnapshotsPerTestPlugin = require('../templates/plugin/TwoSnapshotsPerTestPlugin');
 
 /***
@@ -12,12 +14,50 @@ class ScreenshotArtifactPlugin extends TwoSnapshotsPerTestPlugin {
 
     this.enabled = takeScreenshots && takeScreenshots !== 'none';
     this.keepOnlyFailedTestsArtifacts = takeScreenshots === 'failing';
+    this._warnAboutNotImplementedMode = _.once(this._warnAboutNotImplementedMode.bind(this));
   }
 
-  async preparePathForSnapshot(testSummary, index) {
-    const pngName = index === 0 ? 'beforeEach.png' : 'afterEach.png';
-    return this.api.preparePathForArtifact(pngName, testSummary);
+  async onBeforeEach(testSummary) {
+    this.context.testSummary = null;
+    await this._flushScreenshots();
+
+    await super.onBeforeEach(testSummary);
+  }
+
+  async onAfterEach(testSummary) {
+    await super.onAfterEach(testSummary);
+    await this._flushScreenshots();
+  }
+
+  async onAfterAll() {
+    await super.onAfterAll();
+    await this._flushScreenshots();
+  }
+
+  async preparePathForSnapshot(testSummary, name) {
+    return this.api.preparePathForArtifact(`${name}.png`, testSummary);
+  }
+
+  async onUserAction({ type, options }) {
+    switch (type) {
+      case 'takeScreenshot':
+        return this.keepOnlyFailedTestsArtifacts
+          ? this._warnAboutNotImplementedMode()
+          : this.takeSnapshot(options.name);
+    }
+  }
+
+  _warnAboutNotImplementedMode() {
+    const message = 'Taking screenshots on-demand is not supported yet for `--take-screenshots failing` mode.';
+    log.warn({ event: 'TAKE_SCREENSHOT_IGNORED' }, message);
+  }
+
+  async _flushScreenshots() {
+    for (const name of Object.keys(this.snapshots)) {
+      this.startSavingSnapshot(name);
+      delete this.snapshots[name];
+    }
   }
 }
 
-module.exports = ScreenshotArtifactPlugin;
+module.exports = ScreenshotArtifactPlugin;

detox/src/artifacts/templates/plugin/ArtifactPlugin.js

@@ -159,6 +159,18 @@ class ArtifactPlugin {
     });
   }
 
+  /**
+   * Hook that is called on demand by some of user actions (e.g. device.takeScreeenshot())
+   *
+   * @protected
+   * @async
+   * @param {Object} event - User action event object
+   * @param {string} event.type - Action type
+   * @param {string} event.options - Action options
+   * @return {Promise<any>} - appropriate result of the action
+   */
+  async onUserAction(event) {}
+
   /**
    * Hook that is called before any test begins
    *
@@ -218,9 +230,10 @@ class ArtifactPlugin {
     this.onBootDevice = _.noop;
     this.onBeforeShutdownDevice = _.noop;
     this.onShutdownDevice = _.noop;
+    this.onBeforeTerminateApp = _.noop;
     this.onBeforeLaunchApp = _.noop;
     this.onLaunchApp = _.noop;
-    this.onBeforeTerminateApp = _.noop;
+    this.onUserAction = _.noop;
     this.onBeforeAll = _.noop;
     this.onBeforeEach = _.noop;
     this.onAfterEach = _.noop;

detox/src/artifacts/templates/plugin/ArtifactPlugin.test.js

@@ -140,6 +140,10 @@ describe(ArtifactPlugin, () => {
       expect(plugin.context).toMatchSnapshot();
     });
 
+    it('should have .onUserAction', async () => {
+      await expect(plugin.onUserAction()).resolves.toBe(void 0);
+    });
+
     it('should have .onBeforeAll, which resets context.testSummary if called', async () => {
       plugin.context.testSummary = {};
       await plugin.onBeforeAll();
@@ -184,6 +188,7 @@ describe(ArtifactPlugin, () => {
         expect(plugin.onBeforeEach).toBe(plugin.onTerminate);
         expect(plugin.onAfterEach).toBe(plugin.onTerminate);
         expect(plugin.onAfterAll).toBe(plugin.onTerminate);
+        expect(plugin.onUserAction).toBe(plugin.onTerminate);
       });
 
       it('should not work after the first call', async () => {

detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.js

@@ -6,30 +6,33 @@ const ArtifactPlugin = require('./ArtifactPlugin');
 class TwoSnapshotsPerTestPlugin extends ArtifactPlugin {
   constructor({ api }) {
     super({ api });
-    this._snapshots = [null, null];
+    this.snapshots = {};
   }
 
-  async onBeforeEach() {
-    await this._takeSnapshot(0);
+  async onBeforeEach(testSummary) {
+    await super.onBeforeEach(testSummary);
+    await this.takeSnapshot('beforeEach');
   }
 
   async onAfterEach(testSummary) {
+    await super.onAfterEach(testSummary);
+
     if (this.shouldKeepArtifactOfTest(testSummary)) {
-      await this._takeSnapshot(1);
-      this._startSavingSnapshot(testSummary, 0);
-      this._startSavingSnapshot(testSummary, 1);
+      await this.takeSnapshot('afterEach');
+      this.startSavingSnapshot('beforeEach');
+      this.startSavingSnapshot('afterEach');
     } else {
-      this._startDiscardingSnapshot(0);
+      this.startDiscardingSnapshot('beforeEach');
     }
 
-    this._clearSnapshotReferences();
+    this._clearAutomaticSnapshotReferences();
   }
 
   /***
    * @protected
    * @abstract
    */
-  async preparePathForSnapshot(testSummary, index) {}
+  async preparePathForSnapshot(testSummary, snapshotName) {}
 
 
   /***
@@ -41,33 +44,48 @@ class TwoSnapshotsPerTestPlugin extends ArtifactPlugin {
    */
   createTestArtifact() {}
 
-  async _takeSnapshot(index) {
+  _clearAutomaticSnapshotReferences() {
+    delete this.snapshots.beforeEach;
+    delete this.snapshots.afterEach;
+  }
+
+  /***
+   * @protected
+   */
+  async takeSnapshot(name) {
     if (!this.enabled) {
       return;
     }
 
-    const snapshot = this.createTestArtifact();
+    const snapshot = this.snapshots[name] = this.createTestArtifact();
     await snapshot.start();
     await snapshot.stop();
-    this._snapshots[index] = snapshot;
+
     this.api.trackArtifact(snapshot);
   }
 
-  _startSavingSnapshot(testSummary, index) {
-    const snapshot = this._snapshots[index];
+  /***
+   * @protected
+   */
+  startSavingSnapshot(name) {
+    const snapshot = this.snapshots[name];
     if (!snapshot) {
       return;
     }
 
+    const {testSummary} = this.context;
     this.api.requestIdleCallback(async () => {
-      const snapshotArtifactPath = await this.preparePathForSnapshot(testSummary, index);
+      const snapshotArtifactPath = await this.preparePathForSnapshot(testSummary, name);
       await snapshot.save(snapshotArtifactPath);
       this.api.untrackArtifact(snapshot);
     });
   }
 
-  _startDiscardingSnapshot(index) {
-    const snapshot = this._snapshots[index];
+  /***
+   * @protected
+   */
+  startDiscardingSnapshot(name) {
+    const snapshot = this.snapshots[name];
     if (!snapshot) {
       return;
     }
@@ -77,11 +95,6 @@ class TwoSnapshotsPerTestPlugin extends ArtifactPlugin {
       this.api.untrackArtifact(snapshot);
     });
   }
-
-  _clearSnapshotReferences() {
-    this._snapshots[0] = null;
-    this._snapshots[1] = null;
-  }
 }
 
 module.exports = TwoSnapshotsPerTestPlugin;

detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.test.js

@@ -110,7 +110,7 @@ describe('TwoSnapshotsPerTestPlugin', () => {
 
         await saveRequest();
 
-        expect(plugin.createdArtifacts[0].save).toBeCalledWith('test/0.png');
+        expect(plugin.createdArtifacts[0].save).toBeCalledWith('test/beforeEach.png');
         expect(api.untrackArtifact).toBeCalledWith(plugin.createdArtifacts[0]);
       });
 
@@ -122,7 +122,7 @@ describe('TwoSnapshotsPerTestPlugin', () => {
 
         await saveRequest();
 
-        expect(plugin.createdArtifacts[1].save).toBeCalledWith('test/1.png');
+        expect(plugin.createdArtifacts[1].save).toBeCalledWith('test/afterEach.png');
         expect(api.untrackArtifact).toBeCalledWith(plugin.createdArtifacts[1]);
       });
     });

detox/src/devices/Device.js

@@ -96,6 +96,14 @@ class Device {
     }
   }
 
+  async takeScreenshot(name) {
+    if (!name) {
+      throw new Error('Cannot take a screenshot with an empty name.');
+    }
+
+    return this.deviceDriver.takeScreenshot(name);
+  }
+
   _isAppInBackground(params, _bundleId) {
     return !params.delete && !params.newInstance && this._processes[_bundleId];
   }