feat: fail tests when multiple done() calls are made

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### Features
 
+- `[jest-circus]` [**BREAKING**] Fail tests when multiple `done()` calls are made ([#10624](https://github.com/facebook/jest/pull/10624))
 - `[jest-config]` [**BREAKING**] Default to Node testing environment instead of browser (JSDOM) ([#9874](https://github.com/facebook/jest/pull/9874))
 - `[jest-config]` [**BREAKING**] Use `jest-circus` as default test runner ([#10686](https://github.com/facebook/jest/pull/10686))
 - `[jest-config, jest-runtime]` Support ESM for files other than `.js` and `.mjs` ([#10823](https://github.com/facebook/jest/pull/10823))

e2e/__tests__/__snapshots__/callDoneTwice.test.ts.snap

@@ -0,0 +1,19 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`\`done()\` should not be called more than once 1`] = `
+FAIL __tests__/index.test.js
+  ✕ \`done()\` should not be called more than once
+
+  ● \`done()\` should not be called more than once
+
+    Expected done to be called once, but it was called multiple times.
+
+       7 | test('\`done()\` should not be called more than once', done => {
+       8 |   done();
+    >  9 |   done();
+         |   ^
+      10 | });
+      11 | 
+
+      at Object.done (__tests__/index.test.js:9:3)
+`;

e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap

@@ -16,7 +16,7 @@ FAIL __tests__/asyncDefinition.test.js
       14 |   });
       15 | });
 
-      at eventHandler (../../packages/jest-circus/build/eventHandler.js:144:11)
+      at eventHandler (../../packages/jest-circus/build/eventHandler.js:145:11)
       at test (__tests__/asyncDefinition.test.js:12:5)
 
   ● Test suite failed to run
@@ -46,7 +46,7 @@ FAIL __tests__/asyncDefinition.test.js
       20 | });
       21 | 
 
-      at eventHandler (../../packages/jest-circus/build/eventHandler.js:144:11)
+      at eventHandler (../../packages/jest-circus/build/eventHandler.js:145:11)
       at test (__tests__/asyncDefinition.test.js:18:3)
 
   ● Test suite failed to run

e2e/__tests__/callDoneTwice.test.ts

@@ -0,0 +1,18 @@
+/**
+ * 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.
+ */
+import wrap from 'jest-snapshot-serializer-raw';
+import {skipSuiteOnJasmine} from '@jest/test-utils';
+import {extractSummary} from '../Utils';
+import runJest from '../runJest';
+
+skipSuiteOnJasmine();
+test('`done()` should not be called more than once', () => {
+  const {exitCode, stderr} = runJest('call-done-twice');
+  const {rest} = extractSummary(stderr);
+  expect(wrap(rest)).toMatchSnapshot();
+  expect(exitCode).toBe(1);
+});

e2e/call-done-twice/__tests__/index.test.js

@@ -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.
+ */
+test('`done()` should not be called more than once', done => {
+  done();
+  done();
+});

e2e/call-done-twice/package.json

@@ -0,0 +1,5 @@
+{
+    "jest": {
+      "testEnvironment": "node"
+    }
+}

packages/jest-circus/src/eventHandler.ts

@@ -113,7 +113,14 @@ const eventHandler: Circus.EventHandler = (
       }
       const parent = currentDescribeBlock;
 
-      currentDescribeBlock.hooks.push({asyncError, fn, parent, timeout, type});
+      currentDescribeBlock.hooks.push({
+        asyncError,
+        fn,
+        parent,
+        seenDone: false,
+        timeout,
+        type,
+      });
       break;
     }
     case 'add_test': {

packages/jest-circus/src/utils.ts

@@ -70,6 +70,7 @@ export const makeTest = (
   mode,
   name: convertDescriptorToString(name),
   parent,
+  seenDone: false,
   startedAt: null,
   status: null,
   timeout,
@@ -189,9 +190,19 @@ export const callAsyncCircusFn = (
     // soon as `done` called.
     if (takesDoneCallback(fn)) {
       let returnedValue: unknown = undefined;
+
       const done = (reason?: Error | string): void => {
+        if (!completed && testOrHook.seenDone) {
+          throw new ErrorWithStack(
+            'Expected done to be called once, but it was called multiple times.',
+            done,
+          );
+        } else {
+          testOrHook.seenDone = true;
+        }
         // We need to keep a stack here before the promise tick
         const errorAtDone = new ErrorWithStack(undefined, done);
+
         // Use `Promise.resolve` to allow the event loop to go a single tick in case `done` is called synchronously
         Promise.resolve().then(() => {
           if (returnedValue !== undefined) {

packages/jest-core/src/__tests__/SearchSource.test.ts

@@ -415,7 +415,7 @@ describe('SearchSource', () => {
     );
     const rootPath = path.join(rootDir, 'root.js');
 
-    beforeEach(done => {
+    beforeEach(async () => {
       const {options: config} = normalize(
         {
           haste: {
@@ -435,12 +435,11 @@ describe('SearchSource', () => {
         },
         {} as Config.Argv,
       );
-      Runtime.createContext(config, {maxWorkers, watchman: false}).then(
-        context => {
-          searchSource = new SearchSource(context);
-          done();
-        },
-      );
+      const context = await Runtime.createContext(config, {
+        maxWorkers,
+        watchman: false,
+      });
+      searchSource = new SearchSource(context);
     });
 
     it('makes sure a file is related to itself', () => {
@@ -477,7 +476,7 @@ describe('SearchSource', () => {
   });
 
   describe('findRelatedTestsFromPattern', () => {
-    beforeEach(done => {
+    beforeEach(async () => {
       const {options: config} = normalize(
         {
           moduleFileExtensions: ['js', 'jsx', 'foobar'],
@@ -487,12 +486,11 @@ describe('SearchSource', () => {
         },
         {} as Config.Argv,
       );
-      Runtime.createContext(config, {maxWorkers, watchman: false}).then(
-        context => {
-          searchSource = new SearchSource(context);
-          done();
-        },
-      );
+      const context = await Runtime.createContext(config, {
+        maxWorkers,
+        watchman: false,
+      });
+      searchSource = new SearchSource(context);
     });
 
     it('returns empty search result for empty input', () => {

packages/jest-types/src/Circus.ts

@@ -28,6 +28,7 @@ export type Hook = {
   fn: HookFn;
   type: HookType;
   parent: DescribeBlock;
+  seenDone: boolean;
   timeout: number | undefined | null;
 };
 
@@ -238,6 +239,7 @@ export type TestEntry = {
   parent: DescribeBlock;
   startedAt?: number | null;
   duration?: number | null;
+  seenDone: boolean;
   status?: TestStatus | null; // whether the test has been skipped or run already
   timeout?: number;
 };