Skip to content

Commit

Permalink
More helpful error message if Detox.init() fails (wix#1705)
Browse files Browse the repository at this point in the history
* feat(ux): clearer error message if Detox.init() fails

* fix: global scope and unit tests

* fix: coverage and global vars
  • Loading branch information
noomorph authored and rotemmiz committed Jan 21, 2020
1 parent 3df0f95 commit 499e3aa
Show file tree
Hide file tree
Showing 6 changed files with 133 additions and 16 deletions.
1 change: 1 addition & 0 deletions detox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"src/devices/ios/AppleSimUtils.js",
"src/server/DetoxServer.js",
".*Driver.js",
"MissingDetox.js",
"EmulatorTelnet.js",
"Emulator.js",
"DeviceDriverBase.js",
Expand Down
8 changes: 7 additions & 1 deletion detox/runners/jest/WorkerAssignReporterImpl.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const _ = require('lodash');
const chalk = require('chalk').default;
const ReporterBase = require('./ReporterBase');
const log = require('../../src/utils/logger').child();
Expand All @@ -9,7 +10,12 @@ class WorkerAssignReporterImpl extends ReporterBase {
}

report(workerName) {
log.info({event: 'WORKER_ASSIGN'}, `${chalk.whiteBright(workerName)} assigned to ${chalk.blueBright(this.device.name)}`);
const deviceName = _.attempt(() => this.device.name);
const formattedDeviceName = _.isError(deviceName)
? chalk.redBright('undefined')
: chalk.blueBright(deviceName);

log.info({event: 'WORKER_ASSIGN'}, `${chalk.whiteBright(workerName)} is assigned to ${formattedDeviceName}`);
}
}

Expand Down
3 changes: 3 additions & 0 deletions detox/src/Detox.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const EmulatorDriver = require('./devices/drivers/EmulatorDriver');
const AttachedAndroidDriver = require('./devices/drivers/AttachedAndroidDriver');
const DetoxRuntimeError = require('./errors/DetoxRuntimeError');
const argparse = require('./utils/argparse');
const MissingDetox = require('./utils/MissingDetox');
const configuration = require('./configuration');
const Client = require('./client/Client');
const DetoxServer = require('./server/DetoxServer');
Expand Down Expand Up @@ -177,4 +178,6 @@ class Detox {
}
}

Detox.none = new MissingDetox();

module.exports = Detox;
21 changes: 14 additions & 7 deletions detox/src/DetoxExportWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const _detox = Symbol('detox');

class DetoxExportWrapper {
constructor() {
this[_detox] = null;
this[_detox] = Detox.none;

this.init = this.init.bind(this);
this.cleanup = this.cleanup.bind(this);
Expand All @@ -29,33 +29,39 @@ class DetoxExportWrapper {
}

async init(config, params) {
if (!params || params.initGlobals !== false) {
Detox.none.initContext(global);
}

this[_detox] = await DetoxExportWrapper._initializeInstance(config, params);
return this[_detox];
}

async cleanup() {
if (this[_detox]) {
Detox.none.cleanupContext(global);

if (this[_detox] !== Detox.none) {
await this[_detox].cleanup();
this[_detox] = null;
this[_detox] = Detox.none;
}
}

_definePassthroughMethod(name) {
this[name] = (...args) => {
if (this[_detox]) {
return this[_detox][name](...args);
}
return this[_detox][name](...args);
};
}

_defineProxy(name) {
this[name] = funpermaproxy(() => (this[_detox] && this[_detox][name]));
this[name] = funpermaproxy(() => this[_detox][name]);
}

static async _initializeInstance(detoxConfig, params) {
let instance = null;

try {
Detox.none.setError(null);

if (!detoxConfig) {
throw new Error(`No configuration was passed to detox, make sure you pass a detoxConfig when calling 'detox.init(detoxConfig)'`);
}
Expand All @@ -81,6 +87,7 @@ class DetoxExportWrapper {
await instance.init(params);
return instance;
} catch (err) {
Detox.none.setError(err);
log.error({ event: 'DETOX_INIT_ERROR' }, '\n', err);

if (instance) {
Expand Down
39 changes: 31 additions & 8 deletions detox/src/index.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const schemes = require('./configurations.mock');

describe('index', () => {
let detox;
let mockDevice;
Expand All @@ -24,7 +25,13 @@ describe('index', () => {
.mock('./devices/Device')
.mock('./utils/logger')
.mock('./client/Client')
.mock('./Detox', () => jest.fn(() => mockDetox))
.mock('./Detox', () => {
const MissingDetox = require('./utils/MissingDetox');
const none = new MissingDetox();
none.cleanupContext = () => {}; // avoid ruining global state

return Object.assign(jest.fn(() => mockDetox), { none });
});

process.env.DETOX_UNIT_TEST = true;
detox = require('./index');
Expand All @@ -39,6 +46,14 @@ describe('index', () => {
.unmock('./Detox')
});

it(`constructs detox without globals if initGlobals = false`, async () => {
const Detox = require('./Detox');

await detox.init(schemes.validOneDeviceNoSession, { initGlobals: false });

expect('by' in global).toBe(false);
});

it(`throws if there was no config passed`, async () => {
const logger = require('./utils/logger');
let exception = undefined;
Expand Down Expand Up @@ -185,8 +200,9 @@ describe('index', () => {
it(`Basic usage with memorized exported objects`, async() => {
const { device, by } = detox;

expect(device.launchApp).toBe(undefined);
expect(by.id).toBe(undefined);
expect(() => { device, by }).not.toThrowError();
expect(() => { device.launchApp }).toThrowError();
expect(() => { by.id }).toThrowError();

await detox.init(schemes.validOneDeviceNoSession);

Expand All @@ -195,8 +211,9 @@ describe('index', () => {

await detox.cleanup();

expect(device.launchApp).toBe(undefined);
expect(by.id).toBe(undefined);
expect(() => { device, by }).not.toThrowError();
expect(() => { device.launchApp }).toThrowError();
expect(() => { by.id }).toThrowError();
});

it(`Basic usage, do not throw an error if cleanup is done twice`, async() => {
Expand All @@ -221,7 +238,14 @@ describe('index', () => {
});

it(`beforeEach() should be covered - with detox not initialized`, async() => {
await detox.beforeEach();
await expect(detox.beforeEach()).rejects.toThrow(/Make sure to call/);
});

it(`error message should be covered - with detox failed to initialize`, async() => {
mockDetox.init.mockReturnValue(Promise.reject(new Error('Test error')));

await expect(detox.init(schemes.validOneDeviceNoSession)).rejects.toThrow(/Test error/);
await expect(detox.beforeEach()).rejects.toThrow(/There was an error/);
});

it(`afterEach() should be covered - with detox initialized`, async() => {
Expand All @@ -230,7 +254,6 @@ describe('index', () => {
});

it(`afterEach() should be covered - with detox not initialized`, async() => {
await detox.afterEach();
await expect(detox.afterEach()).rejects.toThrow(/Make sure to call/);
});

});
77 changes: 77 additions & 0 deletions detox/src/utils/MissingDetox.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
const DetoxRuntimeError = require('../errors/DetoxRuntimeError');

class MissingDetox {
constructor() {
this.throwError = this.throwError.bind(this);
this.initContext(this);

this._defineRequiredProperty(this, 'beforeEach', async () => this.throwError(), true);
this._defineRequiredProperty(this, 'afterEach', async () => this.throwError(), true);
}

initContext(context) {
const readonly = context === this;

this._defineRequiredProperty(context, 'by', undefined, readonly);
this._defineRequiredProperty(context, 'device', undefined, readonly);
this._defineRequiredProperty(context, 'element', this.throwError, readonly);
this._defineRequiredProperty(context, 'expect', this.throwError, readonly);
this._defineRequiredProperty(context, 'waitFor', this.throwError, readonly);
}

cleanupContext(context) {
this._cleanupProperty(context, 'by');
this._cleanupProperty(context, 'device');
this._cleanupProperty(context, 'element');
this._cleanupProperty(context, 'expect');
this._cleanupProperty(context, 'waitFor');
}

_cleanupProperty(context, name) {
if (context.hasOwnProperty(name)) {
context[name] = undefined;
}
}

_defineRequiredProperty(context, name, initialValue, readonly) {
if (context.hasOwnProperty(name)) {
return;
}

let _value = initialValue;

const descriptor = {
get: () => {
if (_value === undefined) {
this.throwError();
}

return _value;
},
};

if (!readonly) {
descriptor.set = (value) => {
_value = value;
};
}

Object.defineProperty(context, name, descriptor);
}

setError(err) {
this._lastError = err;
}

throwError() {
throw new DetoxRuntimeError({
message: 'Detox instance has not been initialized',
hint: this._lastError
? 'There was an error on attempt to call detox.init()'
: 'Make sure to call detox.init() before your test begins',
debugInfo: this._lastError && this._lastError.stack || '',
});
}
}

module.exports = MissingDetox;

0 comments on commit 499e3aa

Please sign in to comment.