Don't include unref'd timers in --detectOpenHandles results (#8941)

Author: pimterry

CHANGELOG.md

@@ -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))

e2e/__tests__/detectOpenHandles.ts

@@ -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');
@@ -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',

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);
+});

packages/jest-core/src/collectHandles.ts

@@ -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;
 
@@ -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});
         }
       },
     });
@@ -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;
   };

packages/test-utils/src/ConditionalTest.ts

@@ -7,6 +7,8 @@
 
 /* eslint-disable jest/no-focused-tests */
 
+import semver = require('semver');
+
 export function isJestCircusRun() {
   return process.env.JEST_CIRCUS === '1';
 }
@@ -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();
+  }
+}

packages/test-utils/src/index.ts

@@ -10,4 +10,5 @@ export {
   skipSuiteOnJasmine,
   skipSuiteOnJestCircus,
   skipSuiteOnWindows,
+  onNodeVersions,
 } from './ConditionalTest';