From d30a586b4baea82295aaabeb52d14e7488359363 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Mon, 4 May 2020 17:49:08 +0200 Subject: [PATCH] fix: disallow hook definitions in tests (#9957) --- CHANGELOG.md | 1 + .../circusDeclarationErrors.test.ts.snap | 4 ++ e2e/__tests__/nestedTestDefinitions.test.ts | 32 +++++++++---- .../__tests__/nestedHookInTest.js | 18 ++++++++ packages/jest-circus/src/eventHandler.ts | 46 +++++++++++++------ .../jest-circus/src/formatNodeAssertErrors.ts | 2 +- packages/jest-types/src/Circus.ts | 2 +- 7 files changed, 78 insertions(+), 27 deletions(-) create mode 100644 e2e/nested-test-definitions/__tests__/nestedHookInTest.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e534df7aab1..444fdcfdcaf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,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-circus]` Throw more descriptive error if hook is defined inside test ([#9957](https://github.com/facebook/jest/pull/9957)) - `[jest-circus]` [**BREAKING**] Align execution order of tests to match `jasmine`'s top to bottom order ([#9965](https://github.com/facebook/jest/pull/9965)) - `[jest-config, jest-resolve]` [**BREAKING**] Remove support for `browser` field ([#9943](https://github.com/facebook/jest/pull/9943)) - `[jest-haste-map]` Stop reporting files as changed when they are only accessed ([#7347](https://github.com/facebook/jest/pull/7347)) diff --git a/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap index 83a0e4eab83d..bbaca7ca6978 100644 --- a/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap +++ b/e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap @@ -16,6 +16,7 @@ FAIL __tests__/asyncDefinition.test.js 14 | }); 15 | }); + at eventHandler (../../packages/jest-circus/build/eventHandler.js:143:11) at test (__tests__/asyncDefinition.test.js:12:5) ● Test suite failed to run @@ -30,6 +31,7 @@ FAIL __tests__/asyncDefinition.test.js 15 | }); 16 | + at eventHandler (../../packages/jest-circus/build/eventHandler.js:112:11) at afterAll (__tests__/asyncDefinition.test.js:13:5) ● Test suite failed to run @@ -44,6 +46,7 @@ FAIL __tests__/asyncDefinition.test.js 20 | }); 21 | + at eventHandler (../../packages/jest-circus/build/eventHandler.js:143:11) at test (__tests__/asyncDefinition.test.js:18:3) ● Test suite failed to run @@ -57,5 +60,6 @@ FAIL __tests__/asyncDefinition.test.js 20 | }); 21 | + at eventHandler (../../packages/jest-circus/build/eventHandler.js:112:11) at afterAll (__tests__/asyncDefinition.test.js:19:3) `; diff --git a/e2e/__tests__/nestedTestDefinitions.test.ts b/e2e/__tests__/nestedTestDefinitions.test.ts index 0a26d4fc14c5..ab000c325e19 100644 --- a/e2e/__tests__/nestedTestDefinitions.test.ts +++ b/e2e/__tests__/nestedTestDefinitions.test.ts @@ -42,16 +42,28 @@ test('print correct error message with nested test definitions inside describe', expect(cleanupRunnerStack(summary.rest)).toMatchSnapshot(); }); -test('print correct message when nesting describe inside it', () => { - if (!isJestCircusRun()) { - return; - } +(isJestCircusRun() ? test : test.skip)( + 'print correct message when nesting describe inside it', + () => { + const result = runJest('nested-test-definitions', ['nestedDescribeInTest']); - const result = runJest('nested-test-definitions', ['nestedDescribeInTest']); + expect(result.exitCode).toBe(1); - expect(result.exitCode).toBe(1); + expect(result.stderr).toContain( + 'Cannot nest a describe inside a test. Describe block "inner describe" cannot run because it is nested within "test".', + ); + }, +); - expect(result.stderr).toContain( - 'Cannot nest a describe inside a test. Describe block "inner describe" cannot run because it is nested within "test".', - ); -}); +(isJestCircusRun() ? test : test.skip)( + 'print correct message when nesting a hook inside it', + () => { + const result = runJest('nested-test-definitions', ['nestedHookInTest']); + + expect(result.exitCode).toBe(1); + + expect(result.stderr).toContain( + 'Hooks cannot be defined inside tests. Hook of type "beforeEach" is nested within "test".', + ); + }, +); diff --git a/e2e/nested-test-definitions/__tests__/nestedHookInTest.js b/e2e/nested-test-definitions/__tests__/nestedHookInTest.js new file mode 100644 index 000000000000..d7b403d9842c --- /dev/null +++ b/e2e/nested-test-definitions/__tests__/nestedHookInTest.js @@ -0,0 +1,18 @@ +/** + * 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'; + +const {getTruthy} = require('../index'); + +test('test', () => { + expect(getTruthy()).toBeTruthy(); + + beforeEach(() => { + // nothing to see here + }); +}); diff --git a/packages/jest-circus/src/eventHandler.ts b/packages/jest-circus/src/eventHandler.ts index 6e825f128e13..590e66ca3a13 100644 --- a/packages/jest-circus/src/eventHandler.ts +++ b/packages/jest-circus/src/eventHandler.ts @@ -39,9 +39,12 @@ const eventHandler: Circus.EventHandler = ( const {currentDescribeBlock, currentlyRunningTest} = state; if (currentlyRunningTest) { - throw new Error( - `Cannot nest a describe inside a test. Describe block "${blockName}" cannot run because it is nested within "${currentlyRunningTest.name}".`, + currentlyRunningTest.errors.push( + new Error( + `Cannot nest a describe inside a test. Describe block "${blockName}" cannot run because it is nested within "${currentlyRunningTest.name}".`, + ), ); + break; } const describeBlock = makeDescribe(blockName, currentDescribeBlock, mode); @@ -90,16 +93,25 @@ const eventHandler: Circus.EventHandler = ( break; } case 'add_hook': { - const {currentDescribeBlock, hasStarted} = state; + const {currentDescribeBlock, currentlyRunningTest, 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); + if (currentlyRunningTest) { + currentlyRunningTest.errors.push( + new Error( + `Hooks cannot be defined inside tests. Hook of type "${type}" is nested within "${currentlyRunningTest.name}".`, + ), + ); + break; + } else if (hasStarted) { + state.unhandledErrors.push( + new Error( + 'Cannot add a hook after tests have started running. Hooks must be defined synchronously.', + ), + ); break; } + const parent = currentDescribeBlock; currentDescribeBlock.hooks.push({asyncError, fn, parent, timeout, type}); break; @@ -109,14 +121,18 @@ const eventHandler: Circus.EventHandler = ( const {asyncError, fn, mode, testName: name, timeout} = event; if (currentlyRunningTest) { - throw new Error( - `Tests cannot be nested. Test "${name}" cannot run because it is nested within "${currentlyRunningTest.name}".`, + currentlyRunningTest.errors.push( + new Error( + `Tests cannot be nested. Test "${name}" cannot run because it is nested within "${currentlyRunningTest.name}".`, + ), + ); + break; + } else if (hasStarted) { + state.unhandledErrors.push( + new Error( + 'Cannot add a test after tests have started running. Tests must be defined synchronously.', + ), ); - } - if (hasStarted) { - asyncError.message = - 'Cannot add a test after tests have started running. Tests must be defined synchronously.'; - state.unhandledErrors.push(asyncError); break; } diff --git a/packages/jest-circus/src/formatNodeAssertErrors.ts b/packages/jest-circus/src/formatNodeAssertErrors.ts index d9dbdee0bb8e..10ffc8d1e25f 100644 --- a/packages/jest-circus/src/formatNodeAssertErrors.ts +++ b/packages/jest-circus/src/formatNodeAssertErrors.ts @@ -43,7 +43,7 @@ const formatNodeAssertErrors = ( state: Circus.State, ): void => { if (event.name === 'test_done') { - event.test.errors = event.test.errors.map((errors: Circus.TestError) => { + event.test.errors = event.test.errors.map(errors => { let error; if (Array.isArray(errors)) { const [originalError, asyncError] = errors; diff --git a/packages/jest-types/src/Circus.ts b/packages/jest-types/src/Circus.ts index e1291bfe1764..963a6a61b537 100644 --- a/packages/jest-types/src/Circus.ts +++ b/packages/jest-types/src/Circus.ts @@ -214,7 +214,7 @@ export type TestError = Exception | [Exception | undefined, Exception]; // the e export type TestEntry = { type: 'test'; asyncError: Exception; // Used if the test failure contains no usable stack trace - errors: TestError; + errors: Array; fn?: TestFn; invocations: number; mode: TestMode;