diff --git a/CHANGELOG.md b/CHANGELOG.md index f5c1a36ce23f..47d9f4a32957 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * `[jest-each]` Support one dimensional array of data ([#6351](https://github.com/facebook/jest/pull/6351)) * `[jest-watch]` create new package `jest-watch` to ease custom watch plugin development ([#6318](https://github.com/facebook/jest/pull/6318)) +* `[jest-circus]` Make hooks in empty describe blocks error ([#6320](https://github.com/facebook/jest/pull/6320)) * Add a config/CLI option `errorOnDeprecated` which makes calling deprecated APIs throw hepful error messages. ### Fixes diff --git a/e2e/__tests__/__snapshots__/empty-describe-with-hooks.test.js.snap b/e2e/__tests__/__snapshots__/empty-describe-with-hooks.test.js.snap new file mode 100644 index 000000000000..6aed70d1a1aa --- /dev/null +++ b/e2e/__tests__/__snapshots__/empty-describe-with-hooks.test.js.snap @@ -0,0 +1,147 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`hook in describe with skipped test 1`] = ` +Object { + "rest": "", + "summary": "Test Suites: 1 skipped, 0 of 1 total +Tests: 1 skipped, 1 total +Snapshots: 0 total +Time: <> +Ran all test suites matching /hook-in-describe-with-skipped-test.test.js/i.", +} +`; + +exports[`hook in empty describe 1`] = ` +Object { + "rest": "FAIL __tests__/hook-in-empty-describe.test.js + + + ● Test suite failed to run + + Invalid: beforeEach() may not be used in a describe block containing no tests. + + 7 | + 8 | describe('a block', () => { + > 9 | beforeEach(() => {}); + | ^ + 10 | }); + 11 | + 12 | describe('another block with tests', () => { + + at __tests__/hook-in-empty-describe.test.js:9:3 + at __tests__/hook-in-empty-describe.test.js:8:1 + +", + "summary": "Test Suites: 1 failed, 1 total +Tests: 1 passed, 1 total +Snapshots: 0 total +Time: <> +Ran all test suites matching /hook-in-empty-describe.test.js/i. +", +} +`; + +exports[`hook in empty nested describe 1`] = ` +Object { + "rest": "FAIL __tests__/hook-in-empty-nested-describe.test.js + + + ● Test suite failed to run + + Invalid: beforeEach() may not be used in a describe block containing no tests. + + 7 | + 8 | describe('a block', () => { + > 9 | beforeEach(() => {}); + | ^ + 10 | describe('another block', () => {}); + 11 | }); + 12 | + + at __tests__/hook-in-empty-nested-describe.test.js:9:3 + at __tests__/hook-in-empty-nested-describe.test.js:8:1 + +", + "summary": "Test Suites: 1 failed, 1 total +Tests: 1 passed, 1 total +Snapshots: 0 total +Time: <> +Ran all test suites matching /hook-in-empty-nested-describe.test.js/i. +", +} +`; + +exports[`multiple hooks in empty describe 1`] = ` +Object { + "rest": "FAIL __tests__/multiple-hooks-in-empty-describe.test.js + + + ● Test suite failed to run + + Invalid: beforeEach() may not be used in a describe block containing no tests. + + 7 | + 8 | describe('a block', () => { + > 9 | beforeEach(() => {}); + | ^ + 10 | afterEach(() => {}); + 11 | afterAll(() => {}); + 12 | beforeAll(() => {}); + + at __tests__/multiple-hooks-in-empty-describe.test.js:9:3 + at __tests__/multiple-hooks-in-empty-describe.test.js:8:1 + + ● Test suite failed to run + + Invalid: afterEach() may not be used in a describe block containing no tests. + + 8 | describe('a block', () => { + 9 | beforeEach(() => {}); + > 10 | afterEach(() => {}); + | ^ + 11 | afterAll(() => {}); + 12 | beforeAll(() => {}); + 13 | }); + + at __tests__/multiple-hooks-in-empty-describe.test.js:10:3 + at __tests__/multiple-hooks-in-empty-describe.test.js:8:1 + + ● Test suite failed to run + + Invalid: afterAll() may not be used in a describe block containing no tests. + + 9 | beforeEach(() => {}); + 10 | afterEach(() => {}); + > 11 | afterAll(() => {}); + | ^ + 12 | beforeAll(() => {}); + 13 | }); + 14 | + + at __tests__/multiple-hooks-in-empty-describe.test.js:11:3 + at __tests__/multiple-hooks-in-empty-describe.test.js:8:1 + + ● Test suite failed to run + + Invalid: beforeAll() may not be used in a describe block containing no tests. + + 10 | afterEach(() => {}); + 11 | afterAll(() => {}); + > 12 | beforeAll(() => {}); + | ^ + 13 | }); + 14 | + 15 | describe('another block with tests', () => { + + at __tests__/multiple-hooks-in-empty-describe.test.js:12:3 + at __tests__/multiple-hooks-in-empty-describe.test.js:8:1 + +", + "summary": "Test Suites: 1 failed, 1 total +Tests: 1 passed, 1 total +Snapshots: 0 total +Time: <> +Ran all test suites matching /multiple-hooks-in-empty-describe.test.js/i. +", +} +`; diff --git a/e2e/__tests__/empty-describe-with-hooks.test.js b/e2e/__tests__/empty-describe-with-hooks.test.js new file mode 100644 index 000000000000..7a366e3b6bf6 --- /dev/null +++ b/e2e/__tests__/empty-describe-with-hooks.test.js @@ -0,0 +1,42 @@ +/** + * Copyright (c) 2018-present, Facebook, Inc. 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. + * + * @flow + */ + +'use strict'; + +const path = require('path'); +const runJest = require('../runJest'); +const {extractSummary} = require('../Utils'); +const dir = path.resolve(__dirname, '../empty-describe-with-hooks'); +const ConditionalTest = require('../../scripts/ConditionalTest'); + +ConditionalTest.skipSuiteOnJasmine(); + +test('hook in empty describe', () => { + const result = runJest(dir, ['hook-in-empty-describe.test.js']); + expect(result.status).toBe(1); + expect(extractSummary(result.stderr)).toMatchSnapshot(); +}); + +test('hook in describe with skipped test', () => { + const result = runJest(dir, ['hook-in-describe-with-skipped-test.test.js']); + expect(result.status).toBe(0); + expect(extractSummary(result.stderr)).toMatchSnapshot(); +}); + +test('hook in empty nested describe', () => { + const result = runJest(dir, ['hook-in-empty-nested-describe.test.js']); + expect(result.status).toBe(1); + expect(extractSummary(result.stderr)).toMatchSnapshot(); +}); + +test('multiple hooks in empty describe', () => { + const result = runJest(dir, ['multiple-hooks-in-empty-describe.test.js']); + expect(result.status).toBe(1); + expect(extractSummary(result.stderr)).toMatchSnapshot(); +}); diff --git a/e2e/empty-describe-with-hooks/__tests__/hook-in-describe-with-skipped-test.test.js b/e2e/empty-describe-with-hooks/__tests__/hook-in-describe-with-skipped-test.test.js new file mode 100644 index 000000000000..c187798a83ef --- /dev/null +++ b/e2e/empty-describe-with-hooks/__tests__/hook-in-describe-with-skipped-test.test.js @@ -0,0 +1,11 @@ +/** + * Copyright (c) 2018-present, Facebook, Inc. 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('a block', () => { + beforeEach(() => {}); + test.skip('skipped test', () => {}); +}); diff --git a/e2e/empty-describe-with-hooks/__tests__/hook-in-empty-describe.test.js b/e2e/empty-describe-with-hooks/__tests__/hook-in-empty-describe.test.js new file mode 100644 index 000000000000..326c05104438 --- /dev/null +++ b/e2e/empty-describe-with-hooks/__tests__/hook-in-empty-describe.test.js @@ -0,0 +1,14 @@ +/** + * Copyright (c) 2018-present, Facebook, Inc. 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('a block', () => { + beforeEach(() => {}); +}); + +describe('another block with tests', () => { + test('this test prevents us from failing due to zero tests', () => {}); +}); diff --git a/e2e/empty-describe-with-hooks/__tests__/hook-in-empty-nested-describe.test.js b/e2e/empty-describe-with-hooks/__tests__/hook-in-empty-nested-describe.test.js new file mode 100644 index 000000000000..ec2b26f3b61f --- /dev/null +++ b/e2e/empty-describe-with-hooks/__tests__/hook-in-empty-nested-describe.test.js @@ -0,0 +1,15 @@ +/** + * Copyright (c) 2018-present, Facebook, Inc. 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('a block', () => { + beforeEach(() => {}); + describe('another block', () => {}); +}); + +describe('another block with tests', () => { + test('this test prevents us from failing due to zero tests', () => {}); +}); diff --git a/e2e/empty-describe-with-hooks/__tests__/multiple-hooks-in-empty-describe.test.js b/e2e/empty-describe-with-hooks/__tests__/multiple-hooks-in-empty-describe.test.js new file mode 100644 index 000000000000..76cdfc1ef283 --- /dev/null +++ b/e2e/empty-describe-with-hooks/__tests__/multiple-hooks-in-empty-describe.test.js @@ -0,0 +1,17 @@ +/** + * Copyright (c) 2018-present, Facebook, Inc. 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('a block', () => { + beforeEach(() => {}); + afterEach(() => {}); + afterAll(() => {}); + beforeAll(() => {}); +}); + +describe('another block with tests', () => { + test('this test prevents us from failing due to zero tests', () => {}); +}); diff --git a/e2e/empty-describe-with-hooks/package.json b/e2e/empty-describe-with-hooks/package.json new file mode 100644 index 000000000000..148788b25446 --- /dev/null +++ b/e2e/empty-describe-with-hooks/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +} diff --git a/packages/jest-circus/src/__tests__/__snapshots__/after_all-test.js.snap b/packages/jest-circus/src/__tests__/__snapshots__/after_all-test.js.snap index 62f1c5843d32..0aefc958dd83 100644 --- a/packages/jest-circus/src/__tests__/__snapshots__/after_all-test.js.snap +++ b/packages/jest-circus/src/__tests__/__snapshots__/after_all-test.js.snap @@ -1,5 +1,62 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`describe block _can_ have hooks if a child describe block has tests 1`] = ` +"start_describe_definition: describe +add_hook: afterEach +add_hook: beforeEach +add_hook: afterAll +add_hook: beforeAll +start_describe_definition: child describe +add_test: my test +finish_describe_definition: child describe +finish_describe_definition: describe +run_start +run_describe_start: ROOT_DESCRIBE_BLOCK +run_describe_start: describe +hook_start: beforeAll +hook_success: beforeAll +run_describe_start: child describe +test_start: my test +hook_start: beforeEach +hook_success: beforeEach +test_fn_start: my test +test_fn_failure: my test +hook_start: afterEach +hook_success: afterEach +test_done: my test +run_describe_finish: child describe +hook_start: afterAll +hook_success: afterAll +run_describe_finish: describe +run_describe_finish: ROOT_DESCRIBE_BLOCK +run_finish + +unhandledErrors: 0 +" +`; + +exports[`describe block cannot have hooks and no tests 1`] = ` +"start_describe_definition: describe +add_hook: afterEach +add_hook: beforeEach +add_hook: afterAll +add_hook: beforeAll +finish_describe_definition: describe +run_start +run_describe_start: ROOT_DESCRIBE_BLOCK +run_describe_start: describe +hook_start: beforeAll +hook_success: beforeAll +hook_start: afterAll +hook_success: afterAll +run_describe_finish: describe +run_describe_finish: ROOT_DESCRIBE_BLOCK +run_finish + +unhandledErrors: 4 +" +`; + exports[`tests are not marked done until their parent afterAll runs 1`] = ` "start_describe_definition: describe add_hook: afterAll diff --git a/packages/jest-circus/src/__tests__/after_all-test.js b/packages/jest-circus/src/__tests__/after_all-test.js index c802a0b479fa..956348e4510c 100644 --- a/packages/jest-circus/src/__tests__/after_all-test.js +++ b/packages/jest-circus/src/__tests__/after_all-test.js @@ -37,3 +37,33 @@ test('tests are not marked done until their parent afterAll runs', () => { expect(stdout).toMatchSnapshot(); }); + +test('describe block cannot have hooks and no tests', () => { + const result = runTest(` + describe('describe', () => { + afterEach(() => {}); + beforeEach(() => {}); + afterAll(() => {}); + beforeAll(() => {}); + }) + `); + + expect(result.stdout).toMatchSnapshot(); +}); + +test('describe block _can_ have hooks if a child describe block has tests', () => { + const result = runTest(` + describe('describe', () => { + afterEach(() => {}); + beforeEach(() => {}); + afterAll(() => {}); + beforeAll(() => {}); + describe('child describe', () => { + test('my test', () => { + expect(true).toBe(true); + }) + }) + }) + `); + expect(result.stdout).toMatchSnapshot(); +}); diff --git a/packages/jest-circus/src/event_handler.js b/packages/jest-circus/src/event_handler.js index f22d1b2c820d..23782bf028e5 100644 --- a/packages/jest-circus/src/event_handler.js +++ b/packages/jest-circus/src/event_handler.js @@ -15,6 +15,7 @@ import { getTestDuration, invariant, makeTest, + describeBlockHasTests, } from './utils'; import { injectGlobalErrorHandlers, @@ -44,6 +45,16 @@ const handler: EventHandler = (event, state): void => { case 'finish_describe_definition': { const {currentDescribeBlock} = state; invariant(currentDescribeBlock, `currentDescribeBlock must be there`); + + if (!describeBlockHasTests(currentDescribeBlock)) { + currentDescribeBlock.hooks.forEach(hook => { + hook.asyncError.message = `Invalid: ${ + hook.type + }() may not be used in a describe block containing no tests.`; + state.unhandledErrors.push(hook.asyncError); + }); + } + if (currentDescribeBlock.parent) { state.currentDescribeBlock = currentDescribeBlock.parent; } diff --git a/packages/jest-circus/src/index.js b/packages/jest-circus/src/index.js index e82cd9a3fd0d..91e315b1241d 100644 --- a/packages/jest-circus/src/index.js +++ b/packages/jest-circus/src/index.js @@ -39,12 +39,23 @@ const _dispatchDescribe = (blockFn, blockName, mode?: BlockMode) => { dispatch({blockName, mode, name: 'finish_describe_definition'}); }; -const _addHook = (fn: HookFn, hookType: HookType, timeout: ?number) => - dispatch({asyncError: new Error(), fn, hookType, name: 'add_hook', timeout}); -const beforeEach: THook = (fn, timeout) => _addHook(fn, 'beforeEach', timeout); -const beforeAll: THook = (fn, timeout) => _addHook(fn, 'beforeAll', timeout); -const afterEach: THook = (fn, timeout) => _addHook(fn, 'afterEach', timeout); -const afterAll: THook = (fn, timeout) => _addHook(fn, 'afterAll', timeout); +const _addHook = (fn: HookFn, hookType: HookType, hookFn, timeout: ?number) => { + const asyncError = new Error(); + if (Error.captureStackTrace) { + Error.captureStackTrace(asyncError, hookFn); + } + dispatch({asyncError, fn, hookType, name: 'add_hook', timeout}); +}; + +// Hooks have to pass themselves to the HOF in order for us to trim stack traces. +const beforeEach: THook = (fn, timeout) => + _addHook(fn, 'beforeEach', beforeEach, timeout); +const beforeAll: THook = (fn, timeout) => + _addHook(fn, 'beforeAll', beforeAll, timeout); +const afterEach: THook = (fn, timeout) => + _addHook(fn, 'afterEach', afterEach, timeout); +const afterAll: THook = (fn, timeout) => + _addHook(fn, 'afterAll', afterAll, timeout); const test = (testName: TestName, fn: TestFn, timeout?: number) => { if (typeof testName !== 'string') { diff --git a/packages/jest-circus/src/legacy_code_todo_rewrite/jest_adapter_init.js b/packages/jest-circus/src/legacy_code_todo_rewrite/jest_adapter_init.js index 990bf18d8479..6a1e50f8614a 100644 --- a/packages/jest-circus/src/legacy_code_todo_rewrite/jest_adapter_init.js +++ b/packages/jest-circus/src/legacy_code_todo_rewrite/jest_adapter_init.js @@ -160,7 +160,9 @@ export const runAndTransformResultsToJestFormat = async ({ failureMessage = (failureMessage || '') + '\n\n' + - formatExecError(testExecError, config, globalConfig); + runResult.unhandledErrors + .map(err => formatExecError(err, config, globalConfig)) + .join('\n'); } dispatch({name: 'teardown'}); diff --git a/packages/jest-circus/src/utils.js b/packages/jest-circus/src/utils.js index 1c6618a1711b..7d3dd4c7213b 100644 --- a/packages/jest-circus/src/utils.js +++ b/packages/jest-circus/src/utils.js @@ -131,6 +131,10 @@ export const getEachHooksForTest = ( return result; }; +export const describeBlockHasTests = (describe: DescribeBlock) => { + return describe.tests.length || describe.children.some(describeBlockHasTests); +}; + const _makeTimeoutMessage = (timeout, isHook) => `Exceeded timeout of ${timeout}ms for a ${ isHook ? 'hook' : 'test'