diff --git a/CHANGELOG.md b/CHANGELOG.md index b35aa33644df..bd655f7171f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ - `[jest-config]` Print error information on preset normalization error ([#7935](https://github.com/facebook/jest/pull/7935)) - `[jest-haste-map]` Add `skipPackageJson` option ([#7778](https://github.com/facebook/jest/pull/7778)) - `[jest-get-type]` Add `isPrimitive` function ([#7708](https://github.com/facebook/jest/pull/7708)) +- `[jest-circus/jest-jasmine2]` Warn if describe returns a value ([#7852](https://github.com/facebook/jest/pull/7852)) +- `[jest-util]` Add `isPromise` ([#7852](https://github.com/facebook/jest/pull/7852)) ### Fixes diff --git a/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap new file mode 100644 index 000000000000..ae697a3dfe6c --- /dev/null +++ b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap @@ -0,0 +1,41 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`warns if describe returns a Promise 1`] = ` +" console.log + ● Test suite failed to run + + Returning a Promise from \\"describe\\" is not supported. Tests must be defined synchronously. + Returning a value from \\"describe\\" will fail the test in a future version of Jest. + + 9 | 'use strict'; + 10 | + > 11 | describe('Promise describe warns', () => { + | ^ + 12 | it('t', () => {}); + 13 | return Promise.resolve(); + 14 | }); + + at Object.describe (__tests__/describeReturnPromise.test.js:11:1) + +" +`; + +exports[`warns if describe returns something 1`] = ` +" console.log + ● Test suite failed to run + + A \\"describe\\" callback must not return a value. + Returning a value from \\"describe\\" will fail the test in a future version of Jest. + + 9 | 'use strict'; + 10 | + > 11 | describe('describe return warns', () => { + | ^ + 12 | it('t', () => {}); + 13 | return 42; + 14 | }); + + at Object.describe (__tests__/describeReturnSomething.test.js:11:1) + +" +`; diff --git a/e2e/__tests__/declarationErrors.test.ts b/e2e/__tests__/declarationErrors.test.ts new file mode 100644 index 000000000000..fcd5050cfe6d --- /dev/null +++ b/e2e/__tests__/declarationErrors.test.ts @@ -0,0 +1,31 @@ +/** + * 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 runJest from '../runJest'; + +const normalizeCircusJasmine = (str: string) => + str + .replace(/console\.log .+:\d+/, 'console.log') + .replace(/.+addSpecsToSuite (.+:\d+:\d+).+\n/, ''); + +it('warns if describe returns a Promise', () => { + const result = runJest('declaration-errors', [ + 'describeReturnPromise.test.js', + ]); + + expect(result.status).toBe(0); + expect(normalizeCircusJasmine(result.stdout)).toMatchSnapshot(); +}); + +it('warns if describe returns something', () => { + const result = runJest('declaration-errors', [ + 'describeReturnSomething.test.js', + ]); + + expect(result.status).toBe(0); + expect(normalizeCircusJasmine(result.stdout)).toMatchSnapshot(); +}); diff --git a/e2e/declaration-errors/__tests__/describeReturnPromise.test.js b/e2e/declaration-errors/__tests__/describeReturnPromise.test.js new file mode 100644 index 000000000000..f40120ab34a1 --- /dev/null +++ b/e2e/declaration-errors/__tests__/describeReturnPromise.test.js @@ -0,0 +1,14 @@ +/** + * 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. + * + */ + +'use strict'; + +describe('Promise describe warns', () => { + it('t', () => {}); + return Promise.resolve(); +}); diff --git a/e2e/declaration-errors/__tests__/describeReturnSomething.test.js b/e2e/declaration-errors/__tests__/describeReturnSomething.test.js new file mode 100644 index 000000000000..d1e5e158ae52 --- /dev/null +++ b/e2e/declaration-errors/__tests__/describeReturnSomething.test.js @@ -0,0 +1,14 @@ +/** + * 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. + * + */ + +'use strict'; + +describe('describe return warns', () => { + it('t', () => {}); + return 42; +}); diff --git a/e2e/declaration-errors/package.json b/e2e/declaration-errors/package.json new file mode 100644 index 000000000000..148788b25446 --- /dev/null +++ b/e2e/declaration-errors/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +} diff --git a/packages/jest-circus/src/index.ts b/packages/jest-circus/src/index.ts index 788af87df0c0..b7e93f3da0f4 100644 --- a/packages/jest-circus/src/index.ts +++ b/packages/jest-circus/src/index.ts @@ -5,8 +5,10 @@ * LICENSE file in the root directory of this source tree. */ +import chalk from 'chalk'; import {bind as bindEach} from 'jest-each'; -import {ErrorWithStack} from 'jest-util'; +import {formatExecError} from 'jest-message-util'; +import {ErrorWithStack, isPromise} from 'jest-util'; import {Global} from '@jest/types'; import { BlockFn, @@ -63,7 +65,39 @@ const _dispatchDescribe = ( mode, name: 'start_describe_definition', }); - blockFn(); + const describeReturn = blockFn(); + + // TODO throw in Jest 25 + if (isPromise(describeReturn)) { + console.log( + formatExecError( + new ErrorWithStack( + chalk.yellow( + 'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.\n' + + 'Returning a value from "describe" will fail the test in a future version of Jest.', + ), + describeFn, + ), + {rootDir: '', testMatch: []}, + {noStackTrace: false}, + ), + ); + } else if (describeReturn !== undefined) { + console.log( + formatExecError( + new ErrorWithStack( + chalk.yellow( + 'A "describe" callback must not return a value.\n' + + 'Returning a value from "describe" will fail the test in a future version of Jest.', + ), + describeFn, + ), + {rootDir: '', testMatch: []}, + {noStackTrace: false}, + ), + ); + } + dispatch({blockName, mode, name: 'finish_describe_definition'}); }; diff --git a/packages/jest-jasmine2/src/jasmine/Env.ts b/packages/jest-jasmine2/src/jasmine/Env.ts index 365c72855fbb..14d3ea9587ee 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.ts +++ b/packages/jest-jasmine2/src/jasmine/Env.ts @@ -31,7 +31,9 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. /* eslint-disable sort-keys */ import {AssertionError} from 'assert'; -import {ErrorWithStack} from 'jest-util'; +import chalk from 'chalk'; +import {formatExecError} from 'jest-message-util'; +import {ErrorWithStack, isPromise} from 'jest-util'; import queueRunner, { Options as QueueRunnerOptions, QueueableFn, @@ -415,12 +417,42 @@ export default function(j$: Jasmine) { currentDeclarationSuite = suite; let declarationError: null | Error = null; + let describeReturnValue: null | Error = null; try { - specDefinitions.call(suite); + describeReturnValue = specDefinitions.call(suite); } catch (e) { declarationError = e; } + // TODO throw in Jest 25: declarationError = new Error + if (isPromise(describeReturnValue)) { + console.log( + formatExecError( + new Error( + chalk.yellow( + 'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.\n' + + 'Returning a value from "describe" will fail the test in a future version of Jest.', + ), + ), + {rootDir: '', testMatch: []}, + {noStackTrace: false}, + ), + ); + } else if (describeReturnValue !== undefined) { + console.log( + formatExecError( + new Error( + chalk.yellow( + 'A "describe" callback must not return a value.\n' + + 'Returning a value from "describe" will fail the test in a future version of Jest.', + ), + ), + {rootDir: '', testMatch: []}, + {noStackTrace: false}, + ), + ); + } + if (declarationError) { this.it('encountered a declaration exception', () => { throw declarationError; diff --git a/packages/jest-util/src/__tests__/isPromise.test.ts b/packages/jest-util/src/__tests__/isPromise.test.ts new file mode 100644 index 000000000000..41d9a758a70b --- /dev/null +++ b/packages/jest-util/src/__tests__/isPromise.test.ts @@ -0,0 +1,25 @@ +/** + * 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 isPromise from '../isPromise'; + +describe('not a Promise: ', () => { + test.each([undefined, null, true, 42, '1337', Symbol(), [], {}])( + '%p', + value => { + expect(isPromise(value)).toBe(false); + }, + ); +}); + +test('a resolved Promise', () => { + expect(isPromise(Promise.resolve(42))).toBe(true); +}); + +test('a rejected Promise', () => { + expect(isPromise(Promise.reject().catch(() => {}))).toBe(true); +}); diff --git a/packages/jest-util/src/index.ts b/packages/jest-util/src/index.ts index 4adeb9e3c470..79c797da9d40 100644 --- a/packages/jest-util/src/index.ts +++ b/packages/jest-util/src/index.ts @@ -21,6 +21,7 @@ import ErrorWithStack from './ErrorWithStack'; import getFailedSnapshotTests from './getFailedSnapshotTests'; import installCommonGlobals from './installCommonGlobals'; import isInteractive from './isInteractive'; +import isPromise from './isPromise'; import setGlobal from './setGlobal'; import deepCyclicCopy from './deepCyclicCopy'; import convertDescriptorToString from './convertDescriptorToString'; @@ -46,6 +47,7 @@ export = { getFailedSnapshotTests, installCommonGlobals, isInteractive, + isPromise, pluralize, preRunMessage, replacePathSepForGlob, diff --git a/packages/jest-util/src/isPromise.ts b/packages/jest-util/src/isPromise.ts new file mode 100644 index 000000000000..1006dac25651 --- /dev/null +++ b/packages/jest-util/src/isPromise.ts @@ -0,0 +1,14 @@ +/** + * 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. + */ + +// capture global.Promise before it may potentially be overwritten +const Promise: any = global.Promise; + +// see ES2015 spec 25.4.4.5, https://stackoverflow.com/a/38339199 +const isPromise = (candidate: unknown): candidate is Promise => + Promise.resolve(candidate) === candidate; +export default isPromise;