Skip to content

Commit

Permalink
fix: do not report unref-ed subprocesses as open handles (#12705)
Browse files Browse the repository at this point in the history
  • Loading branch information
SimenB authored Apr 21, 2022
1 parent 54f0ded commit 4beda2d
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
- `[jest-config]` [**BREAKING**] Add `mjs` and `cjs` to default `moduleFileExtensions` config ([#12578](https://github.com/facebook/jest/pull/12578))
- `[jest-config, jest-haste-map]` Allow searching for tests in `node_modules` by exposing `retainAllFiles` ([#11084](https://github.com/facebook/jest/pull/11084))
- `[jest-core]` [**BREAKING**] Exit with status `1` if no tests are found with `--findRelatedTests` flag ([#12487](https://github.com/facebook/jest/pull/12487))
- `[jest-core]` Do not report unref-ed subprocesses as open handles ([#12705](https://github.com/facebook/jest/pull/12705))
- `[jest-each]` `%#` is not replaced with index of the test case ([#12517](https://github.com/facebook/jest/pull/12517))
- `[jest-each]` Fixes error message with incorrect count of missing arguments ([#12464](https://github.com/facebook/jest/pull/12464))
- `[jest-environment-jsdom]` Make `jsdom` accessible to extending environments again ([#12232](https://github.com/facebook/jest/pull/12232))
Expand Down
11 changes: 11 additions & 0 deletions e2e/__tests__/detectOpenHandles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,17 @@ it('does not report timeouts using unref', () => {
expect(textAfterTest).toBe('');
});

it('does not report child_process using unref', () => {
// The test here is basically that it exits cleanly without reporting anything (does not need `until`)
const {stderr} = runJest('detect-open-handles', [
'child_process',
'--detectOpenHandles',
]);
const textAfterTest = getTextAfterTest(stderr);

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

it('prints out info about open handlers from inside tests', async () => {
const run = runContinuous('detect-open-handles', [
'inside',
Expand Down
21 changes: 21 additions & 0 deletions e2e/detect-open-handles/__tests__/child_process.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* 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.
*/

const {spawn} = require('child_process');

test('something', () => {
const subprocess = spawn(
process.argv[0],
[require.resolve('../interval-code')],
{
detached: true,
stdio: 'ignore',
},
);
subprocess.unref();
expect(true).toBe(true);
});
10 changes: 10 additions & 0 deletions e2e/detect-open-handles/interval-code.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* 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.
*/

setInterval(() => {
console.log('XX');
}, 1000);
25 changes: 10 additions & 15 deletions packages/jest-core/src/collectHandles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,24 +93,19 @@ export default function collectHandles(): HandleCollectionResult {
if (fromUser) {
let isActive: () => boolean;

if (type === 'Timeout' || type === 'Immediate') {
// Timer that supports hasRef (Node v11+)
if ('hasRef' in resource) {
if (hasWeakRef) {
// @ts-expect-error: doesn't exist in v12 typings
const ref = new WeakRef(resource);
isActive = () => {
return ref.deref()?.hasRef() ?? false;
};
} else {
isActive = resource.hasRef.bind(resource);
}
// Handle that supports hasRef
if ('hasRef' in resource) {
if (hasWeakRef) {
// @ts-expect-error: doesn't exist in v12 typings
const ref = new WeakRef(resource);
isActive = () => {
return ref.deref()?.hasRef() ?? false;
};
} else {
// Timer that doesn't support hasRef
isActive = alwaysActive;
isActive = resource.hasRef.bind(resource);
}
} else {
// Any other async resource
// Handle that doesn't support hasRef
isActive = alwaysActive;
}

Expand Down

0 comments on commit 4beda2d

Please sign in to comment.