From f1ae80554a600cce83d4c3cd54cf6c395745751b Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Mon, 14 Dec 2020 12:21:25 +0100 Subject: [PATCH] feat(jest-circus,jest-jasmine2): throw when describe returns a value (#10947) --- CHANGELOG.md | 1 + .../declarationErrors.test.ts.snap | 52 +++++++------------ e2e/__tests__/declarationErrors.test.ts | 30 ++++++----- .../__tests__/describeReturnPromise.test.js | 2 +- .../__tests__/describeReturnSomething.test.js | 2 +- packages/jest-circus/src/index.ts | 35 +++---------- packages/jest-jasmine2/src/jasmine/Env.ts | 31 ++--------- 7 files changed, 51 insertions(+), 102 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1853cc3957ca..5d075aed3528 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Features +- `[jest-circus, jest-jasmine2]` [**BREAKING**] Fail the test instead of just warning when describe returns a value ([#10947](https://github.com/facebook/jest/pull/10947)) - `[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)) diff --git a/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap index 017f5cc6d332..af2d42a56fb3 100644 --- a/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap +++ b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap @@ -1,41 +1,29 @@ // 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[`errors if describe returns a Promise 1`] = ` + Returning a Promise from "describe" is not supported. Tests must be defined synchronously. + 9 | 'use strict'; + 10 | + > 11 | describe('Promise describe errors', () => { + | ^ + 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) +exports[`errors if describe returns something 1`] = ` + A "describe" callback must not return a value. + 9 | 'use strict'; + 10 | + > 11 | describe('describe return errors', () => { + | ^ + 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 index d0d1dda1db16..777096cbc4e7 100644 --- a/e2e/__tests__/declarationErrors.test.ts +++ b/e2e/__tests__/declarationErrors.test.ts @@ -6,38 +6,44 @@ */ import wrap from 'jest-snapshot-serializer-raw'; +import {extractSummary} from '../Utils'; import runJest from '../runJest'; -const normalizeCircusJasmine = (str: string) => +const extractMessage = (str: string) => wrap( - str - .replace(/console\.log .+:\d+/, 'console.log') - .replace(/.+addSpecsToSuite (.+:\d+:\d+).+\n/g, '') - .replace(/.+_dispatchDescribe (.+:\d+:\d+).+\n/g, ''), + extractSummary(str) + .rest.replace( + // circus-jasmine normalization + /.+addSpecsToSuite (.+:\d+:\d+).+\n/g, + '', + ) + .match( + // all lines from the first to the last mentioned "describe" after the "●" line + /●(.|\n)*?\n(?.*describe((.|\n)*describe)*.*)(\n|$)/im, + )?.groups?.lines ?? '', ); -it('warns if describe returns a Promise', () => { +it('errors if describe returns a Promise', () => { const result = runJest('declaration-errors', [ 'describeReturnPromise.test.js', ]); - expect(result.exitCode).toBe(0); - expect(normalizeCircusJasmine(result.stdout)).toMatchSnapshot(); + expect(result.exitCode).toBe(1); + expect(extractMessage(result.stderr)).toMatchSnapshot(); }); -it('warns if describe returns something', () => { +it('errors if describe returns something', () => { const result = runJest('declaration-errors', [ 'describeReturnSomething.test.js', ]); - expect(result.exitCode).toBe(0); - expect(normalizeCircusJasmine(result.stdout)).toMatchSnapshot(); + expect(result.exitCode).toBe(1); + expect(extractMessage(result.stderr)).toMatchSnapshot(); }); it('errors if describe throws', () => { const result = runJest('declaration-errors', ['describeThrow.test.js']); expect(result.exitCode).toBe(1); - expect(result.stdout).toBe(''); expect(result.stderr).toContain('whoops'); }); diff --git a/e2e/declaration-errors/__tests__/describeReturnPromise.test.js b/e2e/declaration-errors/__tests__/describeReturnPromise.test.js index f40120ab34a1..847158d0408e 100644 --- a/e2e/declaration-errors/__tests__/describeReturnPromise.test.js +++ b/e2e/declaration-errors/__tests__/describeReturnPromise.test.js @@ -8,7 +8,7 @@ 'use strict'; -describe('Promise describe warns', () => { +describe('Promise describe errors', () => { it('t', () => {}); return Promise.resolve(); }); diff --git a/e2e/declaration-errors/__tests__/describeReturnSomething.test.js b/e2e/declaration-errors/__tests__/describeReturnSomething.test.js index d1e5e158ae52..765ba14477ef 100644 --- a/e2e/declaration-errors/__tests__/describeReturnSomething.test.js +++ b/e2e/declaration-errors/__tests__/describeReturnSomething.test.js @@ -8,7 +8,7 @@ 'use strict'; -describe('describe return warns', () => { +describe('describe return errors', () => { it('t', () => {}); return 42; }); diff --git a/packages/jest-circus/src/index.ts b/packages/jest-circus/src/index.ts index 117c4631bfcd..ca212b3fd657 100644 --- a/packages/jest-circus/src/index.ts +++ b/packages/jest-circus/src/index.ts @@ -5,10 +5,8 @@ * LICENSE file in the root directory of this source tree. */ -import chalk = require('chalk'); import type {Circus, Global} from '@jest/types'; import {bind as bindEach} from 'jest-each'; -import {formatExecError} from 'jest-message-util'; import {ErrorWithStack, isPromise} from 'jest-util'; import {dispatchSync} from './state'; @@ -60,36 +58,15 @@ const _dispatchDescribe = ( }); const describeReturn = blockFn(); - // TODO throw in Jest 25 if (isPromise(describeReturn)) { - // eslint-disable-next-line no-console - 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}, - ), + throw new ErrorWithStack( + 'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.', + describeFn, ); } else if (describeReturn !== undefined) { - // eslint-disable-next-line no-console - 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}, - ), + throw new ErrorWithStack( + 'A "describe" callback must not return a value.', + describeFn, ); } diff --git a/packages/jest-jasmine2/src/jasmine/Env.ts b/packages/jest-jasmine2/src/jasmine/Env.ts index ec3ee8c5d7f1..385123fb45e2 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.ts +++ b/packages/jest-jasmine2/src/jasmine/Env.ts @@ -31,8 +31,6 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. /* eslint-disable sort-keys, local/prefer-spread-eventually, local/prefer-rest-params-eventually */ import {AssertionError} from 'assert'; -import chalk = require('chalk'); -import {formatExecError} from 'jest-message-util'; import {ErrorWithStack, isPromise} from 'jest-util'; import assertionErrorMessage from '../assertionErrorMessage'; import isError from '../isError'; @@ -444,34 +442,13 @@ export default function (j$: Jasmine) { declarationError = e; } - // TODO throw in Jest 25: declarationError = new Error if (isPromise(describeReturnValue)) { - // eslint-disable-next-line no-console - 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}, - ), + declarationError = new Error( + 'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.', ); } else if (describeReturnValue !== undefined) { - // eslint-disable-next-line no-console - 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}, - ), + declarationError = new Error( + 'A "describe" callback must not return a value.', ); }