Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: cross-testrunner message formatting for DetoxRuntimeError #861

Merged
merged 5 commits into from
Jul 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion detox/src/Detox.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const _ = require('lodash');
const util = require('util');
const logger = require('./utils/logger');
const log = require('./utils/logger').child({ __filename });
const Device = require('./devices/Device');
Expand Down Expand Up @@ -121,7 +122,7 @@ class Detox {
hint: 'Maybe you are still using an old undocumented signature detox.beforeEach(string, string, string) in init.js ?' +
'\nSee the article for the guidance: ' +
'https://github.com/wix/detox/blob/master/docs/APIRef.TestLifecycle.md',
debugInfo: `testSummary was: ${JSON.stringify(testSummary, null, 2)}`,
debugInfo: `testSummary was: ${util.inspect(testSummary)}`,
});
}

Expand Down
4 changes: 2 additions & 2 deletions detox/src/__snapshots__/Detox.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Expected to get an object of type: { title: string; fullName: string; status: \\
HINT: Maybe you are still using an old undocumented signature detox.beforeEach(string, string, string) in init.js ?
See the article for the guidance: https://github.com/wix/detox/blob/master/docs/APIRef.TestLifecycle.md

testSummary was: \\"test\\""
testSummary was: 'test'"
`;

exports[`Detox .artifactsManager Calling .beforeEach() and .afterEach() with a deprecated signature will throw an exception 2`] = `
Expand All @@ -17,7 +17,7 @@ Expected to get an object of type: { title: string; fullName: string; status: \\
HINT: Maybe you are still using an old undocumented signature detox.beforeEach(string, string, string) in init.js ?
See the article for the guidance: https://github.com/wix/detox/blob/master/docs/APIRef.TestLifecycle.md

testSummary was: \\"test\\""
testSummary was: 'test'"
`;

exports[`Detox .artifactsManager Calling .beforeEach() and .afterEach() with incorrect test status will throw an exception 1`] = `
Expand Down
10 changes: 5 additions & 5 deletions detox/src/devices/Device.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
const _ = require('lodash');
const fs = require('fs');
const path = require('path');
const _ = require('lodash');
const log = require('../utils/logger').child({ __filename });
const argparse = require('../utils/argparse');
const debug = require('../utils/debug'); //debug utils, leave here even if unused
const logError = require('../utils/logError');

class Device {

Expand All @@ -26,9 +25,10 @@ class Device {
async emit(eventName, eventObj) {
const fire = async (fn) => fn(eventObj);
const logEmitError = (err) => {
const errorLogger = log.child({ event: 'DEVICE_EMIT_EVENT_ERROR', eventName });
errorLogger.error(`Caught an exception in: device.emit("${eventName}", ${JSON.stringify(eventObj)})`);
logError(errorLogger, err);
log.error(
{ event: 'DEVICE_EMIT_EVENT_ERROR', eventName },
`Caught an exception in: device.emit("${eventName}", ${JSON.stringify(eventObj)})\n\n`, err
);
};

await Promise.all(this._listeners[eventName].map(fn => fire(fn).catch(logEmitError)));
Expand Down
21 changes: 10 additions & 11 deletions detox/src/devices/Device.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ describe('Device', () => {
let Client;
let client;
let logger;
let logError;

beforeEach(async () => {
jest.mock('fs');
Expand All @@ -27,9 +26,6 @@ describe('Device', () => {

Device = require('./Device');

jest.mock('../utils/logError');
logError = require('../utils/logError');

jest.mock('../utils/sh');
sh = require('../utils/sh');
sh.cp = jest.fn();
Expand Down Expand Up @@ -145,15 +141,18 @@ describe('Device', () => {

await device.launchApp();

const eventObject = {
deviceId: device._deviceId,
bundleId: device._bundleId,
pid: 2,
const extraLoggerFields = {
event: 'DEVICE_EMIT_EVENT_ERROR',
eventName: 'launchApp',
};

expect(logger.error).toHaveBeenCalledWith(`Caught an exception in: device.emit("launchApp", {"pid":2})`);
expect(logError).toHaveBeenCalledWith(logger, errorSync);
expect(logError).toHaveBeenCalledWith(logger, errorAsync);
expect(logger.error).toHaveBeenCalledWith(
extraLoggerFields, `Caught an exception in: device.emit("launchApp", {"pid":2})\n\n`, errorSync
);

expect(logger.error).toHaveBeenCalledWith(
extraLoggerFields, `Caught an exception in: device.emit("launchApp", {"pid":2})\n\n`, errorAsync
);
});

it(`relaunchApp()`, async () => {
Expand Down
16 changes: 4 additions & 12 deletions detox/src/errors/DetoxRuntimeError.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
class DetoxRuntimeError extends Error {
constructor({ message = '', hint = '', debugInfo = '' } = {}) {
super(message);
super(`DetoxRuntimeError: ${message}` +
(!hint ? '' : '\n\nHINT: ' + hint) +
(!debugInfo ? '' : '\n\n' + debugInfo));

Error.captureStackTrace(this, DetoxRuntimeError);
this.hint = hint;
this.debugInfo = debugInfo;

if (DetoxRuntimeError.formatMessagesOnFly) {
this.message = this.toString();
}
}

toString() {
return `${this.constructor.name}: ${this.message}` +
(!this.hint ? '' : '\n\nHINT: ' + this.hint) +
(!this.debugInfo ? '' : '\n\n' + this.debugInfo);
return super.toString().replace(/^Error: /, '\n');
}
}

DetoxRuntimeError.formatMessagesOnFly = typeof jest !== 'undefined';

module.exports = DetoxRuntimeError;
52 changes: 30 additions & 22 deletions detox/src/errors/DetoxRuntimeError.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,42 @@ const _ = require('lodash');
const DetoxRuntimeError = require('./DetoxRuntimeError');

describe(DetoxRuntimeError, () => {
afterAll(() => {
DetoxRuntimeError.formatMessagesOnFly = true;
});

it('should have various ways to create it', () => {
DetoxRuntimeError.formatMessagesOnFly = false;

expect(_.mapValues(varietiesOfInstantiation(), e => ({
message: e.message,
hint: e.hint,
debugInfo: e.debugInfo,
}))).toMatchSnapshot();
});

it('should format its fields to a single message if needed', () => {
DetoxRuntimeError.formatMessagesOnFly = true;

expect(_.mapValues(varietiesOfInstantiation(), 'message')).toMatchSnapshot();
it('should format all fields to a single message', () => {
_.forEach(varietiesOfInstantiation(), (error, description) => {
expect(error.toString()).toMatchSnapshot(`.toString() - ${description}`);
expect(() => { throw error; }).toThrowErrorMatchingSnapshot(`throw error - ${description}`);
});
});

function varietiesOfInstantiation() {
return {
'no args': new DetoxRuntimeError(),
'empty object': new DetoxRuntimeError({}),
'only message': new DetoxRuntimeError({ message: 'message' }),
'message with hint': new DetoxRuntimeError({ message: 'message', hint: 'hint' }),
'message with debug info': new DetoxRuntimeError({ message: 'message', debugInfo: 'debugInfo' }),
'message with hint and debug info': new DetoxRuntimeError({ message: 'message', hint: 'hint', debugInfo: 'debugInfo' }),
'only message': new DetoxRuntimeError({
message: `The video is not being recorded on device (${'emulator-5554'}) at path: ${'/sdcard/712398.mp4'}`,
}),
'message with hint': new DetoxRuntimeError({
message: 'Detox adapter to Jest is malfunctioning.',
hint: `Make sure you register it as Jasmine reporter inside init.js:\n` +
`-------------------------------------------------------------\n` +
'jasmine.getEnv().addReporter(adapter);',
}),
'message with debug info': new DetoxRuntimeError({
message: 'no filename was given to constructSafeFilename()',
debugInfo: 'the arguments were: ' + JSON.stringify({
prefix: 'detox - ',
trimmable: undefined,
suffix: undefined,
}, null, 2),
}),
'message with hint and debug info': new DetoxRuntimeError({
message: `Invalid test summary was passed to detox.beforeEach(testSummary)` +
'\nExpected to get an object of type: { title: string; fullName: string; status: "running" | "passed" | "failed"; }',
hint: 'Maybe you are still using an old undocumented signature detox.beforeEach(string, string, string) in init.js ?' +
'\nSee the article for the guidance: ' +
'https://github.com/wix/detox/blob/master/docs/APIRef.TestLifecycle.md',
debugInfo: `testSummary was: ${JSON.stringify('test name')}`,
}),
};
}
});
126 changes: 74 additions & 52 deletions detox/src/errors/__snapshots__/DetoxRuntimeError.test.js.snap
Original file line number Diff line number Diff line change
@@ -1,55 +1,77 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`DetoxRuntimeError should format its fields to a single message if needed 1`] = `
Object {
"empty object": "DetoxRuntimeError: ",
"message with debug info": "DetoxRuntimeError: message

debugInfo",
"message with hint": "DetoxRuntimeError: message

HINT: hint",
"message with hint and debug info": "DetoxRuntimeError: message

HINT: hint

debugInfo",
"no args": "DetoxRuntimeError: ",
"only message": "DetoxRuntimeError: message",
}
`;

exports[`DetoxRuntimeError should have various ways to create it 1`] = `
Object {
"empty object": Object {
"debugInfo": "",
"hint": "",
"message": "",
},
"message with debug info": Object {
"debugInfo": "debugInfo",
"hint": "",
"message": "message",
},
"message with hint": Object {
"debugInfo": "",
"hint": "hint",
"message": "message",
},
"message with hint and debug info": Object {
"debugInfo": "debugInfo",
"hint": "hint",
"message": "message",
},
"no args": Object {
"debugInfo": "",
"hint": "",
"message": "",
},
"only message": Object {
"debugInfo": "",
"hint": "",
"message": "message",
},
}
exports[`DetoxRuntimeError should format all fields to a single message: .toString() - empty object 1`] = `
"
DetoxRuntimeError: "
`;

exports[`DetoxRuntimeError should format all fields to a single message: .toString() - message with debug info 1`] = `
"
DetoxRuntimeError: no filename was given to constructSafeFilename()

the arguments were: {
\\"prefix\\": \\"detox - \\"
}"
`;

exports[`DetoxRuntimeError should format all fields to a single message: .toString() - message with hint 1`] = `
"
DetoxRuntimeError: Detox adapter to Jest is malfunctioning.

HINT: Make sure you register it as Jasmine reporter inside init.js:
-------------------------------------------------------------
jasmine.getEnv().addReporter(adapter);"
`;

exports[`DetoxRuntimeError should format all fields to a single message: .toString() - message with hint and debug info 1`] = `
"
DetoxRuntimeError: Invalid test summary was passed to detox.beforeEach(testSummary)
Expected to get an object of type: { title: string; fullName: string; status: \\"running\\" | \\"passed\\" | \\"failed\\"; }

HINT: Maybe you are still using an old undocumented signature detox.beforeEach(string, string, string) in init.js ?
See the article for the guidance: https://github.com/wix/detox/blob/master/docs/APIRef.TestLifecycle.md

testSummary was: \\"test name\\""
`;

exports[`DetoxRuntimeError should format all fields to a single message: .toString() - no args 1`] = `
"
DetoxRuntimeError: "
`;

exports[`DetoxRuntimeError should format all fields to a single message: .toString() - only message 1`] = `
"
DetoxRuntimeError: The video is not being recorded on device (emulator-5554) at path: /sdcard/712398.mp4"
`;

exports[`DetoxRuntimeError should format all fields to a single message: throw error - empty object 1`] = `"DetoxRuntimeError: "`;

exports[`DetoxRuntimeError should format all fields to a single message: throw error - message with debug info 1`] = `
"DetoxRuntimeError: no filename was given to constructSafeFilename()

the arguments were: {
\\"prefix\\": \\"detox - \\"
}"
`;

exports[`DetoxRuntimeError should format all fields to a single message: throw error - message with hint 1`] = `
"DetoxRuntimeError: Detox adapter to Jest is malfunctioning.

HINT: Make sure you register it as Jasmine reporter inside init.js:
-------------------------------------------------------------
jasmine.getEnv().addReporter(adapter);"
`;

exports[`DetoxRuntimeError should format all fields to a single message: throw error - message with hint and debug info 1`] = `
"DetoxRuntimeError: Invalid test summary was passed to detox.beforeEach(testSummary)
Expected to get an object of type: { title: string; fullName: string; status: \\"running\\" | \\"passed\\" | \\"failed\\"; }

HINT: Maybe you are still using an old undocumented signature detox.beforeEach(string, string, string) in init.js ?
See the article for the guidance: https://github.com/wix/detox/blob/master/docs/APIRef.TestLifecycle.md

testSummary was: \\"test name\\""
`;

exports[`DetoxRuntimeError should format all fields to a single message: throw error - no args 1`] = `"DetoxRuntimeError: "`;

exports[`DetoxRuntimeError should format all fields to a single message: throw error - only message 1`] = `"DetoxRuntimeError: The video is not being recorded on device (emulator-5554) at path: /sdcard/712398.mp4"`;
3 changes: 1 addition & 2 deletions detox/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const platform = require('./platform');
const exportWrapper = require('./exportWrapper');
const argparse = require('./utils/argparse');
const log = require('./utils/logger').child({ __filename });
const logError = require('./utils/logError');
const onTerminate = require('./utils/onTerminate');
const configuration = require('./configuration');

Expand Down Expand Up @@ -54,7 +53,7 @@ async function init(config, params) {
try {
await initializeDetox(config, params);
} catch (err) {
logError(log, err);
log.error({ event: 'DETOX_INIT_ERROR' }, '\n', err);
await cleanup();

detox = null;
Expand Down
6 changes: 3 additions & 3 deletions detox/src/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('index', () => {
.mock('./server/DetoxServer')
.mock('./devices/Device')
.mock('./utils/onTerminate')
.mock('./utils/logError')
.mock('./utils/logger')
.mock('./client/Client')
.mock('./Detox', () => jest.fn(() => mockDetox))
.mock('./platform');
Expand All @@ -36,7 +36,7 @@ describe('index', () => {
});

it(`throws if there was no config passed`, async () => {
let logError = require('./utils/logError');
const logger = require('./utils/logger');
let exception = undefined;

try {
Expand All @@ -46,7 +46,7 @@ describe('index', () => {
}

expect(exception).toBeDefined();
expect(logError).toHaveBeenCalledWith(expect.anything(), exception);
expect(logger.error).toHaveBeenCalledWith({ event: 'DETOX_INIT_ERROR' }, '\n', exception);
});

it(`throws if there is no devices in config`, async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ exports[`constructSafeFilename should throw exception when non-trimmable prefix
`;

exports[`constructSafeFilename should throw if no arguments passed 1`] = `
"DetoxRuntimeError: no filename was given to constructSafeFilename()
"DetoxRuntimeError: no trimmable filename was given to constructSafeFilename(prefix, trimmable, suffix)

the arguments were: {
\\"prefix\\": \\"\\",
Expand All @@ -25,7 +25,7 @@ the arguments were: {
`;

exports[`constructSafeFilename should throw if no arguments passed 2`] = `
"DetoxRuntimeError: no filename was given to constructSafeFilename()
"DetoxRuntimeError: no trimmable filename was given to constructSafeFilename(prefix, trimmable, suffix)

the arguments were: {
\\"prefix\\": \\"\\",
Expand Down
Loading