feat(globals): make test return values stricter

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 - `[jest-circus, jest-config, jest-runtime]` Add new `injectGlobals` config and CLI option to disable injecting global variables into the runtime ([#10484](https://github.com/facebook/jest/pull/10484))
 - `[jest-each]` Fixes `.each` type to always be callable ([#10447](https://github.com/facebook/jest/pull/10447))
+- `[jest-globals]` [**BREAKING**] Disallow return values other than a `Promise` from hooks and tests
+- `[jest-globals]` [**BREAKING**] Disallow mixing a done callback and returning a `Promise` from hooks and tests
 
 ### Fixes
 

packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts

@@ -81,7 +81,7 @@ export const initialize = async ({
   globalsObject.test.concurrent = (test => {
     const concurrent = (
       testName: string,
-      testFn: () => Promise<unknown>,
+      testFn: Global.ConcurrentTestFn,
       timeout?: number,
     ) => {
       // For concurrent tests we first run the function that returns promise, and then register a
@@ -96,7 +96,7 @@ export const initialize = async ({
 
     const only = (
       testName: string,
-      testFn: () => Promise<unknown>,
+      testFn: Global.ConcurrentTestFn,
       timeout?: number,
     ) => {
       const promise = mutex(() => testFn());

packages/jest-circus/src/utils.ts

@@ -5,7 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import type {Circus} from '@jest/types';
+import type {Circus, Global} from '@jest/types';
 import {convertDescriptorToString, formatTime} from 'jest-util';
 import isGeneratorFn from 'is-generator-fn';
 import co from 'co';
@@ -17,6 +17,10 @@ import {ROOT_DESCRIBE_BLOCK_NAME, getState} from './state';
 
 const stackUtils = new StackUtils({cwd: 'A path that does not exist'});
 
+function takesDoneCallback(fn: Circus.AsyncFn): fn is Global.DoneTakingTestFn {
+  return fn.length > 0;
+}
+
 export const makeDescribe = (
   name: Circus.BlockName,
   parent?: Circus.DescribeBlock,
@@ -172,7 +176,7 @@ export const callAsyncCircusFn = (
 
     // If this fn accepts `done` callback we return a promise that fulfills as
     // soon as `done` called.
-    if (fn.length) {
+    if (takesDoneCallback(fn)) {
       let returnedValue: unknown = undefined;
       const done = (reason?: Error | string): void => {
         // We need to keep a stack here before the promise tick

packages/jest-jasmine2/src/jasmineAsyncInstall.ts

@@ -165,7 +165,7 @@ function makeConcurrent(
 ): Global.ItConcurrentBase {
   const concurrentFn = function (
     specName: string,
-    fn: Global.TestFn,
+    fn: Global.ConcurrentTestFn,
     timeout?: number,
   ) {
     let promise: Promise<unknown> = Promise.resolve();

packages/jest-types/src/Global.ts

@@ -7,14 +7,16 @@
 
 import type {CoverageMapData} from 'istanbul-lib-coverage';
 
+type ValidTestReturnValues = void | undefined;
+type TestReturnValuePromise = Promise<ValidTestReturnValues>;
+type TestReturnValue = ValidTestReturnValues | TestReturnValuePromise;
+
 export type DoneFn = (reason?: string | Error) => void;
+export type DoneTakingTestFn = (done: DoneFn) => ValidTestReturnValues;
+export type PromiseReturningTestFn = () => TestReturnValue;
 export type TestName = string;
-export type TestFn = (
-  done?: DoneFn,
-) => Promise<void | undefined | unknown> | void | undefined;
-export type ConcurrentTestFn = (
-  done?: DoneFn,
-) => Promise<void | undefined | unknown>;
+export type TestFn = PromiseReturningTestFn | DoneTakingTestFn;
+export type ConcurrentTestFn = () => TestReturnValuePromise;
 export type BlockFn = () => void;
 export type BlockName = string;
 export type HookFn = TestFn;

test-types/top-level-globals.test.ts

@@ -7,7 +7,7 @@
  * @type ./empty.d.ts
  */
 
-import {expectType} from 'mlh-tsd';
+import {expectError, expectType} from 'mlh-tsd';
 import {
   afterAll,
   afterEach,
@@ -17,8 +17,12 @@ import {
   test,
   //eslint-disable-next-line import/no-extraneous-dependencies
 } from '@jest/globals';
+import type {Global} from '@jest/types';
 
 const fn = () => {};
+const doneFn: Global.DoneTakingTestFn = done => {
+  done();
+};
 const asyncFn = async () => {};
 const timeout = 5;
 const testName = 'Test name';
@@ -28,15 +32,42 @@ const testTable = [[1, 2]];
 expectType<void>(afterAll(fn));
 expectType<void>(afterAll(asyncFn));
 expectType<void>(afterAll(fn, timeout));
+expectType<void>(afterAll(asyncFn, timeout));
 expectType<void>(afterEach(fn));
 expectType<void>(afterEach(asyncFn));
 expectType<void>(afterEach(fn, timeout));
+expectType<void>(afterEach(asyncFn, timeout));
 expectType<void>(beforeAll(fn));
 expectType<void>(beforeAll(asyncFn));
 expectType<void>(beforeAll(fn, timeout));
+expectType<void>(beforeAll(asyncFn, timeout));
 expectType<void>(beforeEach(fn));
 expectType<void>(beforeEach(asyncFn));
 expectType<void>(beforeEach(fn, timeout));
+expectType<void>(beforeEach(asyncFn, timeout));
+
+expectType<void>(test(testName, fn));
+expectType<void>(test(testName, asyncFn));
+expectType<void>(test(testName, doneFn));
+expectType<void>(test(testName, fn, timeout));
+expectType<void>(test(testName, asyncFn, timeout));
+expectType<void>(test(testName, doneFn, timeout));
+
+// wrong arguments
+expectError(test(testName));
+expectError(test(testName, timeout));
+expectError(test(timeout, fn));
+
+// wrong return value
+expectError(test(testName, () => 42));
+expectError(test(testName, async () => 42));
+
+// mixing done callback and promise
+expectError(
+  test(testName, async done => {
+    done();
+  }),
+);
 
 expectType<void>(test.each(testTable)(testName, fn));
 expectType<void>(test.each(testTable)(testName, fn, timeout));