diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f18737254d9..ac92ed7f0cd7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - `[jest-circus, jest-console, jest-jasmine2, jest-reporters, jest-util, pretty-format]` Fix time durating formatting and consolidate time formatting code ([#9765](https://github.com/facebook/jest/pull/9765)) - `[jest-circus]` [**BREAKING**] Fail tests if a test takes a done callback and have return values ([#9129](https://github.com/facebook/jest/pull/9129)) +- `[jest-circus]` [**BREAKING**] Throw a proper error if a test / hook is defined asynchronously ([#8096](https://github.com/facebook/jest/pull/8096)) - `[jest-config, jest-resolve]` [**BREAKING**] Remove support for `browser` field ([#9943](https://github.com/facebook/jest/pull/9943)) ### Chore & Maintenance diff --git a/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap new file mode 100644 index 000000000000..83a0e4eab83d --- /dev/null +++ b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap @@ -0,0 +1,61 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`defining tests and hooks asynchronously throws 1`] = ` +FAIL __tests__/asyncDefinition.test.js + + + ● Test suite failed to run + + Cannot add a test after tests have started running. Tests must be defined synchronously. + + 10 | + 11 | Promise.resolve().then(() => { + > 12 | test('async definition inside describe', () => {}); + | ^ + 13 | afterAll(() => {}); + 14 | }); + 15 | }); + + at test (__tests__/asyncDefinition.test.js:12:5) + + ● Test suite failed to run + + Cannot add a hook after tests have started running. Hooks must be defined synchronously. + + 11 | Promise.resolve().then(() => { + 12 | test('async definition inside describe', () => {}); + > 13 | afterAll(() => {}); + | ^ + 14 | }); + 15 | }); + 16 | + + at afterAll (__tests__/asyncDefinition.test.js:13:5) + + ● Test suite failed to run + + Cannot add a test after tests have started running. Tests must be defined synchronously. + + 16 | + 17 | Promise.resolve().then(() => { + > 18 | test('async definition outside describe', () => {}); + | ^ + 19 | afterAll(() => {}); + 20 | }); + 21 | + + at test (__tests__/asyncDefinition.test.js:18:3) + + ● Test suite failed to run + + Cannot add a hook after tests have started running. Hooks must be defined synchronously. + + 17 | Promise.resolve().then(() => { + 18 | test('async definition outside describe', () => {}); + > 19 | afterAll(() => {}); + | ^ + 20 | }); + 21 | + + at afterAll (__tests__/asyncDefinition.test.js:19:3) +`; diff --git a/e2e/__tests__/beforeEachQueue.ts b/e2e/__tests__/beforeEachQueue.ts index a7d6738b39d5..23f84ad72e4a 100644 --- a/e2e/__tests__/beforeEachQueue.ts +++ b/e2e/__tests__/beforeEachQueue.ts @@ -5,9 +5,12 @@ * LICENSE file in the root directory of this source tree. */ +import {skipSuiteOnJestCircus} from '@jest/test-utils'; import {wrap} from 'jest-snapshot-serializer-raw'; import runJest from '../runJest'; +skipSuiteOnJestCircus(); // Circus does not support funky async definitions + describe('Correct beforeEach order', () => { it('ensures the correct order for beforeEach', () => { const result = runJest('before-each-queue'); diff --git a/e2e/__tests__/circusDeclarationErrors.test.ts b/e2e/__tests__/circusDeclarationErrors.test.ts new file mode 100644 index 000000000000..9de4ff365930 --- /dev/null +++ b/e2e/__tests__/circusDeclarationErrors.test.ts @@ -0,0 +1,24 @@ +/** + * 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 {skipSuiteOnJasmine} from '@jest/test-utils'; +import {wrap} from 'jest-snapshot-serializer-raw'; +import {extractSummary} from '../Utils'; +import runJest from '../runJest'; + +skipSuiteOnJasmine(); + +it('defining tests and hooks asynchronously throws', () => { + const result = runJest('circus-declaration-errors', [ + 'asyncDefinition.test.js', + ]); + + expect(result.exitCode).toBe(1); + + const {rest} = extractSummary(result.stderr); + expect(wrap(rest)).toMatchSnapshot(); +}); diff --git a/e2e/circus-declaration-errors/__tests__/asyncDefinition.test.js b/e2e/circus-declaration-errors/__tests__/asyncDefinition.test.js new file mode 100644 index 000000000000..5a519faa3be7 --- /dev/null +++ b/e2e/circus-declaration-errors/__tests__/asyncDefinition.test.js @@ -0,0 +1,20 @@ +/** + * 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. + */ + +describe('describe', () => { + test('correct test def', () => {}); + + Promise.resolve().then(() => { + test('async definition inside describe', () => {}); + afterAll(() => {}); + }); +}); + +Promise.resolve().then(() => { + test('async definition outside describe', () => {}); + afterAll(() => {}); +}); diff --git a/e2e/circus-declaration-errors/package.json b/e2e/circus-declaration-errors/package.json new file mode 100644 index 000000000000..148788b25446 --- /dev/null +++ b/e2e/circus-declaration-errors/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +} diff --git a/packages/jest-circus/src/eventHandler.ts b/packages/jest-circus/src/eventHandler.ts index 2e1327a8ad6b..87a2061a649f 100644 --- a/packages/jest-circus/src/eventHandler.ts +++ b/packages/jest-circus/src/eventHandler.ts @@ -88,14 +88,22 @@ const eventHandler: Circus.EventHandler = ( break; } case 'add_hook': { - const {currentDescribeBlock} = state; + const {currentDescribeBlock, hasStarted} = state; const {asyncError, fn, hookType: type, timeout} = event; const parent = currentDescribeBlock; + + if (hasStarted) { + asyncError.message = + 'Cannot add a hook after tests have started running. Hooks must be defined synchronously.'; + state.unhandledErrors.push(asyncError); + break; + } + currentDescribeBlock.hooks.push({asyncError, fn, parent, timeout, type}); break; } case 'add_test': { - const {currentDescribeBlock, currentlyRunningTest} = state; + const {currentDescribeBlock, currentlyRunningTest, hasStarted} = state; const {asyncError, fn, mode, testName: name, timeout} = event; if (currentlyRunningTest) { @@ -103,6 +111,12 @@ const eventHandler: Circus.EventHandler = ( `Tests cannot be nested. Test "${name}" cannot run because it is nested within "${currentlyRunningTest.name}".`, ); } + if (hasStarted) { + asyncError.message = + 'Cannot add a test after tests have started running. Tests must be defined synchronously.'; + state.unhandledErrors.push(asyncError); + break; + } const test = makeTest( fn, @@ -168,6 +182,7 @@ const eventHandler: Circus.EventHandler = ( break; } case 'run_start': { + state.hasStarted = true; global[TEST_TIMEOUT_SYMBOL] && (state.testTimeout = global[TEST_TIMEOUT_SYMBOL]); break; diff --git a/packages/jest-circus/src/state.ts b/packages/jest-circus/src/state.ts index 3342479821b1..be81a73045f6 100644 --- a/packages/jest-circus/src/state.ts +++ b/packages/jest-circus/src/state.ts @@ -24,7 +24,8 @@ const INITIAL_STATE: Circus.State = { currentDescribeBlock: ROOT_DESCRIBE_BLOCK, currentlyRunningTest: null, expand: undefined, - hasFocusedTests: false, // whether .only has been used on any test/describe + hasFocusedTests: false, + hasStarted: false, includeTestLocationInResult: false, parentProcess: null, rootDescribeBlock: ROOT_DESCRIBE_BLOCK, diff --git a/packages/jest-types/src/Circus.ts b/packages/jest-types/src/Circus.ts index bd82d0e0082a..845529c85907 100644 --- a/packages/jest-types/src/Circus.ts +++ b/packages/jest-types/src/Circus.ts @@ -187,6 +187,7 @@ export type State = { currentlyRunningTest?: TestEntry | null; // including when hooks are being executed expand?: boolean; // expand error messages hasFocusedTests: boolean; // that are defined using test.only + hasStarted: boolean; // whether the rootDescribeBlock has started running // Store process error handlers. During the run we inject our own // handlers (so we could fail tests on unhandled errors) and later restore // the original ones.