jest-circus: throw if a test / hook is defined asynchronously

CHANGELOG.md

@@ -33,6 +33,7 @@
 
 - `[expect]` Display `expectedDiff` more carefully in `toBeCloseTo` ([#8389](https://github.com/facebook/jest/pull/8389))
 - `[expect]` Avoid incorrect difference for subset when `toMatchObject` fails ([#9005](https://github.com/facebook/jest/pull/9005))
+- `[jest-circus]` [**BREAKING**] Throw a proper error if a test / hooks is defined asynchronously ([#8096](https://github.com/facebook/jest/pull/8096))
 - `[jest-config]` Use half of the available cores when `watchAll` mode is enabled ([#9117](https://github.com/facebook/jest/pull/9117))
 - `[jest-console]` Add missing `console.group` calls to `NullConsole` ([#9024](https://github.com/facebook/jest/pull/9024))
 - `[jest-core]` Don't include unref'd timers in --detectOpenHandles results ([#8941](https://github.com/facebook/jest/pull/8941))

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__/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.status).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

@@ -80,12 +80,28 @@ const eventHandler: Circus.EventHandler = (event, state): void => {
       const {currentDescribeBlock} = state;
       const {asyncError, fn, hookType: type, timeout} = event;
       const parent = currentDescribeBlock;
+
+      if (state.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} = state;
       const {asyncError, fn, mode, testName: name, timeout} = event;
+
+      if (state.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,
         mode,
@@ -150,6 +166,7 @@ const eventHandler: Circus.EventHandler = (event, state): void => {
       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

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