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

[jest-util] Add ErrorWithStack class #7067

Merged
merged 11 commits into from
Sep 30, 2018
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

### Chore & Maintenance

- `[jest-util]` Add ErrorWithStack class ([#7067](https://github.com/facebook/jest/pull/7067))
- `[docs]` Document `--runTestsByPath` CLI parameter ([#7046](https://github.com/facebook/jest/pull/7046))
- `[docs]` Fix babel-core installation instructions ([#6745](https://github.com/facebook/jest/pull/6745))
- `[docs]` Explain how to rewrite assertions to avoid large irrelevant diff ([#6971](https://github.com/facebook/jest/pull/6971))
Expand Down
37 changes: 10 additions & 27 deletions packages/jest-circus/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
TestName,
} from 'types/Circus';
import {bind as bindEach} from 'jest-each';
import {ErrorWithStack} from 'jest-util';
import {dispatch} from './state';

type THook = (fn: HookFn, timeout?: number) => void;
Expand Down Expand Up @@ -44,10 +45,7 @@ const _addHook = (fn: HookFn, hookType: HookType, hookFn, timeout: ?number) => {
throw new Error('Invalid first argument. It must be a callback function.');
}

const asyncError = new Error();
if (Error.captureStackTrace) {
Error.captureStackTrace(asyncError, hookFn);
}
const asyncError = new ErrorWithStack(undefined, hookFn);
dispatch({asyncError, fn, hookType, name: 'add_hook', timeout});
};

Expand Down Expand Up @@ -78,10 +76,7 @@ const test = (testName: TestName, fn: TestFn, timeout?: number) => {
);
}

const asyncError = new Error();
if (Error.captureStackTrace) {
Error.captureStackTrace(asyncError, test);
}
const asyncError = new ErrorWithStack(undefined, test);

return dispatch({
asyncError,
Expand All @@ -93,10 +88,7 @@ const test = (testName: TestName, fn: TestFn, timeout?: number) => {
};
const it = test;
test.skip = (testName: TestName, fn?: TestFn, timeout?: number) => {
const asyncError = new Error();
if (Error.captureStackTrace) {
Error.captureStackTrace(asyncError, test);
}
const asyncError = new ErrorWithStack(undefined, test);

return dispatch({
asyncError,
Expand All @@ -108,10 +100,7 @@ test.skip = (testName: TestName, fn?: TestFn, timeout?: number) => {
});
};
test.only = (testName: TestName, fn: TestFn, timeout?: number) => {
const asyncError = new Error();
if (Error.captureStackTrace) {
Error.captureStackTrace(asyncError, test);
}
const asyncError = new ErrorWithStack(undefined, test);

return dispatch({
asyncError,
Expand All @@ -125,19 +114,13 @@ test.only = (testName: TestName, fn: TestFn, timeout?: number) => {

test.todo = (testName: TestName, ...rest: Array<mixed>) => {
if (rest.length > 0 || typeof testName !== 'string') {
const e = new Error('Todo must be called with only a description.');

if (Error.captureStackTrace) {
Error.captureStackTrace(e, test.todo);
}

throw e;
throw new ErrorWithStack(
'Todo must be called with only a description.',
test.todo,
);
}

const asyncError = new Error();
if (Error.captureStackTrace) {
Error.captureStackTrace(asyncError, test);
}
const asyncError = new ErrorWithStack(undefined, test);

return dispatch({
asyncError,
Expand Down
7 changes: 2 additions & 5 deletions packages/jest-cli/src/collectHandles.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import type {ProjectConfig} from 'types/Config';

import {formatExecError} from 'jest-message-util';
import {ErrorWithStack} from 'jest-util';
import stripAnsi from 'strip-ansi';

function stackIsFromUser(stack) {
Expand Down Expand Up @@ -43,11 +44,7 @@ export default function collectHandles(): () => Array<Error> {
if (type === 'PROMISE' || type === 'TIMERWRAP') {
return;
}
const error = new Error(type);

if (Error.captureStackTrace) {
Error.captureStackTrace(error, initHook);
}
const error = new ErrorWithStack(type, initHook);

if (stackIsFromUser(error.stack)) {
activeHandles.set(asyncId, error);
Expand Down
1 change: 1 addition & 0 deletions packages/jest-each/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"license": "MIT",
"dependencies": {
"chalk": "^2.0.1",
"jest-util": "^23.4.0",
"pretty-format": "^23.6.0"
}
}
13 changes: 3 additions & 10 deletions packages/jest-each/src/bind.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import util from 'util';
import chalk from 'chalk';
import pretty from 'pretty-format';
import {ErrorWithStack} from 'jest-util';

type Table = Array<Array<any>>;
type PrettyArgs = {
Expand All @@ -23,21 +24,13 @@ const SUPPORTED_PLACEHOLDERS = /%[sdifjoOp%]/g;
const PRETTY_PLACEHOLDER = '%p';
const INDEX_PLACEHOLDER = '%#';

const errorWithStack = (message, callsite) => {
const error = new Error(message);
if (Error.captureStackTrace) {
Error.captureStackTrace(error, callsite);
}
return error;
};

export default (cb: Function, supportsDone: boolean = true) => (...args: any) =>
function eachBind(title: string, test: Function, timeout: number): void {
if (args.length === 1) {
const [tableArg] = args;

if (!Array.isArray(tableArg)) {
const error = errorWithStack(
const error = new ErrorWithStack(
'`.each` must be called with an Array or Tagged Template String.\n\n' +
`Instead was called with: ${pretty(tableArg, {
maxDepth: 1,
Expand Down Expand Up @@ -70,7 +63,7 @@ export default (cb: Function, supportsDone: boolean = true) => (...args: any) =>
const missingData = data.length % keys.length;

if (missingData > 0) {
const error = errorWithStack(
const error = new ErrorWithStack(
'Not enough arguments supplied for given headings:\n' +
EXPECTED_COLOR(keys.join(' | ')) +
'\n\n' +
Expand Down
7 changes: 2 additions & 5 deletions packages/jest-jasmine2/src/error_on_private.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type {Global} from '../../../types/Global';
import {ErrorWithStack} from 'jest-util';

// prettier-ignore
const disabledGlobals = {
Expand Down Expand Up @@ -60,9 +61,5 @@ export function installErrorOnPrivate(global: Global): void {
}

function throwAtFunction(message, fn) {
const e = new Error(message);
if (Error.captureStackTrace) {
Error.captureStackTrace(e, fn);
}
throw e;
throw new ErrorWithStack(message, fn);
}
12 changes: 5 additions & 7 deletions packages/jest-jasmine2/src/jasmine/Env.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import queueRunner from '../queue_runner';
import treeProcessor from '../tree_processor';
import checkIsError from '../is_error';
import assertionErrorMessage from '../assert_support';
import {ErrorWithStack} from 'jest-util';

// Try getting the real promise object from the context, if available. Someone
// could have overridden it in a test. Async functions return it implicitly.
Expand Down Expand Up @@ -497,13 +498,10 @@ export default function(j$) {
this.todo = function() {
const description = arguments[0];
if (arguments.length !== 1 || typeof description !== 'string') {
const e = new Error('Todo must be called with only a description.');

if (Error.captureStackTrace) {
Error.captureStackTrace(e, test.todo);
}

throw e;
throw new ErrorWithStack(
'Todo must be called with only a description.',
test.todo,
);
}

const spec = specFactory(description, () => {}, currentDeclarationSuite);
Expand Down
10 changes: 5 additions & 5 deletions packages/jest-runner/src/run_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import fs from 'graceful-fs';
import {
BufferedConsole,
Console,
ErrorWithStack,
NullConsole,
getConsoleOutput,
setGlobal,
Expand Down Expand Up @@ -154,11 +155,10 @@ async function runTestInternal(
const realExit = environment.global.process.exit;

environment.global.process.exit = function exit(...args) {
const error = new Error(`process.exit called with "${args.join(', ')}"`);

if (Error.captureStackTrace) {
Error.captureStackTrace(error, exit);
}
const error = new ErrorWithStack(
`process.exit called with "${args.join(', ')}"`,
exit,
);

const formattedError = formatExecError(
error,
Expand Down
25 changes: 25 additions & 0 deletions packages/jest-util/src/__tests__/error_with_stack.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Copyright (c) 2018-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import ErrorWithStack from '../error_with_stack';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be cool to flow it up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's worth adding flow to the test, it causes the following issue (which I'd have to disable):

[flow] Cannot assign `jest.fn()` to `Error.captureStackTrace` because property `captureStackTrace` is not writable. (error_with_stack.test.js:17:11)

Happy to add it though if you think we should :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. It's just nice to have flow in tests too so you don't end up testing scenarios that are already covered/prevented by type checker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, I've updated to use jest.spyOn instead and added @flow with no warnings 🎉


describe('ErrorWithStack', () => {
const message = '💩 something went wrong';
const callsite = () => {};

it('calls Error.captureStackTrace with given callsite when capture exists', () => {
jest.spyOn(Error, 'captureStackTrace');

const actual = new ErrorWithStack(message, callsite);

expect(actual).toBeInstanceOf(Error);
expect(actual.message).toBe(message);
expect(Error.captureStackTrace).toHaveBeenCalledWith(actual, callsite);
});
});
17 changes: 17 additions & 0 deletions packages/jest-util/src/error_with_stack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Copyright (c) 2018-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

export default class ErrorWithStack extends Error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mind adding preamble and flow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woops just added :)

constructor(message: ?string, callsite: Function) {
super(message);
if (Error.captureStackTrace) {
Error.captureStackTrace(this, callsite);
}
}
}
2 changes: 2 additions & 0 deletions packages/jest-util/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import mkdirp from 'mkdirp';
import BufferedConsole from './buffered_console';
import clearLine from './clear_line';
import Console from './Console';
import ErrorWithStack from './error_with_stack';
import FakeTimers from './fake_timers';
import formatTestResults from './format_test_results';
import getFailedSnapshotTests from './get_failed_snapshot_tests';
Expand All @@ -37,6 +38,7 @@ const createDirectory = (path: string) => {
module.exports = {
BufferedConsole,
Console,
ErrorWithStack,
FakeTimers,
NullConsole,
clearLine,
Expand Down