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

fix: don't assume stack is always a string #10697

Merged
merged 17 commits into from Oct 27, 2020
1 change: 1 addition & 0 deletions CHANGELOG.md
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
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 {}
SimenB marked this conversation as resolved.
Show resolved Hide resolved
`;

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
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
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
@@ -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
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
@@ -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
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
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
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
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
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');
SimenB marked this conversation as resolved.
Show resolved Hide resolved
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
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
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