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: handle open handles from inside tests #6263

Merged
merged 6 commits into from
Aug 9, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
- `[jest-cli]` Improve the message when running coverage while there are no files matching global threshold ([#6334](https://github.com/facebook/jest/pull/6334))
- `[jest-snapshot]` Correctly merge property matchers with the rest of the snapshot in `toMatchSnapshot`. ([#6528](https://github.com/facebook/jest/pull/6528))
- `[jest-snapshot]` Add error messages for invalid property matchers. ([#6528](https://github.com/facebook/jest/pull/6528))
- `[jest-cli]` Show open handles from inside test files as well ([#6263](https://github.com/facebook/jest/pull/6263))

### Chore & Maintenance

Expand Down
15 changes: 15 additions & 0 deletions e2e/__tests__/__snapshots__/detect_open_handles.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,18 @@ exports[`prints message about flag on slow tests 1`] = `

This usually means that there are asynchronous operations that weren't stopped in your tests. Consider running Jest with \`--detectOpenHandles\` to troubleshoot this issue."
`;

exports[`prints out info about open handlers from inside tests 1`] = `
"Jest has detected the following 1 open handle potentially keeping Jest from exiting:

● Timeout

1 | test('something', () => {
> 2 | setTimeout(() => {}, 30000);
| ^
3 | expect(true).toBe(true);
4 | });
5 |

at Object.<anonymous>.test (__tests__/inside.js:2:3)"
`;
11 changes: 11 additions & 0 deletions e2e/__tests__/detect_open_handles.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,14 @@ it('does not report promises', () => {

expect(textAfterTest).toBe('');
});

it('prints out info about open handlers from inside tests', async () => {
const {stderr} = await runJest.until(
'detect-open-handles',
['inside', '--detectOpenHandles'],
'Jest has detected',
);
const textAfterTest = getTextAfterTest(stderr);

expect(textAfterTest).toMatchSnapshot();
});
4 changes: 4 additions & 0 deletions e2e/detect-open-handles/__tests__/inside.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
test('something', () => {
setTimeout(() => {}, 30000);
expect(true).toBe(true);
});
22 changes: 11 additions & 11 deletions packages/jest-circus/src/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import type {

import {getState, dispatch} from './state';
import {
callAsyncFn,
callAsyncCircusFn,
getAllHooksForDescribe,
getEachHooksForTest,
getTestID,
Expand All @@ -44,7 +44,7 @@ const _runTestsForDescribeBlock = async (describeBlock: DescribeBlock) => {
const {beforeAll, afterAll} = getAllHooksForDescribe(describeBlock);

for (const hook of beforeAll) {
await _callHook({describeBlock, hook});
await _callCircusHook({describeBlock, hook});
}

// Tests that fail and are retried we run after other tests
Expand Down Expand Up @@ -77,7 +77,7 @@ const _runTestsForDescribeBlock = async (describeBlock: DescribeBlock) => {
}

for (const hook of afterAll) {
await _callHook({describeBlock, hook});
await _callCircusHook({describeBlock, hook});
}
dispatch({describeBlock, name: 'run_describe_finish'});
};
Expand Down Expand Up @@ -105,13 +105,13 @@ const _runTest = async (test: TestEntry): Promise<void> => {
// hooks after that.
break;
}
await _callHook({hook, test, testContext});
await _callCircusHook({hook, test, testContext});
}

await _callTest(test, testContext);
await _callCircusTest(test, testContext);

for (const hook of afterEach) {
await _callHook({hook, test, testContext});
await _callCircusHook({hook, test, testContext});
}

// `afterAll` hooks should not affect test status (pass or fail), because if
Expand All @@ -120,7 +120,7 @@ const _runTest = async (test: TestEntry): Promise<void> => {
dispatch({name: 'test_done', test});
};

const _callHook = ({
const _callCircusHook = ({
hook,
test,
describeBlock,
Expand All @@ -133,14 +133,14 @@ const _callHook = ({
}): Promise<mixed> => {
dispatch({hook, name: 'hook_start'});
const timeout = hook.timeout || getState().testTimeout;
return callAsyncFn(hook.fn, testContext, {isHook: true, timeout})
return callAsyncCircusFn(hook.fn, testContext, {isHook: true, timeout})
.then(() => dispatch({describeBlock, hook, name: 'hook_success', test}))
.catch(error =>
dispatch({describeBlock, error, hook, name: 'hook_failure', test}),
);
};

const _callTest = async (
const _callCircusTest = (
test: TestEntry,
testContext: TestContext,
): Promise<void> => {
Expand All @@ -150,10 +150,10 @@ const _callTest = async (

if (test.errors.length) {
// We don't run the test if there's already an error in before hooks.
return;
return Promise.resolve();
}

await callAsyncFn(test.fn, testContext, {isHook: false, timeout})
return callAsyncCircusFn(test.fn, testContext, {isHook: false, timeout})
.then(() => dispatch({name: 'test_fn_success', test}))
.catch(error => dispatch({error, name: 'test_fn_failure', test}));
};
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-circus/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ const _makeTimeoutMessage = (timeout, isHook) =>
// the original values in the variables before we require any files.
const {setTimeout, clearTimeout} = global;

export const callAsyncFn = (
export const callAsyncCircusFn = (
fn: AsyncFn,
testContext: ?TestContext,
{isHook, timeout}: {isHook?: ?boolean, timeout: number},
Expand Down
6 changes: 4 additions & 2 deletions packages/jest-cli/src/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,14 @@ export const runCLI = async (
const {openHandles} = results;

if (openHandles && openHandles.length) {
const openHandlesString = pluralize('open handle', openHandles.length, 's');
const formatted = formatHandleErrors(openHandles, configs[0]);

const openHandlesString = pluralize('open handle', formatted.length, 's');

const message =
chalk.red(
`\nJest has detected the following ${openHandlesString} potentially keeping Jest from exiting:\n\n`,
) + formatHandleErrors(openHandles, configs[0]).join('\n\n');
) + formatted.join('\n\n');

console.error(message);
}
Expand Down
57 changes: 53 additions & 4 deletions packages/jest-cli/src/collectHandles.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,37 @@
import type {ProjectConfig} from 'types/Config';

import {formatExecError} from 'jest-message-util';
import stripAnsi from 'strip-ansi';

function stackIsFromUser(stack) {
// Either the test file, or something required by it
if (stack.includes('Runtime.requireModule')) {
return true;
}

// jest-jasmine it or describe call
if (stack.includes('asyncJestTest') || stack.includes('asyncJestLifecycle')) {
return true;
}

// An async function call from within circus
if (stack.includes('callAsyncCircusFn')) {
// jest-circus it or describe call
return (
stack.includes('_callCircusTest') || stack.includes('_callCircusHook')
);
}

return false;
}

// Inspired by https://github.com/mafintosh/why-is-node-running/blob/master/index.js
// Extracted as we want to format the result ourselves
export default function collectHandles(): () => Array<Error> {
const activeHandles: Map<string, Error> = new Map();

function initHook(asyncId, type) {
if (type === 'PROMISE') {
if (type === 'PROMISE' || type === 'TIMERWRAP') {
return;
}
const error = new Error(type);
Expand All @@ -26,7 +49,7 @@ export default function collectHandles(): () => Array<Error> {
Error.captureStackTrace(error, initHook);
}

if (error.stack.includes('Runtime.requireModule')) {
if (stackIsFromUser(error.stack)) {
activeHandles.set(asyncId, error);
}
}
Expand Down Expand Up @@ -68,7 +91,33 @@ export function formatHandleErrors(
errors: Array<Error>,
config: ProjectConfig,
): Array<string> {
return errors.map(err =>
formatExecError(err, config, {noStackTrace: false}, undefined, true),
const stacks = new Set();

return (
errors
.map(err =>
formatExecError(err, config, {noStackTrace: false}, undefined, true),
)
// E.g. timeouts might give multiple traces to the same line of code
// This hairy filtering tries to remove entries with duplicate stack traces
.filter(handle => {
const ansiFree: string = stripAnsi(handle);

const match = ansiFree.match(/\s+at(.*)/);

if (!match || match.length < 2) {
return true;
}

const stack = ansiFree.substr(ansiFree.indexOf(match[1])).trim();

if (stacks.has(stack)) {
return false;
}

stacks.add(stack);

return true;
})
);
}
8 changes: 4 additions & 4 deletions packages/jest-jasmine2/src/jasmine_async.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ function promisifyLifeCycleFunction(originalFn, env) {

// We make *all* functions async and run `done` right away if they
// didn't return a promise.
const asyncFn = function(done) {
const asyncJestLifecycle = function(done) {
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 seemed easiest 🤷‍♂️

const wrappedFn = isGeneratorFn(fn) ? co.wrap(fn) : fn;
const returnValue = wrappedFn.call({});

Expand All @@ -57,7 +57,7 @@ function promisifyLifeCycleFunction(originalFn, env) {
}
};

return originalFn.call(env, asyncFn, timeout);
return originalFn.call(env, asyncJestLifecycle, timeout);
};
}

Expand All @@ -79,7 +79,7 @@ function promisifyIt(originalFn, env) {

const extraError = new Error();

const asyncFn = function(done) {
const asyncJestTest = function(done) {
const wrappedFn = isGeneratorFn(fn) ? co.wrap(fn) : fn;
const returnValue = wrappedFn.call({});

Expand All @@ -103,7 +103,7 @@ function promisifyIt(originalFn, env) {
}
};

return originalFn.call(env, specName, asyncFn, timeout);
return originalFn.call(env, specName, asyncJestTest, timeout);
};
}

Expand Down