diff --git a/detox/src/artifacts/ArtifactsManager.js b/detox/src/artifacts/ArtifactsManager.js index 15ab7981b6..3e0e15f1dd 100644 --- a/detox/src/artifacts/ArtifactsManager.js +++ b/detox/src/artifacts/ArtifactsManager.js @@ -1,6 +1,7 @@ const _ = require('lodash'); const fs = require('fs-extra'); const path = require('path'); +const util = require('util'); const log = require('../utils/logger').child({ __filename }); const argparse = require('../utils/argparse'); const DetoxRuntimeError = require('../errors/DetoxRuntimeError'); @@ -8,23 +9,21 @@ const ArtifactPathBuilder = require('./utils/ArtifactPathBuilder'); class ArtifactsManager { constructor(pathBuilder) { - this.onBootDevice = this.onBootDevice.bind(this); - this.onShutdownDevice = this.onShutdownDevice.bind(this); - this.onBeforeLaunchApp = this.onBeforeLaunchApp.bind(this); - this.onLaunchApp = this.onLaunchApp.bind(this); this.onTerminate = _.once(this.onTerminate.bind(this)); - this._executeIdleCallback = this._executeIdleCallback.bind(this); this._idlePromise = Promise.resolve(); - this._onIdleCallbacks = []; + this._idleCallbackRequests = []; this._activeArtifacts = []; this._artifactPlugins = []; this._pathBuilder = pathBuilder || new ArtifactPathBuilder({ artifactsRootDir: argparse.getArgValue('artifacts-location') || 'artifacts', }); - this._callersMap = new WeakMap(); + } + + _instantitateArtifactPlugin(pluginFactory) { + const artifactsApi = { + plugin: { name: '(unknown plugin)' }, - this.artifactsApi = { preparePathForArtifact: async (artifactName, testSummary) => { const artifactPath = this._pathBuilder.buildPathForTestArtifact(artifactName, testSummary); const artifactDir = path.dirname(artifactPath); @@ -41,41 +40,47 @@ class ArtifactsManager { _.pull(this._activeArtifacts, artifact); }, - requestIdleCallback: (callback, caller) => { - if (caller) { - this._callersMap.set(callback, caller); - } - - this._onIdleCallbacks.push(callback); + requestIdleCallback: (callback) => { + this._idleCallbackRequests.push({ + caller: artifactsApi.plugin, + callback, + }); this._idlePromise = this._idlePromise.then(() => { - const nextCallback = this._onIdleCallbacks.shift(); - return this._executeIdleCallback(nextCallback); + const nextCallbackRequest = this._idleCallbackRequests.shift(); + + if (nextCallbackRequest) { + return this._executeIdleCallbackRequest(nextCallbackRequest); + } }); }, }; + + const plugin = pluginFactory(artifactsApi); + artifactsApi.plugin = plugin; + + return plugin; } - _executeIdleCallback(callback) { - if (callback) { - return Promise.resolve() - .then(callback) - .catch(e => this._idleCallbackErrorHandle(e, callback)); - } + _executeIdleCallbackRequest({ callback, caller }) { + return Promise.resolve() + .then(callback) + .catch(e => this._idleCallbackErrorHandle(e, caller)); } registerArtifactPlugins(artifactPluginFactoriesMap = {}) { - const api = this.artifactsApi; const artifactPluginsFactories = Object.values(artifactPluginFactoriesMap); - this._artifactPlugins = artifactPluginsFactories.map(factory => factory(api)); + this._artifactPlugins = artifactPluginsFactories.map((factory) => { + return this._instantitateArtifactPlugin(factory); + }); } subscribeToDeviceEvents(deviceEmitter) { - deviceEmitter.on('bootDevice', this.onBootDevice); - deviceEmitter.on('shutdownDevice', this.onShutdownDevice); - deviceEmitter.on('beforeLaunchApp', this.onBeforeLaunchApp); - deviceEmitter.on('launchApp', this.onLaunchApp); + deviceEmitter.on('bootDevice', this.onBootDevice.bind(this)); + deviceEmitter.on('shutdownDevice', this.onShutdownDevice.bind(this)); + deviceEmitter.on('beforeLaunchApp', this.onBeforeLaunchApp.bind(this)); + deviceEmitter.on('launchApp', this.onLaunchApp.bind(this)); } async onBootDevice(deviceInfo) { @@ -119,7 +124,9 @@ class ArtifactsManager { log.info({ event: 'TERMINATE_START' }, 'finalizing the recorded artifacts, this can take some time...'); await this._callPlugins('onTerminate'); - await Promise.all(this._onIdleCallbacks.splice(0).map(this._executeIdleCallback)); + + const allCallbackRequests = this._idleCallbackRequests.splice(0); + await Promise.all(allCallbackRequests.map(this._executeIdleCallbackRequest.bind(this))); await this._idlePromise; await Promise.all(this._activeArtifacts.map(artifact => artifact.discard())); @@ -130,7 +137,8 @@ class ArtifactsManager { } async _callPlugins(methodName, ...args) { - log.trace(Object.assign({ event: 'LIFECYCLE', fn: methodName }, ...args), `${methodName}`); + const callSignature = this._composeCallSignature('artifactsManager', methodName, args); + log.trace(Object.assign({ event: 'LIFECYCLE', fn: methodName }, ...args), callSignature); await Promise.all(this._artifactPlugins.map(async (plugin) => { try { @@ -141,14 +149,24 @@ class ArtifactsManager { })); } - _unhandledPluginExceptionHandler(err, { plugin, methodName }) { - const eventObject = { event: 'PLUGIN_ERROR', plugin: plugin.name || 'unknown', methodName, err }; - log.error(eventObject, `Caught exception inside plugin (${eventObject.plugin}) at phase ${methodName}`); + _composeCallSignature(object, methodName, args) { + const argsString = args.map(arg => util.inspect(arg)).join(', '); + return `${object}.${methodName}(${argsString})`; } - _idleCallbackErrorHandle(err, callback) { - const caller = this._callersMap.get(callback) || {}; + _unhandledPluginExceptionHandler(err, { plugin, methodName, args }) { + const logObject = { + event: 'PLUGIN_ERROR', + plugin: plugin.name, + err, + methodName, + }; + + const callSignature = this._composeCallSignature(plugin.name, methodName, args); + log.error(logObject, `Caught exception inside function call: ${callSignature}`); + } + _idleCallbackErrorHandle(err, caller) { this._unhandledPluginExceptionHandler(err, { plugin: caller, methodName: 'onIdleCallback', @@ -158,4 +176,4 @@ class ArtifactsManager { } -module.exports = ArtifactsManager; \ No newline at end of file +module.exports = ArtifactsManager; diff --git a/detox/src/artifacts/ArtifactsManager.test.js b/detox/src/artifacts/ArtifactsManager.test.js index f1fb85a2d0..a26e95840d 100644 --- a/detox/src/artifacts/ArtifactsManager.test.js +++ b/detox/src/artifacts/ArtifactsManager.test.js @@ -202,16 +202,6 @@ describe('ArtifactsManager', () => { expect(callbacks[1]).toHaveBeenCalled(); }); - it('should gracefully handle a case when plugin object is not passed and use "unknown" as a name placeholder', async () => { - artifactsApi.requestIdleCallback(callbacks[0]); await sleep(0); - await rejects[0](new Error('test onIdleCallback error')); - - expect(proxy.logger.error.mock.calls).toMatchSnapshot(); - - artifactsApi.requestIdleCallback(callbacks[1], testPlugin); await sleep(0); - expect(callbacks[1]).toHaveBeenCalled(); - }); - it('should work correctly even when operations are flushed on Detox termination', async () => { artifactsApi.requestIdleCallback(callbacks[0], testPlugin); artifactsApi.requestIdleCallback(callbacks[1], testPlugin); diff --git a/detox/src/artifacts/__snapshots__/ArtifactsManager.test.js.snap b/detox/src/artifacts/__snapshots__/ArtifactsManager.test.js.snap index 7e3e60b32e..8d5d59c160 100644 --- a/detox/src/artifacts/__snapshots__/ArtifactsManager.test.js.snap +++ b/detox/src/artifacts/__snapshots__/ArtifactsManager.test.js.snap @@ -9,21 +9,7 @@ Array [ "methodName": "onIdleCallback", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onIdleCallback", - ], -] -`; - -exports[`ArtifactsManager .artifactsApi .requestIdleCallback() should gracefully handle a case when plugin object is not passed and use "unknown" as a name placeholder 1`] = ` -Array [ - Array [ - Object { - "err": [Error: test onIdleCallback error], - "event": "PLUGIN_ERROR", - "methodName": "onIdleCallback", - "plugin": "unknown", - }, - "Caught exception inside plugin (unknown) at phase onIdleCallback", + "Caught exception inside function call: testPlugin.onIdleCallback()", ], ] `; @@ -37,7 +23,7 @@ Array [ "methodName": "onAfterAll", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onAfterAll", + "Caught exception inside function call: testPlugin.onAfterAll()", ], ] `; @@ -51,7 +37,7 @@ Array [ "methodName": "onAfterEach", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onAfterEach", + "Caught exception inside function call: testPlugin.onAfterEach({ title: 'test', fullName: 'Suite test', status: 'passed' })", ], ] `; @@ -65,7 +51,7 @@ Array [ "methodName": "onBeforeAll", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onBeforeAll", + "Caught exception inside function call: testPlugin.onBeforeAll()", ], ] `; @@ -79,7 +65,7 @@ Array [ "methodName": "onBeforeEach", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onBeforeEach", + "Caught exception inside function call: testPlugin.onBeforeEach({ title: 'test', fullName: 'Suite test', status: 'running' })", ], ] `; @@ -93,7 +79,7 @@ Array [ "methodName": "onBeforeLaunchApp", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onBeforeLaunchApp", + "Caught exception inside function call: testPlugin.onBeforeLaunchApp({ bundleId: 'testBundleId', deviceId: 'testDeviceId' })", ], ] `; @@ -107,7 +93,7 @@ Array [ "methodName": "onBootDevice", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onBootDevice", + "Caught exception inside function call: testPlugin.onBootDevice({ coldBoot: false, deviceId: 'testDeviceId' })", ], ] `; @@ -121,7 +107,7 @@ Array [ "methodName": "onLaunchApp", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onLaunchApp", + "Caught exception inside function call: testPlugin.onLaunchApp({ bundleId: 'testBundleId', deviceId: 'testDeviceId', pid: 2018 })", ], ] `; @@ -135,7 +121,7 @@ Array [ "methodName": "onShutdownDevice", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onShutdownDevice", + "Caught exception inside function call: testPlugin.onShutdownDevice({ deviceId: 'testDeviceId' })", ], ] `; @@ -149,7 +135,7 @@ Array [ "methodName": "onTerminate", "plugin": "testPlugin", }, - "Caught exception inside plugin (testPlugin) at phase onTerminate", + "Caught exception inside function call: testPlugin.onTerminate()", ], ] `; diff --git a/detox/src/artifacts/log/android/ADBLogcatPlugin.js b/detox/src/artifacts/log/android/ADBLogcatPlugin.js index 292dc19d76..5c96dee9f4 100644 --- a/detox/src/artifacts/log/android/ADBLogcatPlugin.js +++ b/detox/src/artifacts/log/android/ADBLogcatPlugin.js @@ -10,7 +10,7 @@ class ADBLogcatPlugin extends LogArtifactPlugin { } async onLaunchApp(event) { - super.onLaunchApp(event); + await super.onLaunchApp(event); if (this.currentRecording) { await this.currentRecording.start({ pid: event.pid }); diff --git a/detox/src/artifacts/screenshot/SimulatorScreenshotPlugin.js b/detox/src/artifacts/screenshot/SimulatorScreenshotPlugin.js index 5e16a8595e..d6b876af0d 100644 --- a/detox/src/artifacts/screenshot/SimulatorScreenshotPlugin.js +++ b/detox/src/artifacts/screenshot/SimulatorScreenshotPlugin.js @@ -13,7 +13,7 @@ class SimulatorScreenshotter extends ScreenshotArtifactPlugin { } async onBootDevice(event) { - super.onBootDevice(event); + await super.onBootDevice(event); if (this.enabled && event.coldBoot) { await this.appleSimUtils.takeScreenshot(event.deviceId, '/dev/null').catch(() => { diff --git a/detox/src/artifacts/templates/plugin/StartupAndTestRecorderPlugin.js b/detox/src/artifacts/templates/plugin/StartupAndTestRecorderPlugin.js index 7e1906cc52..292b7b321e 100644 --- a/detox/src/artifacts/templates/plugin/StartupAndTestRecorderPlugin.js +++ b/detox/src/artifacts/templates/plugin/StartupAndTestRecorderPlugin.js @@ -102,14 +102,14 @@ class StartupAndTestRecorderPlugin extends WholeTestRecorderPlugin { const artifactPath = await this.preparePathForStartupArtifact(); await startupRecording.save(artifactPath); this.api.untrackArtifact(startupRecording); - }, this); + }); } _startDiscardingStartupRecording(startupRecording) { this.api.requestIdleCallback(async () => { await startupRecording.discard(); this.api.untrackArtifact(startupRecording); - }, this); + }); } } diff --git a/detox/src/artifacts/templates/plugin/StartupAndTestRecorderPlugin.test.js b/detox/src/artifacts/templates/plugin/StartupAndTestRecorderPlugin.test.js index e7aed59a48..c29bed7446 100644 --- a/detox/src/artifacts/templates/plugin/StartupAndTestRecorderPlugin.test.js +++ b/detox/src/artifacts/templates/plugin/StartupAndTestRecorderPlugin.test.js @@ -187,7 +187,7 @@ describe('StartupAndTestRecorderPlugin', () => { it('should schedule saving of the start-up recording', () => { expect(api.requestIdleCallback).toHaveBeenCalledTimes(1); - expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function), plugin]); + expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function)]); }); it('should reset .startupRecording property to null', async () => { @@ -269,8 +269,8 @@ describe('StartupAndTestRecorderPlugin', () => { it('should schedule two operations', () => { expect(api.requestIdleCallback).toHaveBeenCalledTimes(2); - expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function), plugin]); - expect(api.requestIdleCallback.mock.calls[1]).toEqual([expect.any(Function), plugin]); + expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function)]); + expect(api.requestIdleCallback.mock.calls[1]).toEqual([expect.any(Function)]); }); it('should schedule saving of the test recording', async () => { @@ -304,7 +304,7 @@ describe('StartupAndTestRecorderPlugin', () => { function itShouldScheduleDiscardingAndUntrackingOfStartupArtifact() { it('should schedule discarding of the start-up recording', () => { expect(api.requestIdleCallback).toHaveBeenCalledTimes(1); - expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function), plugin]); + expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function)]); }); it('should reset .startupRecording property to null', async () => { diff --git a/detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.js b/detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.js index 0f14c3c59f..bc57a10c3a 100644 --- a/detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.js +++ b/detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.js @@ -63,7 +63,7 @@ class TwoSnapshotsPerTestPlugin extends ArtifactPlugin { const snapshotArtifactPath = await this.preparePathForSnapshot(testSummary, index); await snapshot.save(snapshotArtifactPath); this.api.untrackArtifact(snapshot); - }, this); + }); } _startDiscardingSnapshot(index) { @@ -75,7 +75,7 @@ class TwoSnapshotsPerTestPlugin extends ArtifactPlugin { this.api.requestIdleCallback(async () => { await snapshot.discard(); this.api.untrackArtifact(snapshot); - }, this); + }); } _clearSnapshotReferences() { diff --git a/detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.test.js b/detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.test.js index 8282c176df..240cec8f2a 100644 --- a/detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.test.js +++ b/detox/src/artifacts/templates/plugin/TwoSnapshotsPerTestPlugin.test.js @@ -98,8 +98,8 @@ describe('TwoSnapshotsPerTestPlugin', () => { it('should schedule two saving operations and specify itself as an initiator', () => { expect(api.requestIdleCallback).toHaveBeenCalledTimes(2); - expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function), plugin]); - expect(api.requestIdleCallback.mock.calls[1]).toEqual([expect.any(Function), plugin]); + expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function)]); + expect(api.requestIdleCallback.mock.calls[1]).toEqual([expect.any(Function)]); }); it('should schedule to save and untrack the first artifact', async () => { @@ -143,7 +143,7 @@ describe('TwoSnapshotsPerTestPlugin', () => { it('should schedule a discard operation for the first artifact and specify itself as an initiator', () => { expect(api.requestIdleCallback).toHaveBeenCalledTimes(1); - expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function), plugin]); + expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function)]); }); it('should ultimately discard and untrack the first artifact', async () => { diff --git a/detox/src/artifacts/templates/plugin/WholeTestRecorderPlugin.js b/detox/src/artifacts/templates/plugin/WholeTestRecorderPlugin.js index fd2cb0133f..ade83aa59c 100644 --- a/detox/src/artifacts/templates/plugin/WholeTestRecorderPlugin.js +++ b/detox/src/artifacts/templates/plugin/WholeTestRecorderPlugin.js @@ -54,14 +54,14 @@ class WholeTestRecorderPlugin extends ArtifactPlugin { const recordingArtifactPath = await this.preparePathForTestArtifact(testSummary); await testRecording.save(recordingArtifactPath); this.api.untrackArtifact(testRecording); - }, this); + }); } _startDiscardingTestRecording(testRecording) { this.api.requestIdleCallback(async () => { await testRecording.discard(); this.api.untrackArtifact(testRecording); - }, this); + }); } } diff --git a/detox/src/artifacts/templates/plugin/WholeTestRecorderPlugin.test.js b/detox/src/artifacts/templates/plugin/WholeTestRecorderPlugin.test.js index 43e5c87109..eaaa56152e 100644 --- a/detox/src/artifacts/templates/plugin/WholeTestRecorderPlugin.test.js +++ b/detox/src/artifacts/templates/plugin/WholeTestRecorderPlugin.test.js @@ -68,7 +68,7 @@ describe('WholeTestRecorderPlugin', () => { it('should schedule a save operation and specify itself as an initiator', () => { expect(api.requestIdleCallback).toHaveBeenCalledTimes(1); - expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function), plugin]); + expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function)]); }); it('should ultimately save the artifact and untrack it', async () => { @@ -100,7 +100,7 @@ describe('WholeTestRecorderPlugin', () => { it('should schedule a discard operation and specify itself as an initiator', () => { expect(api.requestIdleCallback).toHaveBeenCalledTimes(1); - expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function), plugin]); + expect(api.requestIdleCallback.mock.calls[0]).toEqual([expect.any(Function)]); }); it('should ultimately discard the artifact and untrack it', async () => {