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 test suite for Node 10 #6055

Merged
merged 6 commits into from
Apr 23, 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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@

### Fixes

* `[jest-jasmine2]` Always remove node core message from assert stack traces
([#6055](https://github.com/facebook/jest/pull/6055))
* `[expect]` Add stack trace when `expect.assertions` and `expect.hasAssertions`
causes test failures. ([#5997](https://github.com/facebook/jest/pull/5997))
* `[jest-runtime]` Throw a more useful error when trying to require modules
Expand Down
40 changes: 13 additions & 27 deletions integration-tests/__tests__/__snapshots__/failures.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ exports[`not throwing Error objects 5`] = `
10 |


at packages/jest-jasmine2/build/jasmine/Spec.js:79:20
at packages/jest-jasmine2/build/jasmine/Spec.js:85:20
at __tests__/during_tests.test.js:7:1

● Boolean thrown during test
Expand All @@ -159,7 +159,7 @@ exports[`not throwing Error objects 5`] = `
14 | });


at packages/jest-jasmine2/build/jasmine/Spec.js:79:20
at packages/jest-jasmine2/build/jasmine/Spec.js:85:20
at __tests__/during_tests.test.js:11:1

● undefined thrown during test
Expand All @@ -175,7 +175,7 @@ exports[`not throwing Error objects 5`] = `
19 | });


at packages/jest-jasmine2/build/jasmine/Spec.js:79:20
at packages/jest-jasmine2/build/jasmine/Spec.js:85:20
at __tests__/during_tests.test.js:16:1

● Object thrown during test
Expand All @@ -198,7 +198,7 @@ exports[`not throwing Error objects 5`] = `
24 | });


at packages/jest-jasmine2/build/jasmine/Spec.js:79:20
at packages/jest-jasmine2/build/jasmine/Spec.js:85:20
at __tests__/during_tests.test.js:21:1

● Error during test
Expand Down Expand Up @@ -391,7 +391,7 @@ exports[`works with async failures 1`] = `
29 |
30 | setTimeout(done, 10);

at packages/jest-jasmine2/build/jasmine/Spec.js:79:20
at packages/jest-jasmine2/build/jasmine/Spec.js:85:20
at __tests__/async_failures.test.js:27:1

"
Expand Down Expand Up @@ -446,7 +446,7 @@ exports[`works with node assert 1`] = `

assert.equal(received, expected) or assert(received)

Expected value to be (operator: ==):
Expected value to be equal to:
true
Received:
false
Expand All @@ -467,7 +467,7 @@ exports[`works with node assert 1`] = `

assert.equal(received, expected) or assert(received)

Expected value to be (operator: ==):
Expected value to be equal to:
true
Received:
false
Expand All @@ -491,7 +491,7 @@ exports[`works with node assert 1`] = `

assert.equal(received, expected) or assert(received)

Expected value to be (operator: ==):
Expected value to be equal to:
true
Received:
false
Expand All @@ -512,7 +512,7 @@ exports[`works with node assert 1`] = `

assert.equal(received, expected) or assert(received)

Expected value to be (operator: ==):
Expected value to be equal to:
true
Received:
false
Expand All @@ -536,7 +536,7 @@ exports[`works with node assert 1`] = `

assert.equal(received, expected) or assert(received)

Expected value to be (operator: ==):
Expected value to be equal to:
2
Received:
1
Expand All @@ -557,7 +557,7 @@ exports[`works with node assert 1`] = `

assert.notEqual(received, expected)

Expected value not to be (operator: !=):
Expected value to not be equal to:
1
Received:
1
Expand Down Expand Up @@ -680,7 +680,7 @@ exports[`works with node assert 1`] = `

assert.strictEqual(received, expected)

Expected value to be (operator: ===):
Expected value to strictly be equal to:
NaN
Received:
1
Expand All @@ -701,7 +701,7 @@ exports[`works with node assert 1`] = `

assert.notStrictEqual(received, expected)

Expected value not to be (operator: !==):
Expected value not be strictly equal to:
1
Received:
1
Expand Down Expand Up @@ -783,20 +783,6 @@ exports[`works with node assert 1`] = `

● assert.ifError

thrown: 1

64 | });
65 |
> 66 | test('assert.ifError', () => {
| ^
67 | assert.ifError(1);
68 | });
69 |


at packages/jest-jasmine2/build/jasmine/Spec.js:79:20
at __tests__/node_assertion_error.test.js:66:1

● assert.doesNotThrow

assert.doesNotThrow(function)
Expand Down
84 changes: 74 additions & 10 deletions integration-tests/__tests__/failures.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,13 @@ test('not throwing Error objects', () => {
});

test('works with node assert', () => {
const nodeMajorVersion = Number(process.versions.node.split('.')[0]);
const {stderr} = runJest(dir, ['node_assertion_error.test.js']);
let summary = normalizeDots(extractSummary(stderr).rest);

// Node 9 started to include the error for `doesNotThrow`
// https://github.com/nodejs/node/pull/12167
if (Number(process.versions.node.split('.')[0]) >= 9) {
if (nodeMajorVersion >= 9) {
expect(summary).toContain(`
assert.doesNotThrow(function)

Expand All @@ -48,9 +49,9 @@ test('works with node assert', () => {

Message:
Got unwanted exception.
err!
err!
`);

expect(summary).toContain(`
69 |
70 | test('assert.doesNotThrow', () => {
> 71 | assert.doesNotThrow(() => {
Expand All @@ -62,16 +63,79 @@ test('works with node assert', () => {
at __tests__/node_assertion_error.test.js:71:10
`);

summary = summary.replace(
`Message:
const commonErrorMessage = `Message:
Got unwanted exception.
`;

if (nodeMajorVersion === 9) {
const specificErrorMessage = `Message:
Got unwanted exception.
err!
err!
`,
`Message:
`;

expect(summary).toContain(specificErrorMessage);
summary = summary.replace(specificErrorMessage, commonErrorMessage);
} else {
const specificErrorMessage = `Message:
Got unwanted exception.
`,
);
Actual message: "err!"
`;

expect(summary).toContain(specificErrorMessage);
summary = summary.replace(specificErrorMessage, commonErrorMessage);
}
}

if (nodeMajorVersion >= 10) {
const ifErrorMessage = `
assert.ifError(received, expected)

Expected value ifError to:
null
Received:
1

Message:
ifError got unwanted exception: 1

Difference:

Comparing two different types of values. Expected null but received number.

65 |
66 | test('assert.ifError', () => {
> 67 | assert.ifError(1);
| ^
68 | });
69 |
70 | test('assert.doesNotThrow', () => {

at __tests__/node_assertion_error.test.js:67:10

at __tests__/node_assertion_error.test.js:66:1
`;

expect(summary).toContain(ifErrorMessage);
summary = summary.replace(ifErrorMessage, '');
} else {
const ifErrorMessage = `
thrown: 1

64 | });
65 |
> 66 | test('assert.ifError', () => {
| ^
67 | assert.ifError(1);
68 | });
69 |


at packages/jest-jasmine2/build/jasmine/Spec.js:85:20
at __tests__/node_assertion_error.test.js:66:1
`;

expect(summary).toContain(ifErrorMessage);
summary = summary.replace(ifErrorMessage, '');
}

expect(summary).toMatchSnapshot();
Expand Down
35 changes: 21 additions & 14 deletions packages/jest-jasmine2/src/assert_support.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@ const assertOperatorsMap = {
const humanReadableOperators = {
deepEqual: 'to deeply equal',
deepStrictEqual: 'to deeply and strictly equal',
equal: 'to be equal',
notDeepEqual: 'not to deeply equal',
notDeepStrictEqual: 'not to deeply and strictly equal',
notEqual: 'to not be equal',
notStrictEqual: 'not be strictly equal',
strictEqual: 'to strictly be equal',
};

const getOperatorName = (operator: ?string, stack: string) => {
Expand All @@ -50,12 +54,15 @@ const getOperatorName = (operator: ?string, stack: string) => {
return '';
};

const operatorMessage = (operator: ?string, negator: boolean) =>
typeof operator === 'string'
? operator.startsWith('!') || operator.startsWith('=')
? `${negator ? 'not ' : ''}to be (operator: ${operator}):\n`
: `${humanReadableOperators[operator] || operator} to:\n`
const operatorMessage = (operator: ?string) => {
const niceOperatorName = getOperatorName(operator, '');
// $FlowFixMe: we default to the operator itseld, so holes in the map doesn't matter
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I fixed it in #6061 :P

const humanReadableOperator = humanReadableOperators[niceOperatorName];

return typeof operator === 'string'
? `${humanReadableOperator || niceOperatorName} to:\n`
: '';
};

const assertThrowingMatcherHint = (operatorName: string) => {
return (
Expand Down Expand Up @@ -88,13 +95,13 @@ const assertMatcherHint = (operator: ?string, operatorName: string) => {
};

function assertionErrorMessage(error: AssertionError, options: DiffOptions) {
const {expected, actual, message, operator, stack} = error;
const {expected, actual, generatedMessage, message, operator, stack} = error;
const diffString = diff(expected, actual, options);
const negator =
typeof operator === 'string' &&
(operator.startsWith('!') || operator.startsWith('not'));
const hasCustomMessage = !error.generatedMessage;
const hasCustomMessage = !generatedMessage;
const operatorName = getOperatorName(operator, stack);
const trimmedStack = stack
.replace(message, '')
Copy link
Member Author

Choose a reason for hiding this comment

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

this replace is what I think needs to land in Jest 22

.replace(/AssertionError(.*)/g, '');

if (operatorName === 'doesNotThrow') {
return (
Expand All @@ -104,7 +111,7 @@ function assertionErrorMessage(error: AssertionError, options: DiffOptions) {
chalk.reset(`Instead, it threw:\n`) +
` ${printReceived(actual)}` +
chalk.reset(hasCustomMessage ? '\n\nMessage:\n ' + message : '') +
stack.replace(/AssertionError(.*)/g, '')
trimmedStack
);
}

Expand All @@ -115,20 +122,20 @@ function assertionErrorMessage(error: AssertionError, options: DiffOptions) {
chalk.reset(`Expected the function to throw an error.\n`) +
chalk.reset(`But it didn't throw anything.`) +
chalk.reset(hasCustomMessage ? '\n\nMessage:\n ' + message : '') +
stack.replace(/AssertionError(.*)/g, '')
trimmedStack
);
}

return (
assertMatcherHint(operator, operatorName) +
'\n\n' +
chalk.reset(`Expected value ${operatorMessage(operator, negator)}`) +
chalk.reset(`Expected value ${operatorMessage(operator)}`) +
` ${printExpected(expected)}\n` +
chalk.reset(`Received:\n`) +
` ${printReceived(actual)}` +
chalk.reset(hasCustomMessage ? '\n\nMessage:\n ' + message : '') +
(diffString ? `\n\nDifference:\n\n${diffString}` : '') +
stack.replace(/AssertionError(.*)/g, '')
trimmedStack
);
}

Expand Down
7 changes: 5 additions & 2 deletions packages/jest-jasmine2/src/jasmine/Spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,14 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
/* @flow */
/* eslint-disable sort-keys */

import {AssertionError} from 'assert';

import ExpectationFailed from '../expectation_failed';

import expectationResultFactory from '../expectation_result_factory';

import assertionErrorMessage from '../assert_support';

export default function Spec(attrs: Object) {
this.resultCallback = attrs.resultCallback || function() {};
this.id = attrs.id;
Expand Down Expand Up @@ -137,8 +141,7 @@ Spec.prototype.onException = function onException(error) {
return;
}

if (error instanceof require('assert').AssertionError) {
const assertionErrorMessage = require('../assert_support').default;
Copy link
Member Author

Choose a reason for hiding this comment

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

We have lazy loading of deps anyways

if (error instanceof AssertionError) {
error = assertionErrorMessage(error, {expand: this.expand});
}

Expand Down