jest-circus: throw if a test / hook is defined asynchronously (#8096)

Author: jeysal

CHANGELOG.md

@@ -8,6 +8,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-config, jest-resolve]` [**BREAKING**] Remove support for `browser` field ([#9943](https://github.com/facebook/jest/pull/9943))
 
 ### Chore & Maintenance

e2e/__tests__/__snapshots__/circusDeclarationErrors.test.ts.snap

@@ -0,0 +1,61 @@
+// 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.
+
+      10 | 
+      11 |   Promise.resolve().then(() => {
+    > 12 |     test('async definition inside describe', () => {});
+         |     ^
+      13 |     afterAll(() => {});
+      14 |   });
+      15 | });
+
+      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.
+
+      11 |   Promise.resolve().then(() => {
+      12 |     test('async definition inside describe', () => {});
+    > 13 |     afterAll(() => {});
+         |     ^
+      14 |   });
+      15 | });
+      16 | 
+
+      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.
+
+      16 | 
+      17 | Promise.resolve().then(() => {
+    > 18 |   test('async definition outside describe', () => {});
+         |   ^
+      19 |   afterAll(() => {});
+      20 | });
+      21 | 
+
+      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.
+
+      17 | Promise.resolve().then(() => {
+      18 |   test('async definition outside describe', () => {});
+    > 19 |   afterAll(() => {});
+         |   ^
+      20 | });
+      21 | 
+
+      at afterAll (__tests__/asyncDefinition.test.js:19:3)
+`;

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');

e2e/__tests__/circusDeclarationErrors.test.ts

@@ -0,0 +1,24 @@
+/**
+ * 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 {wrap} from 'jest-snapshot-serializer-raw';
+import {extractSummary} from '../Utils';
+import runJest from '../runJest';
+
+skipSuiteOnJasmine();
+
+it('defining tests and hooks asynchronously throws', () => {
+  const result = runJest('circus-declaration-errors', [
+    'asyncDefinition.test.js',
+  ]);
+
+  expect(result.exitCode).toBe(1);
+
+  const {rest} = extractSummary(result.stderr);
+  expect(wrap(rest)).toMatchSnapshot();
+});

e2e/circus-declaration-errors/__tests__/asyncDefinition.test.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.
+ */
+
+describe('describe', () => {
+  test('correct test def', () => {});
+
+  Promise.resolve().then(() => {
+    test('async definition inside describe', () => {});
+    afterAll(() => {});
+  });
+});
+
+Promise.resolve().then(() => {
+  test('async definition outside describe', () => {});
+  afterAll(() => {});
+});

e2e/circus-declaration-errors/package.json

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

packages/jest-circus/src/eventHandler.ts

@@ -88,21 +88,35 @@ const eventHandler: Circus.EventHandler = (
       break;
     }
     case 'add_hook': {
-      const {currentDescribeBlock} = state;
+      const {currentDescribeBlock, 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);
+        break;
+      }
+
       currentDescribeBlock.hooks.push({asyncError, fn, parent, timeout, type});
       break;
     }
     case 'add_test': {
-      const {currentDescribeBlock, currentlyRunningTest} = state;
+      const {currentDescribeBlock, currentlyRunningTest, hasStarted} = 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}".`,
         );
       }
+      if (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,
@@ -168,6 +182,7 @@ const eventHandler: Circus.EventHandler = (
       break;
     }
     case 'run_start': {
+      state.hasStarted = true;
       global[TEST_TIMEOUT_SYMBOL] &&
         (state.testTimeout = global[TEST_TIMEOUT_SYMBOL]);
       break;

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,

packages/jest-types/src/Circus.ts

@@ -187,6 +187,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.