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

add stack trace on expect.(has)Assertions #5997

Merged
merged 3 commits into from
Apr 17, 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 @@ -63,6 +63,8 @@

### Fixes

* `[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
after the test environment is torn down
([#5888](https://github.com/facebook/jest/pull/5888))
Expand Down
52 changes: 51 additions & 1 deletion integration-tests/__tests__/__snapshots__/failures.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -36,37 +36,87 @@ exports[`not throwing Error objects 4`] = `
✕ throws on assertion
.hasAssertions()
✕ throws when there are not assertions

● .assertions() › throws

expect(received).toBeTruthy()

Received: false

11 | const throws = () => {
12 | expect.assertions(2);
> 13 | expect(false).toBeTruthy();
| ^
14 | };
15 | const redeclare = () => {
16 | expect.assertions(1);

at __tests__/assertion_count.test.js:13:17

● .assertions() › throws

expect.assertions(2)

Expected two assertions to be called but received one assertion call.

10 |
11 | const throws = () => {
> 12 | expect.assertions(2);
| ^
13 | expect(false).toBeTruthy();
14 | };
15 | const redeclare = () => {

at __tests__/assertion_count.test.js:12:10

● .assertions() › throws on redeclare of assertion count

expect(received).toBeTruthy()

Received: false

15 | const redeclare = () => {
16 | expect.assertions(1);
> 17 | expect(false).toBeTruthy();
| ^
18 | expect.assertions(2);
19 | };
20 |
20 |

at __tests__/assertion_count.test.js:17:17

● .assertions() › throws on assertion

expect.assertions(0)

Expected zero assertions to be called but received one assertion call.

20 |
21 | const noAssertions = () => {
> 22 | expect.assertions(0);
| ^
23 | expect(true).toBeTruthy();
24 | };
25 |

at __tests__/assertion_count.test.js:22:10

● .hasAssertions() › throws when there are not assertions

expect.hasAssertions()

Expected at least one assertion to be called but received none.

25 |
26 | const hasNoAssertions = () => {
> 27 | expect.hasAssertions();
| ^
28 | };
29 |
30 | describe('.assertions()', () => {

at __tests__/assertion_count.test.js:27:10

"
`;

Expand Down
26 changes: 1 addition & 25 deletions integration-tests/__tests__/failures.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,6 @@ const normalizeDots = text => text.replace(/\.{1,}$/gm, '.');

SkipOnWindows.suite();

const cleanupStackTrace = stderr => {
Copy link
Member Author

Choose a reason for hiding this comment

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

Unexpected that I was able to remove this 😛

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

oh.. i completely forgot about it 😀
i'm glad i didn't write a TODO: remove after it lands comment that dated early 2016 or something haha

const STACK_REGEXP = /^.*at.*(setup-jest-globals|extractExpectedAssertionsErrors).*\n/gm;
if (!STACK_REGEXP.test(stderr)) {
throw new Error(
`
This function is used to remove inconsistent stack traces between
jest-jasmine2 and jest-circus. If you see this error, that means the
stack traces are no longer inconsistent and this function can be
safely removed.

output:
${stderr}
`,
);
}

return (
stderr
.replace(STACK_REGEXP, '')
// Also remove trailing whitespace.
.replace(/\s+$/gm, '')
);
};

test('not throwing Error objects', () => {
let stderr;
stderr = runJest(dir, ['throw_number.test.js']).stderr;
Expand All @@ -51,7 +27,7 @@ test('not throwing Error objects', () => {
stderr = runJest(dir, ['throw_object.test.js']).stderr;
expect(extractSummary(stderr).rest).toMatchSnapshot();
stderr = runJest(dir, ['assertion_count.test.js']).stderr;
expect(extractSummary(cleanupStackTrace(stderr)).rest).toMatchSnapshot();
expect(extractSummary(stderr).rest).toMatchSnapshot();
stderr = runJest(dir, ['during_tests.test.js']).stderr;
expect(extractSummary(stderr).rest).toMatchSnapshot();
});
Expand Down
30 changes: 17 additions & 13 deletions packages/expect/src/extract_expected_assertions_errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@ const resetAssertionsLocalState = () => {
};

// Create and format all errors related to the mismatched number of `expect`
// calls and reset the matchers state.
// calls and reset the matcher's state.
const extractExpectedAssertionsErrors = () => {
const result = [];
const {
assertionCalls,
expectedAssertionsNumber,
expectedAssertionsNumberError,
isExpectingAssertions,
isExpectingAssertionsError,
} = getState();

resetAssertionsLocalState();
Expand All @@ -43,34 +45,36 @@ const extractExpectedAssertionsErrors = () => {
const numOfAssertionsExpected = EXPECTED_COLOR(
pluralize('assertion', expectedAssertionsNumber),
);
const error = new Error(

expectedAssertionsNumberError.message =
matcherHint('.assertions', '', String(expectedAssertionsNumber), {
isDirectExpectCall: true,
}) +
'\n\n' +
`Expected ${numOfAssertionsExpected} to be called but received ` +
RECEIVED_COLOR(pluralize('assertion call', assertionCalls || 0)) +
'.',
);
'\n\n' +
`Expected ${numOfAssertionsExpected} to be called but received ` +
RECEIVED_COLOR(pluralize('assertion call', assertionCalls || 0)) +
'.';

result.push({
actual: assertionCalls,
error,
error: expectedAssertionsNumberError,
expected: expectedAssertionsNumber,
});
}
if (isExpectingAssertions && assertionCalls === 0) {
const expected = EXPECTED_COLOR('at least one assertion');
const received = RECEIVED_COLOR('received none');
const error = new Error(

isExpectingAssertionsError.message =
matcherHint('.hasAssertions', '', '', {
isDirectExpectCall: true,
}) +
'\n\n' +
`Expected ${expected} to be called but ${received}.`,
);
'\n\n' +
`Expected ${expected} to be called but ${received}.`;

result.push({
actual: 'none',
error,
error: isExpectingAssertionsError,
expected: 'at least one',
});
}
Expand Down
29 changes: 22 additions & 7 deletions packages/expect/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,19 +314,34 @@ const _validateResult = result => {
}
};

function assertions(expected: number) {
const error = new Error();
if (Error.captureStackTrace) {
Error.captureStackTrace(error, assertions);
}

getState().expectedAssertionsNumber = expected;
getState().expectedAssertionsNumberError = error;
}
function hasAssertions(...args) {
Copy link
Member Author

Choose a reason for hiding this comment

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

has to be a flow bug that I was only yelled at after extracting this (complaining that hasAssertions takes args when the type says no args)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious what caused this (would be great to setup a repro for Flow folks). But ...args seems ok here.

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, unable to reproduce in their repl...

const error = new Error();
if (Error.captureStackTrace) {
Error.captureStackTrace(error, hasAssertions);
}

utils.ensureNoExpected(args[0], '.hasAssertions');
getState().isExpectingAssertions = true;
getState().isExpectingAssertionsError = error;
}

// add default jest matchers
setMatchers(matchers, true, expect);
setMatchers(spyMatchers, true, expect);
setMatchers(toThrowMatchers, true, expect);

expect.addSnapshotSerializer = () => void 0;
expect.assertions = (expected: number) => {
getState().expectedAssertionsNumber = expected;
};
expect.hasAssertions = (expected: any) => {
utils.ensureNoExpected(expected, '.hasAssertions');
getState().isExpectingAssertions = true;
};
expect.assertions = assertions;
expect.hasAssertions = hasAssertions;
expect.getState = getState;
expect.setState = setState;
expect.extractExpectedAssertionsErrors = extractExpectedAssertionsErrors;
Expand Down