From 53ff9cd4d944f9e317b54aa0d4a68639075efba7 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sat, 2 Mar 2019 23:29:14 +0100 Subject: [PATCH 1/4] jest-circus: throw if a test / hook is defined asynchronously --- .../circusDeclarationErrors.test.ts.snap | 68 +++++++++++++++++++ e2e/__tests__/circusDeclarationErrors.test.ts | 20 ++++++ .../__tests__/asyncDefinition.test.js | 13 ++++ e2e/circus-declaration-errors/package.json | 5 ++ packages/jest-circus/src/eventHandler.ts | 17 +++++ packages/jest-circus/src/state.ts | 3 +- packages/jest-types/src/Circus.ts | 1 + 7 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap create mode 100644 e2e/__tests__/circusDeclarationErrors.test.ts create mode 100644 e2e/circus-declaration-errors/__tests__/asyncDefinition.test.js create mode 100644 e2e/circus-declaration-errors/package.json diff --git a/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap new file mode 100644 index 000000000000..4771d15ef42f --- /dev/null +++ b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap @@ -0,0 +1,68 @@ +// 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. + + 3 | + 4 | Promise.resolve().then(() => { + > 5 | test('async definition inside describe', () => {}); + | ^ + 6 | afterAll(() => {}); + 7 | }); + 8 | }); + + at test (__tests__/asyncDefinition.test.js:5:5) + + ● Test suite failed to run + + Cannot add a hook after tests have started running. Hooks must be defined synchronously. + + 4 | Promise.resolve().then(() => { + 5 | test('async definition inside describe', () => {}); + > 6 | afterAll(() => {}); + | ^ + 7 | }); + 8 | }); + 9 | + + at afterAll (__tests__/asyncDefinition.test.js:6:5) + + ● Test suite failed to run + + Cannot add a test after tests have started running. Tests must be defined synchronously. + + 9 | + 10 | Promise.resolve().then(() => { + > 11 | test('async definition outside describe', () => {}); + | ^ + 12 | afterAll(() => {}); + 13 | }); + 14 | + + at test (__tests__/asyncDefinition.test.js:11:3) + + ● Test suite failed to run + + Cannot add a hook after tests have started running. Hooks must be defined synchronously. + + 10 | Promise.resolve().then(() => { + 11 | test('async definition outside describe', () => {}); + > 12 | afterAll(() => {}); + | ^ + 13 | }); + 14 | + + at afterAll (__tests__/asyncDefinition.test.js:12:3) + +Test Suites: 1 failed, 1 total +Tests: 1 passed, 1 total +Snapshots: 0 total +Time: 0.281s, estimated 1s +Ran all test suites matching /asyncDefinition.test.js/i. +" +`; diff --git a/e2e/__tests__/circusDeclarationErrors.test.ts b/e2e/__tests__/circusDeclarationErrors.test.ts new file mode 100644 index 000000000000..119624b79186 --- /dev/null +++ b/e2e/__tests__/circusDeclarationErrors.test.ts @@ -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. + */ + +import {skipSuiteOnJasmine} from '@jest/test-utils'; +import runJest from '../runJest'; + +skipSuiteOnJasmine(); + +it('defining tests and hooks asynchronously throws', () => { + const result = runJest('circus-declaration-errors', [ + 'asyncDefinition.test.js', + ]); + + expect(result.status).toBe(1); + expect(result.stderr).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..1baed83c7f14 --- /dev/null +++ b/e2e/circus-declaration-errors/__tests__/asyncDefinition.test.js @@ -0,0 +1,13 @@ +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 29dd7d8a2796..2ee26bb775b5 100644 --- a/packages/jest-circus/src/eventHandler.ts +++ b/packages/jest-circus/src/eventHandler.ts @@ -80,12 +80,28 @@ const eventHandler: Circus.EventHandler = (event, state): void => { const {currentDescribeBlock} = state; const {asyncError, fn, hookType: type, timeout} = event; const parent = currentDescribeBlock; + + if (state.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} = state; const {asyncError, fn, mode, testName: name, timeout} = event; + + if (state.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, mode, @@ -150,6 +166,7 @@ const eventHandler: Circus.EventHandler = (event, state): void => { 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 d42928e90bfa..fb4190b26872 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 6ae1bddd4baa..500baf7b6d27 100644 --- a/packages/jest-types/src/Circus.ts +++ b/packages/jest-types/src/Circus.ts @@ -180,6 +180,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. From 6ab0ac1d555383e8da0d165e2b6cfbaa668f07ef Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sat, 9 Mar 2019 19:24:13 +0100 Subject: [PATCH 2/4] fix CI --- .../circusDeclarationErrors.test.ts.snap | 67 +++++++++---------- e2e/__tests__/circusDeclarationErrors.test.ts | 6 +- .../__tests__/asyncDefinition.test.js | 7 ++ 3 files changed, 42 insertions(+), 38 deletions(-) diff --git a/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap index 4771d15ef42f..83a0e4eab83d 100644 --- a/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap +++ b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap @@ -1,68 +1,61 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`defining tests and hooks asynchronously throws 1`] = ` -"FAIL __tests__/asyncDefinition.test.js +FAIL __tests__/asyncDefinition.test.js ● Test suite failed to run Cannot add a test after tests have started running. Tests must be defined synchronously. - 3 | - 4 | Promise.resolve().then(() => { - > 5 | test('async definition inside describe', () => {}); - | ^ - 6 | afterAll(() => {}); - 7 | }); - 8 | }); + 10 | + 11 | Promise.resolve().then(() => { + > 12 | test('async definition inside describe', () => {}); + | ^ + 13 | afterAll(() => {}); + 14 | }); + 15 | }); - at test (__tests__/asyncDefinition.test.js:5:5) + 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. - 4 | Promise.resolve().then(() => { - 5 | test('async definition inside describe', () => {}); - > 6 | afterAll(() => {}); - | ^ - 7 | }); - 8 | }); - 9 | + 11 | Promise.resolve().then(() => { + 12 | test('async definition inside describe', () => {}); + > 13 | afterAll(() => {}); + | ^ + 14 | }); + 15 | }); + 16 | - at afterAll (__tests__/asyncDefinition.test.js:6:5) + 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. - 9 | - 10 | Promise.resolve().then(() => { - > 11 | test('async definition outside describe', () => {}); + 16 | + 17 | Promise.resolve().then(() => { + > 18 | test('async definition outside describe', () => {}); | ^ - 12 | afterAll(() => {}); - 13 | }); - 14 | + 19 | afterAll(() => {}); + 20 | }); + 21 | - at test (__tests__/asyncDefinition.test.js:11:3) + 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. - 10 | Promise.resolve().then(() => { - 11 | test('async definition outside describe', () => {}); - > 12 | afterAll(() => {}); + 17 | Promise.resolve().then(() => { + 18 | test('async definition outside describe', () => {}); + > 19 | afterAll(() => {}); | ^ - 13 | }); - 14 | + 20 | }); + 21 | - at afterAll (__tests__/asyncDefinition.test.js:12:3) - -Test Suites: 1 failed, 1 total -Tests: 1 passed, 1 total -Snapshots: 0 total -Time: 0.281s, estimated 1s -Ran all test suites matching /asyncDefinition.test.js/i. -" + at afterAll (__tests__/asyncDefinition.test.js:19:3) `; diff --git a/e2e/__tests__/circusDeclarationErrors.test.ts b/e2e/__tests__/circusDeclarationErrors.test.ts index 119624b79186..14b965b3f98e 100644 --- a/e2e/__tests__/circusDeclarationErrors.test.ts +++ b/e2e/__tests__/circusDeclarationErrors.test.ts @@ -6,6 +6,8 @@ */ import {skipSuiteOnJasmine} from '@jest/test-utils'; +import {wrap} from 'jest-snapshot-serializer-raw'; +import {extractSummary} from '../Utils'; import runJest from '../runJest'; skipSuiteOnJasmine(); @@ -16,5 +18,7 @@ it('defining tests and hooks asynchronously throws', () => { ]); expect(result.status).toBe(1); - expect(result.stderr).toMatchSnapshot(); + + 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 index 1baed83c7f14..5a519faa3be7 100644 --- a/e2e/circus-declaration-errors/__tests__/asyncDefinition.test.js +++ b/e2e/circus-declaration-errors/__tests__/asyncDefinition.test.js @@ -1,3 +1,10 @@ +/** + * 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', () => {}); From 2a2fb980694a593627ed8537c831d5a7fc174225 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sat, 9 Mar 2019 19:31:24 +0100 Subject: [PATCH 3/4] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a69936a8fb79..fb8f5d8b1e82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - `[jest-fake-timers]` `getTimerCount` will not include cancelled immediates ([#8764](https://github.com/facebook/jest/pull/8764)) - `[jest-leak-detector]` [**BREAKING**] Use `weak-napi` instead of `weak` package ([#8686](https://github.com/facebook/jest/pull/8686)) - `[jest-snapshot]` Remove only the added newlines in multiline snapshots ([#8859](https://github.com/facebook/jest/pull/8859)) +- `[jest-circus]` Throw a proper error if a test / hooks is defined asynchronously ([#8096](https://github.com/facebook/jest/pull/8096)) ### Chore & Maintenance From 4b07a31c84aaafeeb959358b598161bc709f952d Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Sat, 2 May 2020 21:46:36 +0200 Subject: [PATCH 4/4] skip test for obsolete hook behavior on circus --- e2e/__tests__/beforeEachQueue.ts | 3 +++ 1 file changed, 3 insertions(+) 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');