Detect open handles with done callbacks (#11382)

Author: Mr0grog

CHANGELOG.md

@@ -68,6 +68,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))
@@ -82,6 +83,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))

e2e/__tests__/__snapshots__/detectOpenHandles.ts.snap

@@ -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)
+`;

e2e/__tests__/detectOpenHandles.ts

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

e2e/detect-open-handles/__tests__/in-done-function.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', done => {
+  setTimeout(() => {}, 10000);
+  expect(true).toBe(true);
+  done();
+});

e2e/detect-open-handles/__tests__/in-done-lifecycle.js

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

packages/jest-core/src/__tests__/collectHandles.test.js

@@ -6,6 +6,7 @@
  *
  */
 
+import http from 'http';
 import {PerformanceObserver} from 'perf_hooks';
 import collectHandles from '../collectHandles';
 
@@ -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();
+      });
+    });
+  });
 });

packages/jest-jasmine2/src/jasmineAsyncInstall.ts

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