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

Detect open handles with done callbacks #11382

Merged
merged 13 commits into from
May 20, 2021
Merged
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
- `[jest-console]` `console.dir` now respects the second argument correctly ([#10638](https://github.com/facebook/jest/pull/10638))
- `[jest-core]` Don't report PerformanceObserver as open handle ([#11123](https://github.com/facebook/jest/pull/11123))
- `[jest-core]` Use `WeakRef` to hold timers when detecting open handles ([#11277](https://github.com/facebook/jest/pull/11277))
- `[jest-core]` Correctly detect open handles that were created in test functions using `done` callbacks ([#11382](https://github.com/facebook/jest/pull/11382))
- `[jest-diff]` [**BREAKING**] Use only named exports ([#11371](https://github.com/facebook/jest/pull/11371))
- `[jest-each]` [**BREAKING**] Ignore excess words in headings ([#8766](https://github.com/facebook/jest/pull/8766))
- `[jest-each]` Support array index with template strings ([#10763](https://github.com/facebook/jest/pull/10763))
Expand All @@ -79,6 +80,7 @@
- `[jest-haste-map]` Vendor `NodeWatcher` from `sane` ([#10919](https://github.com/facebook/jest/pull/10919))
- `[jest-jasmine2]` Fixed the issue of `beforeAll` & `afterAll` hooks getting executed even if it is inside a skipped `describe` block when it has child `tests` marked as either `only` or `todo` [#10451](https://github.com/facebook/jest/issues/10451)
- `[jest-jasmine2]` Fixed the issues of child `tests` marked with `only` or `todo` getting executed even if it is inside a skipped parent `describe` block [#10451](https://github.com/facebook/jest/issues/10451)
- `[jest-jasmine2]` Wrap all test functions so they open handles that were created in test functions using `done` callbacks can be detected ([#11382](https://github.com/facebook/jest/pull/11382))
- `[jest-reporter]` Handle empty files when reporting code coverage with V8 ([#10819](https://github.com/facebook/jest/pull/10819))
- `[jest-resolve]` Replace read-pkg-up with escalade package ([#10781](https://github.com/facebook/jest/pull/10781))
- `[jest-resolve]` Disable `jest-pnp-resolver` for Yarn 2 ([#10847](https://github.com/facebook/jest/pull/10847))
Expand Down
32 changes: 32 additions & 0 deletions e2e/__tests__/__snapshots__/detectOpenHandles.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,35 @@ Jest has detected the following 1 open handle potentially keeping Jest from exit

at Object.setTimeout (__tests__/inside.js:9:3)
`;

exports[`prints out info about open handlers from lifecycle functions with a \`done\` callback 1`] = `
Jest has detected the following 1 open handle potentially keeping Jest from exiting:

● Timeout

7 |
8 | beforeAll(done => {
> 9 | setTimeout(() => {}, 10000);
| ^
10 | done();
11 | });
12 |

at setTimeout (__tests__/in-done-lifecycle.js:9:3)
`;

exports[`prints out info about open handlers from tests with a \`done\` callback 1`] = `
Jest has detected the following 1 open handle potentially keeping Jest from exiting:

● Timeout

7 |
8 | test('something', done => {
> 9 | setTimeout(() => {}, 10000);
| ^
10 | expect(true).toBe(true);
11 | done();
12 | });

at Object.setTimeout (__tests__/in-done-function.js:9:3)
`;
32 changes: 32 additions & 0 deletions e2e/__tests__/detectOpenHandles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,35 @@ it('prints out info about open handlers from inside tests', async () => {

expect(wrap(textAfterTest)).toMatchSnapshot();
});

it('prints out info about open handlers from tests with a `done` callback', async () => {
const run = runContinuous('detect-open-handles', [
'in-done-function',
'--detectOpenHandles',
]);
await run.waitUntil(({stderr}) => stderr.includes('Jest has detected'));
const {stderr} = await run.end();
const textAfterTest = getTextAfterTest(stderr);

expect(wrap(textAfterTest)).toMatchSnapshot();
});

it('prints out info about open handlers from lifecycle functions with a `done` callback', async () => {
const run = runContinuous('detect-open-handles', [
'in-done-lifecycle',
'--detectOpenHandles',
]);
await run.waitUntil(({stderr}) => stderr.includes('Jest has detected'));
const {stderr} = await run.end();
let textAfterTest = getTextAfterTest(stderr);

// Circus and Jasmine have different contexts, leading to slightly different
// names for call stack functions. The difference shouldn't be problematic
// for users, so this normalizes them so the test works in both environments.
textAfterTest = textAfterTest.replace(
'at Object.setTimeout',
'at setTimeout',
);

expect(wrap(textAfterTest)).toMatchSnapshot();
});
12 changes: 12 additions & 0 deletions e2e/detect-open-handles/__tests__/in-done-function.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

test('something', done => {
setTimeout(() => {}, 10000);
expect(true).toBe(true);
done();
});
15 changes: 15 additions & 0 deletions e2e/detect-open-handles/__tests__/in-done-lifecycle.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

beforeAll(done => {
setTimeout(() => {}, 10000);
done();
});

test('something', () => {
expect(true).toBe(true);
});
17 changes: 17 additions & 0 deletions packages/jest-core/src/__tests__/collectHandles.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*
*/

import http from 'http';
import {PerformanceObserver} from 'perf_hooks';
import collectHandles from '../collectHandles';

Expand Down Expand Up @@ -33,4 +34,20 @@ describe('collectHandles', () => {
expect(openHandles).toHaveLength(0);
obs.disconnect();
});

it('should collect handles opened in test functions with `done` callbacks', done => {
const handleCollector = collectHandles();
const server = http.createServer((_, response) => response.end('ok'));
server.listen(0, () => {
// Collect results while server is still open.
const openHandles = handleCollector();

server.close(() => {
expect(openHandles).toContainEqual(
expect.objectContaining({message: 'TCPSERVERWRAP'}),
);
done();
});
});
});
});
35 changes: 31 additions & 4 deletions packages/jest-jasmine2/src/jasmineAsyncInstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,24 @@ function promisifyLifeCycleFunction(
return originalFn.call(env);
}

const hasDoneCallback = typeof fn === 'function' && fn.length > 0;
if (typeof fn !== 'function') {
// Pass non-functions to Jest, which throws a nice error.
return originalFn.call(env, fn, timeout);
}

const hasDoneCallback = fn.length > 0;

if (hasDoneCallback) {
// Jasmine will handle it
return originalFn.call(env, fn, timeout);
// Give the function a name so it can be detected in call stacks, but
// otherwise Jasmine will handle it.
const asyncJestLifecycleWithCallback = function (
this: Global.TestContext,
...args: Array<any>
) {
// @ts-expect-error: Support possible extra args at runtime
return fn.apply(this, args);
};
return originalFn.call(env, asyncJestLifecycleWithCallback, timeout);
}

const extraError = new Error();
Expand Down Expand Up @@ -106,10 +119,24 @@ function promisifyIt(
return spec;
}

if (typeof fn !== 'function') {
// Pass non-functions to Jest, which throws a nice error.
return originalFn.call(env, specName, fn, timeout);
}

const hasDoneCallback = fn.length > 0;

if (hasDoneCallback) {
return originalFn.call(env, specName, fn, timeout);
// Give the function a name so it can be detected in call stacks, but
// otherwise Jasmine will handle it.
const asyncJestTestWithCallback = function (
this: Global.TestContext,
...args: Array<any>
) {
// @ts-expect-error: Support possible extra args at runtime
return fn.apply(this, args);
};
return originalFn.call(env, specName, asyncJestTestWithCallback, timeout);
}

const extraError = new Error();
Expand Down