From 937012b249d7f7c5783208749c2cf4ddc74a3a6a Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sun, 13 Dec 2020 14:43:11 +0100 Subject: [PATCH 1/3] feat(jest-circus,jest-jasmine2): throw when describe returns a value BREAKING CHANGE: Returning a value from `describe` will now fail the test instead of just warn. --- .../declarationErrors.test.ts.snap | 54 ++++++++----------- e2e/__tests__/declarationErrors.test.ts | 30 ++++++----- .../__tests__/describeReturnPromise.test.js | 2 +- .../__tests__/describeReturnSomething.test.js | 2 +- packages/jest-circus/src/index.ts | 37 +++---------- packages/jest-jasmine2/src/jasmine/Env.ts | 33 +++--------- 6 files changed, 56 insertions(+), 102 deletions(-) diff --git a/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap index 017f5cc6d332..dfab732f7095 100644 --- a/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap +++ b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap @@ -1,41 +1,31 @@ // 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. + Returning a value from "describe" will fail the test in a future version of Jest. + 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. + Returning a value from "describe" will fail the test in a future version of Jest. + 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..3eac1638d912 100644 --- a/e2e/__tests__/declarationErrors.test.ts +++ b/e2e/__tests__/declarationErrors.test.ts @@ -7,37 +7,43 @@ import wrap from 'jest-snapshot-serializer-raw'; import runJest from '../runJest'; +import {extractSummary} from '../Utils'; -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..58e6b27e14ea 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,17 @@ 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.\n' + + 'Returning a value from "describe" will fail the test in a future version of Jest.', + 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.\n' + + 'Returning a value from "describe" will fail the test in a future version of Jest.', + describeFn, ); } diff --git a/packages/jest-jasmine2/src/jasmine/Env.ts b/packages/jest-jasmine2/src/jasmine/Env.ts index ec3ee8c5d7f1..102d2fa7186e 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,15 @@ 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.\n' + + 'Returning a value from "describe" will fail the test in a future version of Jest.', ); } 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.\n' + + 'Returning a value from "describe" will fail the test in a future version of Jest.', ); } From 70877fd466dbb8a53a5a7d070d49cd8066799c74 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sun, 13 Dec 2020 15:27:31 +0100 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7e8e3b07be7..e5f24d649de0 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)) From e09a21d35d7f68104140e21020471e66495331e4 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sun, 13 Dec 2020 15:41:40 +0100 Subject: [PATCH 3/3] fix msg and lint --- e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap | 2 -- e2e/__tests__/declarationErrors.test.ts | 2 +- packages/jest-circus/src/index.ts | 6 ++---- packages/jest-jasmine2/src/jasmine/Env.ts | 6 ++---- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap index dfab732f7095..af2d42a56fb3 100644 --- a/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap +++ b/e2e/__tests__/__snapshots__/declarationErrors.test.ts.snap @@ -2,7 +2,6 @@ exports[`errors if describe returns a Promise 1`] = ` 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 | @@ -17,7 +16,6 @@ exports[`errors if describe returns a Promise 1`] = ` exports[`errors if describe returns something 1`] = ` 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 | diff --git a/e2e/__tests__/declarationErrors.test.ts b/e2e/__tests__/declarationErrors.test.ts index 3eac1638d912..777096cbc4e7 100644 --- a/e2e/__tests__/declarationErrors.test.ts +++ b/e2e/__tests__/declarationErrors.test.ts @@ -6,8 +6,8 @@ */ import wrap from 'jest-snapshot-serializer-raw'; -import runJest from '../runJest'; import {extractSummary} from '../Utils'; +import runJest from '../runJest'; const extractMessage = (str: string) => wrap( diff --git a/packages/jest-circus/src/index.ts b/packages/jest-circus/src/index.ts index 58e6b27e14ea..ca212b3fd657 100644 --- a/packages/jest-circus/src/index.ts +++ b/packages/jest-circus/src/index.ts @@ -60,14 +60,12 @@ const _dispatchDescribe = ( if (isPromise(describeReturn)) { throw new ErrorWithStack( - '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.', + 'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.', describeFn, ); } else if (describeReturn !== undefined) { throw new ErrorWithStack( - 'A "describe" callback must not return a value.\n' + - 'Returning a value from "describe" will fail the test in a future version of Jest.', + '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 102d2fa7186e..385123fb45e2 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.ts +++ b/packages/jest-jasmine2/src/jasmine/Env.ts @@ -444,13 +444,11 @@ export default function (j$: Jasmine) { if (isPromise(describeReturnValue)) { declarationError = new Error( - '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.', + 'Returning a Promise from "describe" is not supported. Tests must be defined synchronously.', ); } else if (describeReturnValue !== undefined) { declarationError = new Error( - 'A "describe" callback must not return a value.\n' + - 'Returning a value from "describe" will fail the test in a future version of Jest.', + 'A "describe" callback must not return a value.', ); }