Skip to content

Commit

Permalink
fix: don't assume stack is always a string (#10697)
Browse files Browse the repository at this point in the history
  • Loading branch information
snitin315 authored Oct 27, 2020
1 parent 7b1568d commit 132e3d1
Show file tree
Hide file tree
Showing 14 changed files with 113 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

- `[jest-config]` Fix bug introduced in watch mode by PR[#10678](https://github.com/facebook/jest/pull/10678/files#r511037803) ([#10692](https://github.com/facebook/jest/pull/10692))
- `[expect]` Stop modifying the sample in `expect.objectContaining()` ([#10711](https://github.com/facebook/jest/pull/10711))
- `[jest-circus, jest-jasmine2]` fix: don't assume `stack` is always a string ([#10697](https://github.com/facebook/jest/pull/10697))
- `[jest-resolve-dependencies]` Resolve mocks as dependencies ([#10713](https://github.com/facebook/jest/pull/10713))

### Chore & Maintenance
Expand Down
81 changes: 54 additions & 27 deletions e2e/__tests__/__snapshots__/failures.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ exports[`not throwing Error objects 3`] = `
FAIL __tests__/throwObject.test.js
● Test suite failed to run
Error: No message was provided
thrown: Object {}
`;

exports[`not throwing Error objects 4`] = `
Expand Down Expand Up @@ -117,6 +117,7 @@ FAIL __tests__/duringTests.test.js
Boolean thrown during test
undefined thrown during test
Object thrown during test
Object with stack prop thrown during test
Error during test
done(Error)
done(non-error)
Expand Down Expand Up @@ -185,33 +186,49 @@ FAIL __tests__/duringTests.test.js
at Object.test (__tests__/duringTests.test.js:28:1)
● Object with stack prop thrown during test
thrown: Object {
"stack": 42,
}
30 | });
31 |
> 32 | test('Object with stack prop thrown during test', () => {
| ^
33 | // eslint-disable-next-line no-throw-literal
34 | throw {stack: 42};
35 | });
at Object.test (__tests__/duringTests.test.js:32:1)
● Error during test
ReferenceError: doesNotExist is not defined
32 | test('Error during test', () => {
33 | // eslint-disable-next-line no-undef
> 34 | doesNotExist.alsoThisNot;
37 | test('Error during test', () => {
38 | // eslint-disable-next-line no-undef
> 39 | doesNotExist.alsoThisNot;
| ^
35 | });
36 |
37 | test('done(Error)', done => {
40 | });
41 |
42 | test('done(Error)', done => {
at Object.doesNotExist (__tests__/duringTests.test.js:34:3)
at Object.doesNotExist (__tests__/duringTests.test.js:39:3)
done(Error)
this is an error
36 |
37 | test('done(Error)', done => {
> 38 | done(new Error('this is an error'));
41 |
42 | test('done(Error)', done => {
> 43 | done(new Error('this is an error'));
| ^
39 | });
40 |
41 | test('done(non-error)', done => {
44 | });
45 |
46 | test('done(non-error)', done => {
at Object.<anonymous> (__tests__/duringTests.test.js:38:8)
at Object.<anonymous> (__tests__/duringTests.test.js:43:8)
● done(non-error)
Expand All @@ -224,15 +241,15 @@ FAIL __tests__/duringTests.test.js
],
}
40 |
41 | test('done(non-error)', done => {
> 42 | done(deepObject);
45 |
46 | test('done(non-error)', done => {
> 47 | done(deepObject);
| ^
43 | });
44 |
45 | test('returned promise rejection', () => Promise.reject(deepObject));
48 | });
49 |
50 | test('returned promise rejection', () => Promise.reject(deepObject));
at Object.done (__tests__/duringTests.test.js:42:3)
at Object.done (__tests__/duringTests.test.js:47:3)
● returned promise rejection
Expand All @@ -245,13 +262,23 @@ FAIL __tests__/duringTests.test.js
],
}
43 | });
44 |
> 45 | test('returned promise rejection', () => Promise.reject(deepObject));
48 | });
49 |
> 50 | test('returned promise rejection', () => Promise.reject(deepObject));
| ^
46 |
51 |
at Object.test (__tests__/duringTests.test.js:45:1)
at Object.test (__tests__/duringTests.test.js:50:1)
`;
exports[`not throwing Error objects 6`] = `
FAIL __tests__/throwObjectWithStackProp.test.js
● Test suite failed to run
thrown: Object {
"stack": 42,
}
`;
exports[`works with assertions in separate files 1`] = `
Expand Down
4 changes: 3 additions & 1 deletion e2e/__tests__/failures.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ test('not throwing Error objects', () => {
stderr = runJest(dir, ['duringTests.test.js']).stderr;

if (nodeMajorVersion < 12) {
const lineEntry = '(__tests__/duringTests.test.js:38:8)';
const lineEntry = '(__tests__/duringTests.test.js:43:8)';

expect(stderr).toContain(`at Object.<anonymous>.done ${lineEntry}`);

Expand All @@ -51,6 +51,8 @@ test('not throwing Error objects', () => {
}

expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
stderr = runJest(dir, ['throwObjectWithStackProp.test.js']).stderr;
expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
});

test('works with node assert', () => {
Expand Down
5 changes: 5 additions & 0 deletions e2e/failures/__tests__/duringTests.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ test('Object thrown during test', () => {
throw deepObject;
});

test('Object with stack prop thrown during test', () => {
// eslint-disable-next-line no-throw-literal
throw {stack: 42};
});

test('Error during test', () => {
// eslint-disable-next-line no-undef
doesNotExist.alsoThisNot;
Expand Down
11 changes: 11 additions & 0 deletions e2e/failures/__tests__/throwObjectWithStackProp.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. 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.
*/

'use strict';

// eslint-disable-next-line no-throw-literal
throw {stack: 42};
5 changes: 3 additions & 2 deletions packages/jest-circus/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ const _getError = (
asyncError = new Error();
}

if (error && (error.stack || error.message)) {
if (error && (typeof error.stack === 'string' || error.message)) {
return error;
}

Expand All @@ -392,7 +392,8 @@ const _getError = (
return asyncError;
};

const getErrorStack = (error: Error): string => error.stack || error.message;
const getErrorStack = (error: Error): string =>
typeof error.stack === 'string' ? error.stack : error.message;

export const addErrorToEachTestUnderDescribe = (
describeBlock: Circus.DescribeBlock,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`expectationResultFactory returns the result if failed (with \`error.stack\` not as a string). 1`] = `
"thrown: Object {
\\"stack\\": 42,
}"
`;

exports[`expectationResultFactory returns the result if passed. 1`] = `
Object {
"error": undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,16 @@ describe('expectationResultFactory', () => {
const result = expectationResultFactory(options);
expect(result.message).toEqual('Expected `Pass`, received `Fail`.');
});

it('returns the result if failed (with `error.stack` not as a string).', () => {
const options = {
actual: 'Fail',
error: {stack: 42},
expected: 'Pass',
matcherName: 'testMatcher',
passed: false,
};
const result = expectationResultFactory(options);
expect(result.message).toMatchSnapshot();
});
});
2 changes: 1 addition & 1 deletion packages/jest-jasmine2/src/expectationResultFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function stackFormatter(
}

if (options.error) {
if (options.error.stack) {
if (typeof options.error.stack === 'string') {
return options.error.stack;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-jasmine2/src/reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export default class Jasmine2Reporter implements Reporter {

specResult.failedExpectations.forEach(failed => {
const message =
!failed.matcherName && failed.stack
!failed.matcherName && typeof failed.stack === 'string'
? this._addMissingMessageToStack(failed.stack, failed.message)
: failed.message || '';
results.failureMessages.push(message);
Expand Down
1 change: 1 addition & 0 deletions packages/jest-message-util/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"chalk": "^4.0.0",
"graceful-fs": "^4.2.4",
"micromatch": "^4.0.2",
"pretty-format": "^26.6.1",
"slash": "^3.0.0",
"stack-utils": "^2.0.2"
},
Expand Down
13 changes: 10 additions & 3 deletions packages/jest-message-util/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import * as fs from 'graceful-fs';
import type {Config, TestResult} from '@jest/types';
import chalk = require('chalk');
import micromatch = require('micromatch');
import prettyFormat = require('pretty-format');
import slash = require('slash');
import {codeFrameColumns} from '@babel/code-frame';
import StackUtils = require('stack-utils');
Expand Down Expand Up @@ -140,7 +141,10 @@ export const formatExecError = (
stack = error;
} else {
message = error.message;
stack = error.stack;
stack =
typeof error.stack === 'string'
? error.stack
: `thrown: ${prettyFormat(error, {maxDepth: 3})}`;
}

const separated = separateMessageFromStack(stack || '');
Expand All @@ -160,9 +164,12 @@ export const formatExecError = (
? '\n' + formatStackTrace(stack, config, options, testPath)
: '';

if (blankStringRegexp.test(message) && blankStringRegexp.test(stack)) {
if (
typeof stack !== 'string' ||
(blankStringRegexp.test(message) && blankStringRegexp.test(stack))
) {
// this can happen if an empty object is thrown.
message = MESSAGE_INDENT + 'Error: No message was provided';
message = `thrown: ${prettyFormat(error, {maxDepth: 3})}`;
}

let messageToUse;
Expand Down
5 changes: 4 additions & 1 deletion packages/jest-message-util/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@
"rootDir": "src",
"outDir": "build"
},
"references": [{"path": "../jest-types"}]
"references": [
{"path": "../jest-types"},
{"path": "../pretty-format"}
]
}
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -11728,6 +11728,7 @@ fsevents@^1.2.7:
chalk: ^4.0.0
graceful-fs: ^4.2.4
micromatch: ^4.0.2
pretty-format: ^26.6.1
slash: ^3.0.0
stack-utils: ^2.0.2
languageName: unknown
Expand Down

0 comments on commit 132e3d1

Please sign in to comment.