diff --git a/CHANGELOG.md b/CHANGELOG.md index ff406166c692..db63ad8a0581 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/e2e/__tests__/__snapshots__/detect_open_handles.js.snap b/e2e/__tests__/__snapshots__/detect_open_handles.js.snap index 10de6b087412..994c7f7d38b0 100644 --- a/e2e/__tests__/__snapshots__/detect_open_handles.js.snap +++ b/e2e/__tests__/__snapshots__/detect_open_handles.js.snap @@ -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..test (__tests__/inside.js:2:3)" +`; diff --git a/e2e/__tests__/detect_open_handles.js b/e2e/__tests__/detect_open_handles.js index 51e4059d7084..3e0081326531 100644 --- a/e2e/__tests__/detect_open_handles.js +++ b/e2e/__tests__/detect_open_handles.js @@ -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(); +}); diff --git a/e2e/detect-open-handles/__tests__/inside.js b/e2e/detect-open-handles/__tests__/inside.js new file mode 100644 index 000000000000..5b1c0d5b3c1d --- /dev/null +++ b/e2e/detect-open-handles/__tests__/inside.js @@ -0,0 +1,4 @@ +test('something', () => { + setTimeout(() => {}, 30000); + expect(true).toBe(true); +}); diff --git a/packages/jest-circus/src/run.js b/packages/jest-circus/src/run.js index 1a05d7e45834..bf6bae4f2683 100644 --- a/packages/jest-circus/src/run.js +++ b/packages/jest-circus/src/run.js @@ -17,7 +17,7 @@ import type { import {getState, dispatch} from './state'; import { - callAsyncFn, + callAsyncCircusFn, getAllHooksForDescribe, getEachHooksForTest, getTestID, @@ -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 @@ -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'}); }; @@ -105,13 +105,13 @@ const _runTest = async (test: TestEntry): Promise => { // 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 @@ -120,7 +120,7 @@ const _runTest = async (test: TestEntry): Promise => { dispatch({name: 'test_done', test}); }; -const _callHook = ({ +const _callCircusHook = ({ hook, test, describeBlock, @@ -133,14 +133,14 @@ const _callHook = ({ }): Promise => { 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 => { @@ -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})); }; diff --git a/packages/jest-circus/src/utils.js b/packages/jest-circus/src/utils.js index 96763d26d345..2f0a44af3c37 100644 --- a/packages/jest-circus/src/utils.js +++ b/packages/jest-circus/src/utils.js @@ -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}, diff --git a/packages/jest-cli/src/cli/index.js b/packages/jest-cli/src/cli/index.js index 13f8af474795..bba73c181171 100644 --- a/packages/jest-cli/src/cli/index.js +++ b/packages/jest-cli/src/cli/index.js @@ -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); } diff --git a/packages/jest-cli/src/collectHandles.js b/packages/jest-cli/src/collectHandles.js index f0fc12144bc2..51e07c502aef 100644 --- a/packages/jest-cli/src/collectHandles.js +++ b/packages/jest-cli/src/collectHandles.js @@ -10,6 +10,29 @@ 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 @@ -17,7 +40,7 @@ export default function collectHandles(): () => Array { const activeHandles: Map = new Map(); function initHook(asyncId, type) { - if (type === 'PROMISE') { + if (type === 'PROMISE' || type === 'TIMERWRAP') { return; } const error = new Error(type); @@ -26,7 +49,7 @@ export default function collectHandles(): () => Array { Error.captureStackTrace(error, initHook); } - if (error.stack.includes('Runtime.requireModule')) { + if (stackIsFromUser(error.stack)) { activeHandles.set(asyncId, error); } } @@ -68,7 +91,33 @@ export function formatHandleErrors( errors: Array, config: ProjectConfig, ): Array { - 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; + }) ); } diff --git a/packages/jest-jasmine2/src/jasmine_async.js b/packages/jest-jasmine2/src/jasmine_async.js index 01b5ec27e87d..d7bb10c481b0 100644 --- a/packages/jest-jasmine2/src/jasmine_async.js +++ b/packages/jest-jasmine2/src/jasmine_async.js @@ -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) { const wrappedFn = isGeneratorFn(fn) ? co.wrap(fn) : fn; const returnValue = wrappedFn.call({}); @@ -57,7 +57,7 @@ function promisifyLifeCycleFunction(originalFn, env) { } }; - return originalFn.call(env, asyncFn, timeout); + return originalFn.call(env, asyncJestLifecycle, timeout); }; } @@ -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({}); @@ -103,7 +103,7 @@ function promisifyIt(originalFn, env) { } }; - return originalFn.call(env, specName, asyncFn, timeout); + return originalFn.call(env, specName, asyncJestTest, timeout); }; }