Skip to content

Commit 0681959

Browse files
stekyczstekycz
committed
Fix reporting of requires or Jest env method access after "teardown"
1 parent 5fd3805 commit 0681959

File tree

11 files changed

+124
-63
lines changed

11 files changed

+124
-63
lines changed

‎e2e/__tests__/__snapshots__/environmentAfterTeardown.test.ts.snap‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,27 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`prints useful error for environment methods after test is done 1`] = `
4-
"ReferenceError: You are trying to access a property or method of the Jest environment after it has been torn down. From __tests__/afterTeardown.test.js.
4+
"FAIL __tests__/afterTeardown.test.js
5+
✕ access environment methods after done (3 ms)
6+
7+
● access environment methods after done
8+
9+
ReferenceError: You are trying to access a property or method of the Jest environment outside of the scope of the test code.
510
611
9 | test('access environment methods after done', () => {
712
10 | setTimeout(() => {
813
> 11 | jest.clearAllTimers();
914
| ^
10-
12 | }, 100);
15+
12 | }, 0);
1116
13 | });
12-
14 |"
17+
14 |
18+
19+
at _getFakeTimers (../../packages/jest-runtime/build/index.js:1977:15)
20+
at Timeout.clearAllTimers [as _onTimeout] (__tests__/afterTeardown.test.js:11:10)
21+
22+
Test Suites: 1 failed, 1 total
23+
Tests: 1 failed, 1 total
24+
Snapshots: 0 total
25+
Time: 0.365 s, estimated 1 s
26+
Ran all test suites."
1327
`;

‎e2e/__tests__/__snapshots__/requireAfterTeardown.test.ts.snap‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,27 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

33
exports[`prints useful error for requires after test is done 1`] = `
4-
"ReferenceError: You are trying to \`import\` a file after the Jest environment has been torn down. From __tests__/lateRequire.test.js.
4+
"FAIL __tests__/lateRequire.test.js
5+
✕ require after done (5 ms)
6+
7+
● require after done
8+
9+
ReferenceError: You are trying to \`import\` a file outside of the scope of the test code.
510
611
9 | test('require after done', () => {
712
10 | setTimeout(() => {
813
> 11 | const double = require('../');
914
| ^
1015
12 |
1116
13 | expect(double(5)).toBe(10);
12-
14 | }, 100);"
17+
14 | }, 0);
18+
19+
at Runtime._execModule (../../packages/jest-runtime/build/index.js:1380:13)
20+
at Timeout.require [as _onTimeout] (__tests__/lateRequire.test.js:11:20)
21+
22+
Test Suites: 1 failed, 1 total
23+
Tests: 1 failed, 1 total
24+
Snapshots: 0 total
25+
Time: 0.332 s, estimated 1 s
26+
Ran all test suites."
1327
`;

‎e2e/__tests__/environmentAfterTeardown.test.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ import runJest from '../runJest';
99

1010
test('prints useful error for environment methods after test is done', () => {
1111
const {stderr} = runJest('environment-after-teardown');
12-
const interestingLines = stderr.split('\n').slice(9, 18).join('\n');
1312

14-
expect(interestingLines).toMatchSnapshot();
15-
expect(stderr.split('\n')[9]).toBe(
16-
'ReferenceError: You are trying to access a property or method of the Jest environment after it has been torn down. From __tests__/afterTeardown.test.js.',
17-
);
13+
expect(stderr).toMatchSnapshot();
1814
});

‎e2e/__tests__/requireAfterTeardown.test.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,5 @@ import runJest from '../runJest';
1010
test('prints useful error for requires after test is done', () => {
1111
const {stderr} = runJest('require-after-teardown');
1212

13-
const interestingLines = stderr.split('\n').slice(9, 18).join('\n');
14-
15-
expect(interestingLines).toMatchSnapshot();
16-
expect(stderr.split('\n')[19]).toMatch(
17-
new RegExp('(__tests__/lateRequire.test.js:11:20)'),
18-
);
13+
expect(stderr).toMatchSnapshot();
1914
});

‎e2e/environment-after-teardown/__tests__/afterTeardown.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@
99
test('access environment methods after done', () => {
1010
setTimeout(() => {
1111
jest.clearAllTimers();
12-
}, 100);
12+
}, 0);
1313
});

‎e2e/require-after-teardown/__tests__/lateRequire.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@ test('require after done', () => {
1111
const double = require('../');
1212

1313
expect(double(5)).toBe(10);
14-
}, 100);
14+
}, 0);
1515
});

‎packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ const jestAdapter = async (
3333
globalConfig,
3434
localRequire: runtime.requireModule.bind(runtime),
3535
parentProcess: process,
36+
runtime,
3637
sendMessageToJest,
3738
setGlobalsForRuntime: runtime.setGlobalsForRuntime.bind(runtime),
3839
testPath,

‎packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
} from '@jest/test-result';
1717
import type {Circus, Config, Global} from '@jest/types';
1818
import {formatExecError, formatResultsErrors} from 'jest-message-util';
19+
import type Runtime from 'jest-runtime';
1920
import {
2021
SnapshotState,
2122
addSerializer,
@@ -30,6 +31,7 @@ import {
3031
getState as getRunnerState,
3132
} from '../state';
3233
import testCaseReportHandler from '../testCaseReportHandler';
34+
import {unhandledRejectionHandler} from '../unhandledRejectionHandler';
3335
import {getTestID} from '../utils';
3436

3537
interface RuntimeGlobals extends Global.TestFrameworkGlobals {
@@ -39,6 +41,7 @@ interface RuntimeGlobals extends Global.TestFrameworkGlobals {
3941
export const initialize = async ({
4042
config,
4143
environment,
44+
runtime,
4245
globalConfig,
4346
localRequire,
4447
parentProcess,
@@ -48,6 +51,7 @@ export const initialize = async ({
4851
}: {
4952
config: Config.ProjectConfig;
5053
environment: JestEnvironment;
54+
runtime: Runtime;
5155
globalConfig: Config.GlobalConfig;
5256
localRequire: <T = unknown>(path: string) => T;
5357
testPath: string;
@@ -129,6 +133,8 @@ export const initialize = async ({
129133
addEventHandler(testCaseReportHandler(testPath, sendMessageToJest));
130134
}
131135

136+
addEventHandler(unhandledRejectionHandler(runtime));
137+
132138
// Return it back to the outer scope (test runner outside the VM).
133139
return {globals: globalsObject, snapshotState};
134140
};

‎packages/jest-circus/src/state.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,10 @@ import type {Circus} from '@jest/types';
99
import eventHandler from './eventHandler';
1010
import formatNodeAssertErrors from './formatNodeAssertErrors';
1111
import {STATE_SYM} from './types';
12-
import unhandledRejectionHandler from './unhandledRejectionHandler';
1312
import {makeDescribe} from './utils';
1413

1514
const eventHandlers: Array<Circus.EventHandler> = [
1615
eventHandler,
17-
unhandledRejectionHandler,
1816
formatNodeAssertErrors,
1917
];
2018

‎packages/jest-circus/src/unhandledRejectionHandler.ts‎

Lines changed: 51 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
import type {Circus} from '@jest/types';
9+
import type Runtime from 'jest-runtime';
910
import {addErrorToEachTestUnderDescribe, invariant} from './utils';
1011

1112
// Global values can be overwritten by mocks or tests. We'll capture
@@ -18,55 +19,62 @@ const untilNextEventLoopTurn = async () => {
1819
});
1920
};
2021

21-
const unhandledRejectionHandler: Circus.EventHandler = async (
22-
event,
23-
state,
24-
): Promise<void> => {
25-
if (event.name === 'hook_success' || event.name === 'hook_failure') {
26-
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
27-
await untilNextEventLoopTurn();
22+
export const unhandledRejectionHandler = (
23+
runtime: Runtime,
24+
): Circus.EventHandler => {
25+
return async (event, state) => {
26+
if (event.name === 'hook_start') {
27+
runtime.enterTestCode();
28+
} else if (event.name === 'hook_success' || event.name === 'hook_failure') {
29+
runtime.leaveTestCode();
2830

29-
const {test, describeBlock, hook} = event;
30-
const {asyncError, type} = hook;
31+
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
32+
await untilNextEventLoopTurn();
3133

32-
if (type === 'beforeAll') {
33-
invariant(describeBlock, 'always present for `*All` hooks');
34-
for (const error of state.unhandledRejectionErrorByPromise.values()) {
35-
addErrorToEachTestUnderDescribe(describeBlock, error, asyncError);
36-
}
37-
} else if (type === 'afterAll') {
38-
// Attaching `afterAll` errors to each test makes execution flow
39-
// too complicated, so we'll consider them to be global.
40-
for (const error of state.unhandledRejectionErrorByPromise.values()) {
41-
state.unhandledErrors.push([error, asyncError]);
34+
const {test, describeBlock, hook} = event;
35+
const {asyncError, type} = hook;
36+
37+
if (type === 'beforeAll') {
38+
invariant(describeBlock, 'always present for `*All` hooks');
39+
for (const error of state.unhandledRejectionErrorByPromise.values()) {
40+
addErrorToEachTestUnderDescribe(describeBlock, error, asyncError);
41+
}
42+
} else if (type === 'afterAll') {
43+
// Attaching `afterAll` errors to each test makes execution flow
44+
// too complicated, so we'll consider them to be global.
45+
for (const error of state.unhandledRejectionErrorByPromise.values()) {
46+
state.unhandledErrors.push([error, asyncError]);
47+
}
48+
} else {
49+
invariant(test, 'always present for `*Each` hooks');
50+
for (const error of test.unhandledRejectionErrorByPromise.values()) {
51+
test.errors.push([error, asyncError]);
52+
}
4253
}
43-
} else {
54+
} else if (event.name === 'test_fn_start') {
55+
runtime.enterTestCode();
56+
} else if (
57+
event.name === 'test_fn_success' ||
58+
event.name === 'test_fn_failure'
59+
) {
60+
runtime.leaveTestCode();
61+
62+
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
63+
await untilNextEventLoopTurn();
64+
65+
const {test} = event;
4466
invariant(test, 'always present for `*Each` hooks');
67+
4568
for (const error of test.unhandledRejectionErrorByPromise.values()) {
46-
test.errors.push([error, asyncError]);
69+
test.errors.push([error, event.test.asyncError]);
4770
}
48-
}
49-
} else if (
50-
event.name === 'test_fn_success' ||
51-
event.name === 'test_fn_failure'
52-
) {
53-
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
54-
await untilNextEventLoopTurn();
55-
56-
const {test} = event;
57-
invariant(test, 'always present for `*Each` hooks');
71+
} else if (event.name === 'teardown') {
72+
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
73+
await untilNextEventLoopTurn();
5874

59-
for (const error of test.unhandledRejectionErrorByPromise.values()) {
60-
test.errors.push([error, event.test.asyncError]);
75+
state.unhandledErrors.push(
76+
...state.unhandledRejectionErrorByPromise.values(),
77+
);
6178
}
62-
} else if (event.name === 'teardown') {
63-
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
64-
await untilNextEventLoopTurn();
65-
66-
state.unhandledErrors.push(
67-
...state.unhandledRejectionErrorByPromise.values(),
68-
);
69-
}
79+
};
7080
};
71-
72-
export default unhandledRejectionHandler;

0 commit comments

Comments
 (0)