Skip to content

Commit

Permalink
Don't include unref'd timers in --detectOpenHandles results (#8941)
Browse files Browse the repository at this point in the history
  • Loading branch information
pimterry authored and SimenB committed Sep 13, 2019
1 parent 5a45171 commit 6c0a16a
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -25,6 +25,7 @@
### Fixes

- `[expect]` Display expectedDiff more carefully in toBeCloseTo ([#8389](https://github.com/facebook/jest/pull/8389))
- `[jest-core]` Don't include unref'd timers in --detectOpenHandles results ([#8941](https://github.com/facebook/jest/pull/8941))
- `[jest-diff]` Do not inverse format if line consists of one change ([#8903](https://github.com/facebook/jest/pull/8903))
- `[jest-fake-timers]` `getTimerCount` will not include cancelled immediates ([#8764](https://github.com/facebook/jest/pull/8764))
- `[jest-leak-detector]` [**BREAKING**] Use `weak-napi` instead of `weak` package ([#8686](https://github.com/facebook/jest/pull/8686))
Expand Down
14 changes: 14 additions & 0 deletions e2e/__tests__/detectOpenHandles.ts
Expand Up @@ -7,6 +7,7 @@

import {wrap} from 'jest-snapshot-serializer-raw';
import runJest, {until} from '../runJest';
import {onNodeVersions} from '../../packages/test-utils';

try {
require('async_hooks');
Expand Down Expand Up @@ -69,6 +70,19 @@ it('does not report promises', () => {
expect(textAfterTest).toBe('');
});

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

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

it('prints out info about open handlers from inside tests', async () => {
const {stderr} = await until(
'detect-open-handles',
Expand Down
12 changes: 12 additions & 0 deletions e2e/detect-open-handles/__tests__/unref.js
@@ -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', () => {
const timeout = setTimeout(() => {}, 30000);
timeout.unref();
expect(true).toBe(true);
});
37 changes: 33 additions & 4 deletions packages/jest-core/src/collectHandles.ts
Expand Up @@ -34,10 +34,15 @@ function stackIsFromUser(stack: string) {
return false;
}

const alwaysActive = () => true;

// 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<number, Error> = new Map();
const activeHandles: Map<
number,
{error: Error; isActive: () => boolean}
> = new Map();

let hook: AsyncHook;

Expand All @@ -47,14 +52,34 @@ export default function collectHandles(): () => Array<Error> {
destroy(asyncId) {
activeHandles.delete(asyncId);
},
init: function initHook(asyncId, type) {
init: function initHook(
asyncId,
type,
_triggerAsyncId,
resource: {} | NodeJS.Timeout,
) {
if (type === 'PROMISE' || type === 'TIMERWRAP') {
return;
}
const error = new ErrorWithStack(type, initHook);

if (stackIsFromUser(error.stack || '')) {
activeHandles.set(asyncId, error);
let isActive: () => boolean;

if (type === 'Timeout' || type === 'Immediate') {
if ('hasRef' in resource) {
// Timer that supports hasRef (Node v11+)
isActive = resource.hasRef.bind(resource);
} else {
// Timer that doesn't support hasRef
isActive = alwaysActive;
}
} else {
// Any other async resource
isActive = alwaysActive;
}

activeHandles.set(asyncId, {error, isActive});
}
},
});
Expand All @@ -74,7 +99,11 @@ export default function collectHandles(): () => Array<Error> {
return () => {
hook.disable();

const result = Array.from(activeHandles.values());
// Get errors for every async resource still referenced at this moment
const result = Array.from(activeHandles.values())
.filter(({isActive}) => isActive())
.map(({error}) => error);

activeHandles.clear();
return result;
};
Expand Down
12 changes: 12 additions & 0 deletions packages/test-utils/src/ConditionalTest.ts
Expand Up @@ -7,6 +7,8 @@

/* eslint-disable jest/no-focused-tests */

import semver = require('semver');

export function isJestCircusRun() {
return process.env.JEST_CIRCUS === '1';
}
Expand Down Expand Up @@ -34,3 +36,13 @@ export function skipSuiteOnWindows() {
});
}
}

export function onNodeVersions(versionRange: string, testBody: () => void) {
if (!semver.satisfies(process.versions.node, versionRange)) {
describe.skip(`[SKIP] tests that require node ${versionRange}`, () => {
testBody();
});
} else {
testBody();
}
}
1 change: 1 addition & 0 deletions packages/test-utils/src/index.ts
Expand Up @@ -10,4 +10,5 @@ export {
skipSuiteOnJasmine,
skipSuiteOnJestCircus,
skipSuiteOnWindows,
onNodeVersions,
} from './ConditionalTest';

0 comments on commit 6c0a16a

Please sign in to comment.