Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from May 2, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -10,6 +10,7 @@
- `[jest-fake-timers]` `getTimerCount` will not include cancelled immediates ([#8764](https://github.com/facebook/jest/pull/8764))
- `[jest-leak-detector]` [**BREAKING**] Use `weak-napi` instead of `weak` package ([#8686](https://github.com/facebook/jest/pull/8686))
- `[jest-snapshot]` Remove only the added newlines in multiline snapshots ([#8859](https://github.com/facebook/jest/pull/8859))
- `[jest-circus]` Throw a proper error if a test / hooks is defined asynchronously ([#8096](https://github.com/facebook/jest/pull/8096))
SimenB marked this conversation as resolved.
Show resolved Hide resolved

### Chore & Maintenance

Expand Down
61 changes: 61 additions & 0 deletions 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)
`;
24 changes: 24 additions & 0 deletions 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();
});
20 changes: 20 additions & 0 deletions 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(() => {});
});
5 changes: 5 additions & 0 deletions e2e/circus-declaration-errors/package.json
@@ -0,0 +1,5 @@
{
"jest": {
"testEnvironment": "node"
}
}
17 changes: 17 additions & 0 deletions packages/jest-circus/src/eventHandler.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion packages/jest-circus/src/state.ts
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-types/src/Circus.ts
Expand Up @@ -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.
Expand Down