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

feat: explicitly mention that test is failing because done() is not being called #13847

Merged
merged 9 commits into from
Feb 2, 2023
Merged
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
### Features

- `[jest-core]` Add newlines to JSON output ([#13817](https://github.com/facebook/jest/pull/13817))
- `[jest-circus]` Added explicit mention of test failing because `done()` is not being called in error message ([#13847](https://github.com/facebook/jest/pull/13847))

### Fixes

Expand Down
8 changes: 8 additions & 0 deletions e2e/__tests__/__snapshots__/timeouts.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,11 @@ Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites."
`;

exports[`exceeds the timeout specifying that \`done\` has not been called 1`] = `
"Test Suites: 1 failed, 1 total
Tests: 1 failed, 1 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites."
`;
21 changes: 21 additions & 0 deletions e2e/__tests__/timeouts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,24 @@ test('does not exceed the command line testTimeout', () => {
expect(summary).toMatchSnapshot();
expect(exitCode).toBe(0);
});

test('exceeds the timeout specifying that `done` has not been called', () => {
writeFiles(DIR, {
'__tests__/a-banana.js': `
jest.setTimeout(20);

test('banana', (done) => {
expect(1 + 1).toBe(2);
});
`,
'package.json': '{}',
});

const {stderr, exitCode} = runJest(DIR, ['-w=1', '--ci=false']);
const {rest, summary} = extractSummary(stderr);
expect(rest).toMatch(
/(jest\.setTimeout|jasmine\.DEFAULT_TIMEOUT_INTERVAL|Exceeded timeout\.while waiting for done to be called)/,
Copy link
Member

Choose a reason for hiding this comment

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

does jasmine have the same message? I can't find it grepping for it

);
expect(summary).toMatchSnapshot();
expect(exitCode).toBe(1);
});
10 changes: 8 additions & 2 deletions packages/jest-circus/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,15 @@ export const describeBlockHasTests = (
child => child.type === 'test' || describeBlockHasTests(child),
);

const _makeTimeoutMessage = (timeout: number, isHook: boolean) =>
const _makeTimeoutMessage = (
timeout: number,
isHook: boolean,
takesDoneCallback: boolean,
) =>
`Exceeded timeout of ${formatTime(timeout)} for a ${
isHook ? 'hook' : 'test'
}${
takesDoneCallback && ' while waiting for done to be called'
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps? Not 100% sure how to make it more clear what "done" is?

Suggested change
takesDoneCallback && ' while waiting for done to be called'
takesDoneCallback && ' while waiting for `done()` to be called'

}.\nUse jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test.`;

// Global values can be overwritten by mocks or tests. We'll capture
Expand All @@ -195,7 +201,7 @@ export const callAsyncCircusFn = (

return new Promise<void>((resolve, reject) => {
timeoutID = setTimeout(
() => reject(_makeTimeoutMessage(timeout, isHook)),
() => reject(_makeTimeoutMessage(timeout, isHook, takesDoneCallback(fn))),
Copy link
Member

Choose a reason for hiding this comment

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

extract takesDoneCallback(fn) - no need to call it twice

timeout,
);

Expand Down