[Jest Circus] Make hooks in empty describe blocks error (#6320)

* [Jest Circus] Make hooks in empty describe blocks error

Fixes #6293

* Prettier

* Trim stack trace for hooks in empty describe blocks

* Document hooks in empty describe blocks in CHANGELOG.md

* Move to e2e

* Add missing snapshots

* Improve error message

* Add test asserting we can have hooks in blocks with skipped tests

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

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:        <<REPLACED>>
+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:        <<REPLACED>>
+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:        <<REPLACED>>
+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:        <<REPLACED>>
+Ran all test suites matching /multiple-hooks-in-empty-describe.test.js/i.
+",
+}
+`;

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();
+});

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', () => {});
+});

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', () => {});
+});

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', () => {});
+});

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', () => {});
+});

e2e/empty-describe-with-hooks/package.json

@@ -0,0 +1,5 @@
+{
+  "jest": {
+    "testEnvironment": "node"
+  }
+}

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

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();
+});

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;
       }