From 3681cca5f53f901715ab0064c5bf78f463914498 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Fri, 17 Apr 2020 13:14:00 +0200 Subject: [PATCH] fix: throw on nested test declarations (#9828) --- CHANGELOG.md | 1 + .../nestedTestDefinitions.test.ts.snap | 40 +++++++++++++ e2e/__tests__/nestedTestDefinitions.test.ts | 57 +++++++++++++++++++ .../__tests__/nestedDescribeInTest.js | 18 ++++++ .../__tests__/nestedTestOutsideDescribe.js | 18 ++++++ .../__tests__/nestedTestWithinDescribe.js | 20 +++++++ e2e/nested-test-definitions/index.js | 10 ++++ e2e/nested-test-definitions/package.json | 5 ++ packages/jest-circus/src/eventHandler.ts | 18 +++++- packages/jest-jasmine2/src/jasmine/Env.ts | 6 +- 10 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 e2e/__tests__/__snapshots__/nestedTestDefinitions.test.ts.snap create mode 100644 e2e/__tests__/nestedTestDefinitions.test.ts create mode 100644 e2e/nested-test-definitions/__tests__/nestedDescribeInTest.js create mode 100644 e2e/nested-test-definitions/__tests__/nestedTestOutsideDescribe.js create mode 100644 e2e/nested-test-definitions/__tests__/nestedTestWithinDescribe.js create mode 100644 e2e/nested-test-definitions/index.js create mode 100644 e2e/nested-test-definitions/package.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f9727101221..4215de6b474b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixes - `[expect]` Restore support for passing functions to `toHaveLength` matcher ([#9796](https://github.com/facebook/jest/pull/9796)) +- `[jest-circus]` Throw on nested test definitions ([#9828](https://github.com/facebook/jest/pull/9828)) - `[jest-changed-files]` `--only-changed` should include staged files ([#9799](https://github.com/facebook/jest/pull/9799)) - `[jest-each]` `each` will throw an error when called with too many arguments ([#9818](https://github.com/facebook/jest/pull/9818)) diff --git a/e2e/__tests__/__snapshots__/nestedTestDefinitions.test.ts.snap b/e2e/__tests__/__snapshots__/nestedTestDefinitions.test.ts.snap new file mode 100644 index 000000000000..daba859d0094 --- /dev/null +++ b/e2e/__tests__/__snapshots__/nestedTestDefinitions.test.ts.snap @@ -0,0 +1,40 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`print correct error message with nested test definitions inside describe 1`] = ` +FAIL __tests__/nestedTestWithinDescribe.js + in describe + ✕ outer test + + ● in describe › outer test + + Tests cannot be nested. Test "inner test" cannot run because it is nested within "outer test". + + 14 | expect(getTruthy()).toBeTruthy(); + 15 | + > 16 | test('inner test', () => { + | ^ + 17 | expect(getTruthy()).toBeTruthy(); + 18 | }); + 19 | }); + + at Object.test (__tests__/nestedTestWithinDescribe.js:16:5) +`; + +exports[`print correct error message with nested test definitions outside describe 1`] = ` +FAIL __tests__/nestedTestOutsideDescribe.js + ✕ outer test + + ● outer test + + Tests cannot be nested. Test "inner test" cannot run because it is nested within "outer test". + + 13 | expect(getTruthy()).toBeTruthy(); + 14 | + > 15 | test('inner test', () => { + | ^ + 16 | expect(getTruthy()).toEqual('This test should not have run'); + 17 | }); + 18 | }); + + at Object.test (__tests__/nestedTestOutsideDescribe.js:15:3) +`; diff --git a/e2e/__tests__/nestedTestDefinitions.test.ts b/e2e/__tests__/nestedTestDefinitions.test.ts new file mode 100644 index 000000000000..0a26d4fc14c5 --- /dev/null +++ b/e2e/__tests__/nestedTestDefinitions.test.ts @@ -0,0 +1,57 @@ +/** + * 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 {wrap} from 'jest-snapshot-serializer-raw'; +import {isJestCircusRun} from '@jest/test-utils'; +import runJest from '../runJest'; +import {extractSummary} from '../Utils'; + +const cleanupRunnerStack = (stderr: string) => + wrap( + stderr + .split('\n') + .filter( + line => + !line.includes('packages/jest-jasmine2/build') && + !line.includes('packages/jest-circus/build'), + ) + .join('\n'), + ); + +test('print correct error message with nested test definitions outside describe', () => { + const result = runJest('nested-test-definitions', ['outside']); + + expect(result.exitCode).toBe(1); + + const summary = extractSummary(result.stderr); + + expect(cleanupRunnerStack(summary.rest)).toMatchSnapshot(); +}); + +test('print correct error message with nested test definitions inside describe', () => { + const result = runJest('nested-test-definitions', ['within']); + + expect(result.exitCode).toBe(1); + + const summary = extractSummary(result.stderr); + + expect(cleanupRunnerStack(summary.rest)).toMatchSnapshot(); +}); + +test('print correct message when nesting describe inside it', () => { + if (!isJestCircusRun()) { + return; + } + + const result = runJest('nested-test-definitions', ['nestedDescribeInTest']); + + 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".', + ); +}); diff --git a/e2e/nested-test-definitions/__tests__/nestedDescribeInTest.js b/e2e/nested-test-definitions/__tests__/nestedDescribeInTest.js new file mode 100644 index 000000000000..186e795484c7 --- /dev/null +++ b/e2e/nested-test-definitions/__tests__/nestedDescribeInTest.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(); + + describe('inner describe', () => { + // nothing to see here + }); +}); diff --git a/e2e/nested-test-definitions/__tests__/nestedTestOutsideDescribe.js b/e2e/nested-test-definitions/__tests__/nestedTestOutsideDescribe.js new file mode 100644 index 000000000000..b8c6eb9726c0 --- /dev/null +++ b/e2e/nested-test-definitions/__tests__/nestedTestOutsideDescribe.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('outer test', () => { + expect(getTruthy()).toBeTruthy(); + + test('inner test', () => { + expect(getTruthy()).toEqual('This test should not have run'); + }); +}); diff --git a/e2e/nested-test-definitions/__tests__/nestedTestWithinDescribe.js b/e2e/nested-test-definitions/__tests__/nestedTestWithinDescribe.js new file mode 100644 index 000000000000..30160cee5a4b --- /dev/null +++ b/e2e/nested-test-definitions/__tests__/nestedTestWithinDescribe.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. + */ + +'use strict'; + +const {getTruthy} = require('../index'); + +describe('in describe', () => { + test('outer test', () => { + expect(getTruthy()).toBeTruthy(); + + test('inner test', () => { + expect(getTruthy()).toBeTruthy(); + }); + }); +}); diff --git a/e2e/nested-test-definitions/index.js b/e2e/nested-test-definitions/index.js new file mode 100644 index 000000000000..849dbee86e41 --- /dev/null +++ b/e2e/nested-test-definitions/index.js @@ -0,0 +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. + */ + +'use strict'; + +module.exports.getTruthy = () => true; diff --git a/e2e/nested-test-definitions/package.json b/e2e/nested-test-definitions/package.json new file mode 100644 index 000000000000..0ded940b7cb7 --- /dev/null +++ b/e2e/nested-test-definitions/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "jsdom" + } +} diff --git a/packages/jest-circus/src/eventHandler.ts b/packages/jest-circus/src/eventHandler.ts index 7c3a0f9299f4..2e1327a8ad6b 100644 --- a/packages/jest-circus/src/eventHandler.ts +++ b/packages/jest-circus/src/eventHandler.ts @@ -36,7 +36,14 @@ const eventHandler: Circus.EventHandler = ( } case 'start_describe_definition': { const {blockName, mode} = event; - const {currentDescribeBlock} = state; + 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}".`, + ); + } + const describeBlock = makeDescribe(blockName, currentDescribeBlock, mode); currentDescribeBlock.children.push(describeBlock); state.currentDescribeBlock = describeBlock; @@ -88,8 +95,15 @@ const eventHandler: Circus.EventHandler = ( break; } case 'add_test': { - const {currentDescribeBlock} = state; + const {currentDescribeBlock, currentlyRunningTest} = state; 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}".`, + ); + } + const test = makeTest( fn, mode, diff --git a/packages/jest-jasmine2/src/jasmine/Env.ts b/packages/jest-jasmine2/src/jasmine/Env.ts index 48e6b82fd4b4..a94173a7321f 100644 --- a/packages/jest-jasmine2/src/jasmine/Env.ts +++ b/packages/jest-jasmine2/src/jasmine/Env.ts @@ -566,11 +566,7 @@ export default function (j$: Jasmine) { // This check throws an error to warn the user about the edge-case. if (currentSpec !== null) { throw new Error( - 'Tests cannot be nested. Test `' + - spec.description + - '` cannot run because it is nested within `' + - currentSpec.description + - '`.', + `Tests cannot be nested. Test "${spec.description}" cannot run because it is nested within "${currentSpec.description}".`, ); } currentDeclarationSuite.addChild(spec);