diff --git a/CHANGELOG.md b/CHANGELOG.md index 8291ae7e5bf2..3643c8d0c80f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ - `[jest-circus]` Fix `testLocation` on Windows when using `test.each` ([#10871](https://github.com/facebook/jest/pull/10871)) - `[jest-console]` `console.dir` now respects the second argument correctly ([#10638](https://github.com/facebook/jest/pull/10638)) - `[jest-environment-jsdom]` Use inner realm’s `ArrayBuffer` constructor ([#10885](https://github.com/facebook/jest/pull/10885)) +- `[jest-globals]` [**BREAKING**] Disallow return values other than a `Promise` from hooks and tests ([#10512](https://github.com/facebook/jest/pull/10512)) +- `[jest-globals]` [**BREAKING**] Disallow mixing a done callback and returning a `Promise` from hooks and tests ([#10512](https://github.com/facebook/jest/pull/10512)) - `[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) diff --git a/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts b/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts index bf4506a218b5..f55934f558f5 100644 --- a/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts +++ b/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts @@ -83,7 +83,7 @@ export const initialize = async ({ globalsObject.test.concurrent = (test => { const concurrent = ( testName: string, - testFn: () => Promise, + testFn: Global.ConcurrentTestFn, timeout?: number, ) => { // For concurrent tests we first run the function that returns promise, and then register a @@ -98,7 +98,7 @@ export const initialize = async ({ const only = ( testName: string, - testFn: () => Promise, + testFn: Global.ConcurrentTestFn, timeout?: number, ) => { const promise = mutex(() => testFn()); diff --git a/packages/jest-circus/src/utils.ts b/packages/jest-circus/src/utils.ts index 85a4f906c589..ae8a6695eead 100644 --- a/packages/jest-circus/src/utils.ts +++ b/packages/jest-circus/src/utils.ts @@ -12,7 +12,7 @@ import isGeneratorFn from 'is-generator-fn'; import slash = require('slash'); import StackUtils = require('stack-utils'); import type {AssertionResult, Status} from '@jest/test-result'; -import type {Circus} from '@jest/types'; +import type {Circus, Global} from '@jest/types'; import {ErrorWithStack, convertDescriptorToString, formatTime} from 'jest-util'; import prettyFormat from 'pretty-format'; import {ROOT_DESCRIBE_BLOCK_NAME, getState} from './state'; @@ -21,6 +21,16 @@ const stackUtils = new StackUtils({cwd: 'A path that does not exist'}); const jestEachBuildDir = slash(path.dirname(require.resolve('jest-each'))); +function takesDoneCallback(fn: Circus.AsyncFn): fn is Global.DoneTakingTestFn { + return fn.length > 0; +} + +function isGeneratorFunction( + fn: Global.PromiseReturningTestFn | Global.GeneratorReturningTestFn, +): fn is Global.GeneratorReturningTestFn { + return isGeneratorFn(fn); +} + export const makeDescribe = ( name: Circus.BlockName, parent?: Circus.DescribeBlock, @@ -177,7 +187,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 @@ -216,25 +226,17 @@ export const callAsyncCircusFn = ( }); }; - returnedValue = fn.call< - Circus.TestContext | undefined, - Array, - void | Promise | Generator | undefined - >(testContext, done); + returnedValue = fn.call(testContext, done); return; } - let returnedValue: any; - if (isGeneratorFn(fn)) { + let returnedValue: Global.TestReturnValue; + if (isGeneratorFunction(fn)) { returnedValue = co.wrap(fn).call({}); } else { try { - returnedValue = fn.call< - Circus.TestContext | undefined, - [], - void | Promise | Generator | undefined - >(testContext); + returnedValue = fn.call(testContext); } catch (error) { reject(error); return; diff --git a/packages/jest-jasmine2/src/jasmineAsyncInstall.ts b/packages/jest-jasmine2/src/jasmineAsyncInstall.ts index 896344f5515a..30748c0461a6 100644 --- a/packages/jest-jasmine2/src/jasmineAsyncInstall.ts +++ b/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 = Promise.resolve(); diff --git a/packages/jest-types/src/Circus.ts b/packages/jest-types/src/Circus.ts index c5f941efc11c..3dab1d8d7abe 100644 --- a/packages/jest-types/src/Circus.ts +++ b/packages/jest-types/src/Circus.ts @@ -20,7 +20,7 @@ export type HookFn = Global.HookFn; export type AsyncFn = TestFn | HookFn; export type SharedHookType = 'afterAll' | 'beforeAll'; export type HookType = SharedHookType | 'afterEach' | 'beforeEach'; -export type TestContext = Record; +export type TestContext = Global.TestContext; export type Exception = any; // Since in JS anything can be thrown as an error. export type FormattedError = string; // String representation of error. export type Hook = { diff --git a/packages/jest-types/src/Global.ts b/packages/jest-types/src/Global.ts index 99346e953588..46a4d0669b96 100644 --- a/packages/jest-types/src/Global.ts +++ b/packages/jest-types/src/Global.ts @@ -7,17 +7,32 @@ import type {CoverageMapData} from 'istanbul-lib-coverage'; -export type GeneratorFn = (...args: Array) => Generator; +export type ValidTestReturnValues = void | undefined; +type TestReturnValuePromise = Promise; +type TestReturnValueGenerator = Generator; +export type TestReturnValue = ValidTestReturnValues | TestReturnValuePromise; + +export type TestContext = Record; + export type DoneFn = (reason?: string | Error) => void; -export type CallbackFn = ( - done?: DoneFn, -) => void | undefined | Promise; +// these should not be undefined +export type DoneTakingTestFn = ( + this: TestContext | undefined, + done: DoneFn, +) => ValidTestReturnValues; +export type PromiseReturningTestFn = ( + this: TestContext | undefined, +) => TestReturnValue; +export type GeneratorReturningTestFn = ( + this: TestContext | undefined, +) => TestReturnValueGenerator; export type TestName = string; -export type TestFn = GeneratorFn | CallbackFn; -export type ConcurrentTestFn = ( - done?: DoneFn, -) => Promise; +export type TestFn = + | PromiseReturningTestFn + | GeneratorReturningTestFn + | DoneTakingTestFn; +export type ConcurrentTestFn = () => TestReturnValuePromise; export type BlockFn = () => void; export type BlockName = string; export type HookFn = TestFn; diff --git a/test-types/top-level-globals.test.ts b/test-types/top-level-globals.test.ts index 03ee7d8b9c89..cd019a976357 100644 --- a/test-types/top-level-globals.test.ts +++ b/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, @@ -16,8 +16,12 @@ import { describe, test, } from '@jest/globals'; +import type {Global} from '@jest/types'; const fn = () => {}; +const doneFn: Global.DoneTakingTestFn = done => { + done(); +}; const asyncFn = async () => {}; const genFn = function* () {}; const timeout = 5; @@ -29,18 +33,55 @@ expectType(afterAll(fn)); expectType(afterAll(asyncFn)); expectType(afterAll(genFn)); expectType(afterAll(fn, timeout)); +expectType(afterAll(asyncFn, timeout)); +expectType(afterAll(genFn, timeout)); expectType(afterEach(fn)); expectType(afterEach(asyncFn)); expectType(afterEach(genFn)); expectType(afterEach(fn, timeout)); +expectType(afterEach(asyncFn, timeout)); +expectType(afterEach(genFn, timeout)); expectType(beforeAll(fn)); expectType(beforeAll(asyncFn)); expectType(beforeAll(genFn)); expectType(beforeAll(fn, timeout)); +expectType(beforeAll(asyncFn, timeout)); +expectType(beforeAll(genFn, timeout)); expectType(beforeEach(fn)); expectType(beforeEach(asyncFn)); expectType(beforeEach(genFn)); expectType(beforeEach(fn, timeout)); +expectType(beforeEach(asyncFn, timeout)); +expectType(beforeEach(genFn, timeout)); + +expectType(test(testName, fn)); +expectType(test(testName, asyncFn)); +expectType(test(testName, doneFn)); +expectType(test(testName, genFn)); +expectType(test(testName, fn, timeout)); +expectType(test(testName, asyncFn, timeout)); +expectType(test(testName, doneFn, timeout)); +expectType(test(testName, genFn, timeout)); + +// wrong arguments +expectError(test(testName)); +expectError(test(testName, timeout)); +expectError(test(timeout, fn)); + +// wrong return value +expectError(test(testName, () => 42)); + +// mixing done callback and promise/generator +expectError( + test(testName, async done => { + done(); + }), +); +expectError( + test(testName, function* (done) { + done(); + }), +); expectType(test(testName, fn)); expectType(test(testName, asyncFn));