Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Handle async errors #4016

Merged
merged 10 commits into from
Aug 24, 2017
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
26 changes: 18 additions & 8 deletions integration_tests/__tests__/jasmine_async.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ describe('async jasmine', () => {
]);
const json = result.json;

expect(json.numTotalTests).toBe(2);
expect(json.numTotalTests).toBe(4);
expect(json.numPassedTests).toBe(1);
expect(json.numFailedTests).toBe(1);
expect(json.numFailedTests).toBe(3);
expect(json.numPendingTests).toBe(0);

const {message} = json.testResults[0];
expect(message).toMatch('with failing timeout');
expect(message).toMatch('Async callback was not invoked within timeout');
expect(message).toMatch('done - with error thrown');
expect(message).toMatch('done - with error called back');
});

it('works with beforeEach', () => {
Expand All @@ -35,11 +37,14 @@ describe('async jasmine', () => {
]);
const json = result.json;

expect(json.numTotalTests).toBe(1);
expect(json.numTotalTests).toBe(3);
expect(json.numPassedTests).toBe(1);
expect(json.numFailedTests).toBe(0);
expect(json.numFailedTests).toBe(2);
expect(json.numPendingTests).toBe(0);
expect(json.testResults[0].message).toBe('');

const {message} = json.testResults[0];
expect(message).toMatch('done - with error thrown');
expect(message).toMatch('done - with error called back');
});

it('works with afterAll', () => {
Expand Down Expand Up @@ -104,12 +109,17 @@ describe('async jasmine', () => {
const json = result.json;
const message = json.testResults[0].message;

expect(json.numTotalTests).toBe(9);
expect(json.numPassedTests).toBe(5);
expect(json.numFailedTests).toBe(4);
expect(json.numTotalTests).toBe(16);
expect(json.numPassedTests).toBe(6);
expect(json.numFailedTests).toBe(9);

expect(message).toMatch('fails if promise is rejected');
expect(message).toMatch('works with done.fail');
expect(message).toMatch('works with done(error)');
expect(message).toMatch('fails if failed expectation with done');
expect(message).toMatch('fails if failed expectation with done - async');
expect(message).toMatch('fails with thrown error with done - sync');
expect(message).toMatch('fails with thrown error with done - async');
expect(message).toMatch('fails a sync test');
expect(message).toMatch('fails if a custom timeout is exceeded');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,19 @@ describe('promise beforeAll', () => {

it('fails', () => {});
});

describe('done - with error thrown', () => {
beforeAll(done => {
throw new Error('fail');
done(); // eslint-disable-line
});
it('fails', () => {});
});

describe('done - with error called back', () => {
beforeAll(done => {
done(new Error('fail'));
});
it('fails', () => {});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,20 @@ describe('promise beforeEach', () => {
it('runs tests after beforeEach asynchronously completes', () => {
expect(this.flag).toBe(1);
});

// failing tests
describe('done - with error thrown', () => {
beforeEach(done => {
throw new Error('fail');
done(); // eslint-disable-line
});
it('fails', () => {});
});

describe('done - with error called back', () => {
beforeEach(done => {
done(new Error('fail'));
});
it('fails', () => {});
});
});
40 changes: 40 additions & 0 deletions integration_tests/jasmine_async/__tests__/promise_it.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ describe('promise it', () => {
done();
});

it('works with async done', done => {
setTimeout(done, 1);
});

it('is bound to context object', () => {
return new Promise(resolve => {
if (this.someContextValue !== 'value') {
Expand All @@ -46,6 +50,42 @@ describe('promise it', () => {
done.fail(new Error('done.fail was called'));
});

it('works with done(error)', done => {
done(new Error('done was called with error'));
});

it('fails if failed expectation with done', done => {
expect(true).toEqual(false);
done();
});

it('fails if failed expectation with done - async', done => {
setTimeout(() => {
expect(true).toEqual(false);
done();
}, 1);
});

it('fails with thrown error with done - sync', done => {
throw new Error('sync fail');
done(); // eslint-disable-line
});

it('fails with thrown error with done - async', done => {
setTimeout(() => {
throw new Error('async fail');
done(); // eslint-disable-line
}, 1);
});

// I wish it was possible to catch this but I do not see a way.
// Currently both jest and mocha will pass this test.
it.skip('fails with thrown error - async', () => {
setTimeout(() => {
throw new Error('async fail - no done');
}, 1);
});

it('fails a sync test', () => {
expect('sync').toBe('failed');
});
Expand Down
3 changes: 2 additions & 1 deletion packages/jest-jasmine2/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"jest-matcher-utils": "^20.0.3",
"jest-matchers": "^20.0.3",
"jest-message-util": "^20.0.3",
"jest-snapshot": "^20.0.3"
"jest-snapshot": "^20.0.3",
"p-cancelable": "^0.3.0"
},
"devDependencies": {
"jest-runtime": "^20.0.4"
Expand Down
26 changes: 26 additions & 0 deletions packages/jest-jasmine2/src/__tests__/queue_runner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,30 @@ describe('queueRunner', () => {

expect(options.fail).toHaveBeenCalledWith('miserably', 'failed');
});

it('calls `fail` when done(error) is invoked', async () => {
const error = new Error('I am an error');
const fail = jest.fn();
const fnOne = jest.fn(next => next(error));
const fnTwo = jest.fn(next => next());
const options = {
clearTimeout,
fail,
onException: () => {},
queueableFns: [
{
fn: fnOne,
},
{
fn: fnTwo,
},
],
setTimeout,
};
await queueRunner(options);
expect(fnOne).toHaveBeenCalled();
expect(fail).toHaveBeenCalledWith(error);
// Even if `fail` is called, the queue keeps running.
expect(fnTwo).toHaveBeenCalled();
});
});
42 changes: 38 additions & 4 deletions packages/jest-jasmine2/src/jasmine/Env.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ module.exports = function(j$) {
return seed;
};

async function queueRunnerFactory(options) {
function queueRunnerFactory(options) {
options.clearTimeout = realClearTimeout;
options.fail = self.fail;
options.setTimeout = realSetTimeout;
Expand All @@ -187,9 +187,31 @@ module.exports = function(j$) {
}
}

reporter.jasmineStarted({
totalSpecsDefined,
});
const uncaught = err => {
if (currentSpec) {
currentSpec.onException(err);
currentSpec.cancel();
} else {
console.error('Unhandled error');
console.error(err.stack);
}
};

// Need to ensure we are the only ones handling these exceptions.
const oldListenersException = process
.listeners('uncaughtException')
.slice();
const oldListenersRejection = process
.listeners('unhandledRejection')
.slice();

process.removeAllListeners('uncaughtException');
process.removeAllListeners('unhandledRejection');

process.on('uncaughtException', uncaught);
process.on('unhandledRejection', uncaught);

reporter.jasmineStarted({totalSpecsDefined});

currentlyExecutingSuites.push(topSuite);

Expand All @@ -215,6 +237,18 @@ module.exports = function(j$) {
reporter.jasmineDone({
failedExpectations: topSuite.result.failedExpectations,
});

process.removeListener('uncaughtException', uncaught);
process.removeListener('unhandledRejection', uncaught);

// restore previous exception handlers
oldListenersException.forEach(listener => {
process.on('uncaughtException', listener);
});

oldListenersRejection.forEach(listener => {
process.on('unhandledRejection', listener);
});
};

this.addReporter = function(reporterToAdd) {
Expand Down
12 changes: 10 additions & 2 deletions packages/jest-jasmine2/src/jasmine/Spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,15 @@ Spec.prototype.execute = function(onComplete, enabled) {
const fns = this.beforeAndAfterFns();
const allFns = fns.befores.concat(this.queueableFn).concat(fns.afters);

this.queueRunnerFactory({
this.currentRun = this.queueRunnerFactory({
queueableFns: allFns,
onException() {
self.onException.apply(self, arguments);
},
userContext: this.userContext(),
}).then(() => complete(true));
});

this.currentRun.then(() => complete(true));

function complete(enabledAgain) {
self.result.status = self.status(enabledAgain);
Expand All @@ -114,6 +116,12 @@ Spec.prototype.execute = function(onComplete, enabled) {
}
};

Spec.prototype.cancel = function cancel() {
if (this.currentRun) {
this.currentRun.cancel();
}
};

Spec.prototype.onException = function onException(error) {
if (Spec.isPendingSpecException(error)) {
this.pend(extractCustomPendingMessage(error));
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-jasmine2/src/jasmine_async.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function promisifyLifeCycleFunction(originalFn, env) {
const returnValue = fn.call({});

if (isPromise(returnValue)) {
returnValue.then(done, done.fail);
returnValue.then(done.bind(null, null), done.fail);
} else {
done();
}
Expand Down Expand Up @@ -68,7 +68,7 @@ function promisifyIt(originalFn, env) {
const returnValue = fn.call({});

if (isPromise(returnValue)) {
returnValue.then(done, done.fail);
returnValue.then(done.bind(null, null), done.fail);
} else if (returnValue === undefined) {
done();
} else {
Expand Down
28 changes: 24 additions & 4 deletions packages/jest-jasmine2/src/queue_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* @flow
*/

import PCancelable from 'p-cancelable';
import pTimeout from './p_timeout';

type Options = {
Expand All @@ -24,10 +25,20 @@ type QueueableFn = {
timeout?: () => number,
};

async function queueRunner(options: Options) {
function queueRunner(options: Options) {
const token = new PCancelable((onCancel, resolve) => {
onCancel(resolve);
});

const mapper = ({fn, timeout}) => {
const promise = new Promise(resolve => {
const next = () => resolve();
let promise = new Promise(resolve => {
const next = function(err) {
if (err) {
options.fail.apply(null, arguments);
}
resolve();
};

next.fail = function() {
options.fail.apply(null, arguments);
resolve();
Expand All @@ -39,6 +50,9 @@ async function queueRunner(options: Options) {
resolve();
}
});

promise = Promise.race([promise, token]);

if (!timeout) {
return promise;
}
Expand All @@ -57,10 +71,16 @@ async function queueRunner(options: Options) {
);
};

return options.queueableFns.reduce(
const result = options.queueableFns.reduce(
(promise, fn) => promise.then(() => mapper(fn)),
Promise.resolve(),
);

return {
cancel: token.cancel.bind(token),
catch: result.catch.bind(result),
then: result.then.bind(result),
};
}

module.exports = queueRunner;
10 changes: 4 additions & 6 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4599,6 +4599,10 @@ osenv@^0.1.4:
os-homedir "^1.0.0"
os-tmpdir "^1.0.0"

p-cancelable@^0.3.0:
version "0.3.0"
resolved "https://registry.yarnpkg.com/p-cancelable/-/p-cancelable-0.3.0.tgz#b9e123800bcebb7ac13a479be195b507b98d30fa"

p-finally@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/p-finally/-/p-finally-1.0.0.tgz#3fbcfb15b899a44123b34b6dcc18b724336a2cae"
Expand Down Expand Up @@ -5208,12 +5212,6 @@ resolve@^1.1.3, resolve@^1.1.4, resolve@^1.1.6, resolve@^1.1.7:
dependencies:
path-parse "^1.0.5"

resolve@^1.3.2:
version "1.3.3"
resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.3.3.tgz#655907c3469a8680dc2de3a275a8fdd69691f0e5"
dependencies:
path-parse "^1.0.5"

resolve@^1.4.0:
version "1.4.0"
resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.4.0.tgz#a75be01c53da25d934a98ebd0e4c4a7312f92a86"
Expand Down