Skip to content

Commit

Permalink
Fix reporting of requires or Jest env method access after "teardown"
Browse files Browse the repository at this point in the history
  • Loading branch information
stekycz committed May 26, 2023
1 parent 5fd3805 commit b412305
Show file tree
Hide file tree
Showing 11 changed files with 99 additions and 57 deletions.
@@ -1,13 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`prints useful error for environment methods after test is done 1`] = `
"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.
" ReferenceError: You are trying to access a property or method of the Jest environment outside of the scope of the test code.
9 | test('access environment methods after done', () => {
10 | setTimeout(() => {
> 11 | jest.clearAllTimers();
| ^
12 | }, 100);
12 | }, 0);
13 | });
14 |"
`;
4 changes: 2 additions & 2 deletions e2e/__tests__/__snapshots__/requireAfterTeardown.test.ts.snap
@@ -1,13 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`prints useful error for requires after test is done 1`] = `
"ReferenceError: You are trying to \`import\` a file after the Jest environment has been torn down. From __tests__/lateRequire.test.js.
" ReferenceError: You are trying to \`import\` a file outside of the scope of the test code.
9 | test('require after done', () => {
10 | setTimeout(() => {
> 11 | const double = require('../');
| ^
12 |
13 | expect(double(5)).toBe(10);
14 | }, 100);"
14 | }, 0);"
`;
6 changes: 3 additions & 3 deletions e2e/__tests__/environmentAfterTeardown.test.ts
Expand Up @@ -9,10 +9,10 @@ import runJest from '../runJest';

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

expect(interestingLines).toMatchSnapshot();

Check failure on line 14 in e2e/__tests__/environmentAfterTeardown.test.ts

View workflow job for this annotation

GitHub Actions / test-ubuntu / Node LTS on ubuntu-latest using jest-jasmine2 (2/4)

prints useful error for environment methods after test is done

expect(received).toMatchSnapshot() Snapshot name: `prints useful error for environment methods after test is done 1` - Snapshot - 5 + Received + 5 - ReferenceError: You are trying to access a property or method of the Jest environment outside of the scope of the test code. + Snapshots: 0 total + Time: 1.014 s + Ran all test suites. + + 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. 9 | test('access environment methods after done', () => { 10 | setTimeout(() => { > 11 | jest.clearAllTimers(); - | ^ - 12 | }, 0); - 13 | }); - 14 | at Object.toMatchSnapshot (e2e/__tests__/environmentAfterTeardown.test.ts:14:28)

Check failure on line 14 in e2e/__tests__/environmentAfterTeardown.test.ts

View workflow job for this annotation

GitHub Actions / test-macos / Node LTS on macos-latest using jest-jasmine2 (2/4)

prints useful error for environment methods after test is done

expect(received).toMatchSnapshot() Snapshot name: `prints useful error for environment methods after test is done 1` - Snapshot - 5 + Received + 5 - ReferenceError: You are trying to access a property or method of the Jest environment outside of the scope of the test code. + Snapshots: 0 total + Time: 2.692 s + Ran all test suites. + + 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. 9 | test('access environment methods after done', () => { 10 | setTimeout(() => { > 11 | jest.clearAllTimers(); - | ^ - 12 | }, 0); - 13 | }); - 14 | at Object.toMatchSnapshot (e2e/__tests__/environmentAfterTeardown.test.ts:14:28)

Check failure on line 14 in e2e/__tests__/environmentAfterTeardown.test.ts

View workflow job for this annotation

GitHub Actions / test-windows / Node LTS on windows-latest using jest-jasmine2 (2/4)

prints useful error for environment methods after test is done

expect(received).toMatchSnapshot() Snapshot name: `prints useful error for environment methods after test is done 1` - Snapshot - 5 + Received + 5 - ReferenceError: You are trying to access a property or method of the Jest environment outside of the scope of the test code. + Snapshots: 0 total + Time: 1.392 s + Ran all test suites. + + 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. 9 | test('access environment methods after done', () => { 10 | setTimeout(() => { > 11 | jest.clearAllTimers(); - | ^ - 12 | }, 0); - 13 | }); - 14 | at Object.toMatchSnapshot (e2e/__tests__/environmentAfterTeardown.test.ts:14:28)
expect(stderr.split('\n')[9]).toBe(
'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.',
expect(stderr.split('\n')[5]).toMatch(
'ReferenceError: You are trying to access a property or method of the Jest environment outside of the scope of the test code.',
);
});
6 changes: 3 additions & 3 deletions e2e/__tests__/requireAfterTeardown.test.ts
Expand Up @@ -10,10 +10,10 @@ import runJest from '../runJest';
test('prints useful error for requires after test is done', () => {
const {stderr} = runJest('require-after-teardown');

const interestingLines = stderr.split('\n').slice(9, 18).join('\n');
const interestingLines = stderr.split('\n').slice(5, 14).join('\n');

expect(interestingLines).toMatchSnapshot();

Check failure on line 15 in e2e/__tests__/requireAfterTeardown.test.ts

View workflow job for this annotation

GitHub Actions / test-ubuntu / Node LTS on ubuntu-latest using jest-jasmine2 (2/4)

prints useful error for requires after test is done

expect(received).toMatchSnapshot() Snapshot name: `prints useful error for requires after test is done 1` - Snapshot - 5 + Received + 5 - ReferenceError: You are trying to `import` a file outside of the scope of the test code. + Snapshots: 0 total + Time: 0.855 s + Ran all test suites. + + ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From __tests__/lateRequire.test.js. 9 | test('require after done', () => { 10 | setTimeout(() => { > 11 | const double = require('../'); - | ^ - 12 | - 13 | expect(double(5)).toBe(10); - 14 | }, 0); at Object.toMatchSnapshot (e2e/__tests__/requireAfterTeardown.test.ts:15:28)

Check failure on line 15 in e2e/__tests__/requireAfterTeardown.test.ts

View workflow job for this annotation

GitHub Actions / test-macos / Node LTS on macos-latest using jest-jasmine2 (2/4)

prints useful error for requires after test is done

expect(received).toMatchSnapshot() Snapshot name: `prints useful error for requires after test is done 1` - Snapshot - 5 + Received + 5 - ReferenceError: You are trying to `import` a file outside of the scope of the test code. + Snapshots: 0 total + Time: 2.237 s + Ran all test suites. + + ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From __tests__/lateRequire.test.js. 9 | test('require after done', () => { 10 | setTimeout(() => { > 11 | const double = require('../'); - | ^ - 12 | - 13 | expect(double(5)).toBe(10); - 14 | }, 0); at Object.toMatchSnapshot (e2e/__tests__/requireAfterTeardown.test.ts:15:28)

Check failure on line 15 in e2e/__tests__/requireAfterTeardown.test.ts

View workflow job for this annotation

GitHub Actions / test-windows / Node LTS on windows-latest using jest-jasmine2 (2/4)

prints useful error for requires after test is done

expect(received).toMatchSnapshot() Snapshot name: `prints useful error for requires after test is done 1` - Snapshot - 5 + Received + 5 - ReferenceError: You are trying to `import` a file outside of the scope of the test code. + Snapshots: 0 total + Time: 1.76 s + Ran all test suites. + + ReferenceError: You are trying to `import` a file after the Jest environment has been torn down. From __tests__/lateRequire.test.js. 9 | test('require after done', () => { 10 | setTimeout(() => { > 11 | const double = require('../'); - | ^ - 12 | - 13 | expect(double(5)).toBe(10); - 14 | }, 0); at Object.toMatchSnapshot (e2e/__tests__/requireAfterTeardown.test.ts:15:28)
expect(stderr.split('\n')[19]).toMatch(
new RegExp('(__tests__/lateRequire.test.js:11:20)'),
expect(stderr.split('\n')[16]).toMatch(
'(__tests__/lateRequire.test.js:11:20)',
);
});
Expand Up @@ -9,5 +9,5 @@
test('access environment methods after done', () => {
setTimeout(() => {
jest.clearAllTimers();
}, 100);
}, 0);
});
2 changes: 1 addition & 1 deletion e2e/require-after-teardown/__tests__/lateRequire.test.js
Expand Up @@ -11,5 +11,5 @@ test('require after done', () => {
const double = require('../');

expect(double(5)).toBe(10);
}, 100);
}, 0);
});
Expand Up @@ -33,6 +33,7 @@ const jestAdapter = async (
globalConfig,
localRequire: runtime.requireModule.bind(runtime),
parentProcess: process,
runtime,
sendMessageToJest,
setGlobalsForRuntime: runtime.setGlobalsForRuntime.bind(runtime),
testPath,
Expand Down
Expand Up @@ -16,6 +16,7 @@ import {
} from '@jest/test-result';
import type {Circus, Config, Global} from '@jest/types';
import {formatExecError, formatResultsErrors} from 'jest-message-util';
import type Runtime from 'jest-runtime';
import {
SnapshotState,
addSerializer,
Expand All @@ -30,6 +31,7 @@ import {
getState as getRunnerState,
} from '../state';
import testCaseReportHandler from '../testCaseReportHandler';
import {unhandledRejectionHandler} from '../unhandledRejectionHandler';
import {getTestID} from '../utils';

interface RuntimeGlobals extends Global.TestFrameworkGlobals {
Expand All @@ -39,6 +41,7 @@ interface RuntimeGlobals extends Global.TestFrameworkGlobals {
export const initialize = async ({
config,
environment,
runtime,
globalConfig,
localRequire,
parentProcess,
Expand All @@ -48,6 +51,7 @@ export const initialize = async ({
}: {
config: Config.ProjectConfig;
environment: JestEnvironment;
runtime: Runtime;
globalConfig: Config.GlobalConfig;
localRequire: <T = unknown>(path: string) => T;
testPath: string;
Expand Down Expand Up @@ -129,6 +133,8 @@ export const initialize = async ({
addEventHandler(testCaseReportHandler(testPath, sendMessageToJest));
}

addEventHandler(unhandledRejectionHandler(runtime));

// Return it back to the outer scope (test runner outside the VM).
return {globals: globalsObject, snapshotState};
};
Expand Down
2 changes: 0 additions & 2 deletions packages/jest-circus/src/state.ts
Expand Up @@ -9,12 +9,10 @@ import type {Circus} from '@jest/types';
import eventHandler from './eventHandler';
import formatNodeAssertErrors from './formatNodeAssertErrors';
import {STATE_SYM} from './types';
import unhandledRejectionHandler from './unhandledRejectionHandler';
import {makeDescribe} from './utils';

const eventHandlers: Array<Circus.EventHandler> = [
eventHandler,
unhandledRejectionHandler,
formatNodeAssertErrors,
];

Expand Down
94 changes: 51 additions & 43 deletions packages/jest-circus/src/unhandledRejectionHandler.ts
Expand Up @@ -6,6 +6,7 @@
*/

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

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

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

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

if (type === 'beforeAll') {
invariant(describeBlock, 'always present for `*All` hooks');
for (const error of state.unhandledRejectionErrorByPromise.values()) {
addErrorToEachTestUnderDescribe(describeBlock, error, asyncError);
}
} else if (type === 'afterAll') {
// Attaching `afterAll` errors to each test makes execution flow
// too complicated, so we'll consider them to be global.
for (const error of state.unhandledRejectionErrorByPromise.values()) {
state.unhandledErrors.push([error, asyncError]);
const {test, describeBlock, hook} = event;
const {asyncError, type} = hook;

if (type === 'beforeAll') {
invariant(describeBlock, 'always present for `*All` hooks');
for (const error of state.unhandledRejectionErrorByPromise.values()) {
addErrorToEachTestUnderDescribe(describeBlock, error, asyncError);
}
} else if (type === 'afterAll') {
// Attaching `afterAll` errors to each test makes execution flow
// too complicated, so we'll consider them to be global.
for (const error of state.unhandledRejectionErrorByPromise.values()) {
state.unhandledErrors.push([error, asyncError]);
}
} else {
invariant(test, 'always present for `*Each` hooks');
for (const error of test.unhandledRejectionErrorByPromise.values()) {
test.errors.push([error, asyncError]);
}
}
} else {
} else if (event.name === 'test_fn_start') {
runtime.enterTestCode();
} else if (
event.name === 'test_fn_success' ||
event.name === 'test_fn_failure'
) {
runtime.leaveTestCode();

// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
await untilNextEventLoopTurn();

const {test} = event;
invariant(test, 'always present for `*Each` hooks');

for (const error of test.unhandledRejectionErrorByPromise.values()) {
test.errors.push([error, asyncError]);
test.errors.push([error, event.test.asyncError]);
}
}
} else if (
event.name === 'test_fn_success' ||
event.name === 'test_fn_failure'
) {
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
await untilNextEventLoopTurn();

const {test} = event;
invariant(test, 'always present for `*Each` hooks');
} else if (event.name === 'teardown') {
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
await untilNextEventLoopTurn();

for (const error of test.unhandledRejectionErrorByPromise.values()) {
test.errors.push([error, event.test.asyncError]);
state.unhandledErrors.push(
...state.unhandledRejectionErrorByPromise.values(),
);
}
} else if (event.name === 'teardown') {
// We need to give event loop the time to actually execute `rejectionHandled`, `uncaughtException` or `unhandledRejection` events
await untilNextEventLoopTurn();

state.unhandledErrors.push(
...state.unhandledRejectionErrorByPromise.values(),
);
}
};
};

export default unhandledRejectionHandler;
29 changes: 29 additions & 0 deletions packages/jest-runtime/src/index.ts
Expand Up @@ -208,6 +208,7 @@ export default class Runtime {
private readonly esmConditions: Array<string>;
private readonly cjsConditions: Array<string>;
private isTornDown = false;
private isInsideTestCode: boolean | undefined;

constructor(
config: Config.ProjectConfig,
Expand Down Expand Up @@ -562,6 +563,11 @@ export default class Runtime {
// @ts-expect-error - exiting
return;
}
if (this.isInsideTestCode === false) {
throw new ReferenceError(
'You are trying to `import` a file outside of the scope of the test code.',
);
}

if (specifier === '@jest/globals') {
const fromCache = this._esmoduleRegistry.get('@jest/globals');
Expand Down Expand Up @@ -706,6 +712,11 @@ export default class Runtime {
process.exitCode = 1;
return;
}
if (this.isInsideTestCode === false) {
throw new ReferenceError(
'You are trying to `import` a file outside of the scope of the test code.',
);
}

if (module.status === 'unlinked') {
// since we might attempt to link the same module in parallel, stick the promise in a weak map so every call to
Expand Down Expand Up @@ -1342,6 +1353,14 @@ export default class Runtime {
this._moduleMocker.clearAllMocks();
}

enterTestCode(): void {
this.isInsideTestCode = true;
}

leaveTestCode(): void {
this.isInsideTestCode = false;
}

teardown(): void {
this.restoreAllMocks();
this.resetModules();
Expand Down Expand Up @@ -1485,6 +1504,11 @@ export default class Runtime {
process.exitCode = 1;
return;
}
if (this.isInsideTestCode === false) {
throw new ReferenceError(
'You are trying to `import` a file outside of the scope of the test code.',
);
}

// If the environment was disposed, prevent this module from being executed.
if (!this._environment.global) {
Expand Down Expand Up @@ -2169,6 +2193,11 @@ export default class Runtime {
);
process.exitCode = 1;
}
if (this.isInsideTestCode === false) {
throw new ReferenceError(
'You are trying to access a property or method of the Jest environment outside of the scope of the test code.',
);
}

return this._fakeTimersImplementation!;
};
Expand Down

0 comments on commit b412305

Please sign in to comment.