From 06819596dd6f9550b1c67d6c5f351c8fff7fbc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20S=CC=8Ctekl?= Date: Fri, 26 May 2023 13:38:15 +0200 Subject: [PATCH] Fix reporting of requires or Jest env method access after "teardown" --- .../environmentAfterTeardown.test.ts.snap | 20 +++- .../requireAfterTeardown.test.ts.snap | 18 +++- .../environmentAfterTeardown.test.ts | 6 +- e2e/__tests__/requireAfterTeardown.test.ts | 7 +- .../__tests__/afterTeardown.test.js | 2 +- .../__tests__/lateRequire.test.js | 2 +- .../legacy-code-todo-rewrite/jestAdapter.ts | 1 + .../jestAdapterInit.ts | 6 ++ packages/jest-circus/src/state.ts | 2 - .../src/unhandledRejectionHandler.ts | 94 ++++++++++--------- packages/jest-runtime/src/index.ts | 29 ++++++ 11 files changed, 124 insertions(+), 63 deletions(-) diff --git a/e2e/__tests__/__snapshots__/environmentAfterTeardown.test.ts.snap b/e2e/__tests__/__snapshots__/environmentAfterTeardown.test.ts.snap index 2af30dfb6bbf..d235588954c7 100644 --- a/e2e/__tests__/__snapshots__/environmentAfterTeardown.test.ts.snap +++ b/e2e/__tests__/__snapshots__/environmentAfterTeardown.test.ts.snap @@ -1,13 +1,27 @@ // 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. +"FAIL __tests__/afterTeardown.test.js + ✕ access environment methods after done (3 ms) + + ● access environment methods after done + + 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 |" + 14 | + + at _getFakeTimers (../../packages/jest-runtime/build/index.js:1977:15) + at Timeout.clearAllTimers [as _onTimeout] (__tests__/afterTeardown.test.js:11:10) + +Test Suites: 1 failed, 1 total +Tests: 1 failed, 1 total +Snapshots: 0 total +Time: 0.365 s, estimated 1 s +Ran all test suites." `; diff --git a/e2e/__tests__/__snapshots__/requireAfterTeardown.test.ts.snap b/e2e/__tests__/__snapshots__/requireAfterTeardown.test.ts.snap index 32009e6f8db4..8bac52866d70 100644 --- a/e2e/__tests__/__snapshots__/requireAfterTeardown.test.ts.snap +++ b/e2e/__tests__/__snapshots__/requireAfterTeardown.test.ts.snap @@ -1,7 +1,12 @@ // 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. +"FAIL __tests__/lateRequire.test.js + ✕ require after done (5 ms) + + ● require after done + + ReferenceError: You are trying to \`import\` a file outside of the scope of the test code. 9 | test('require after done', () => { 10 | setTimeout(() => { @@ -9,5 +14,14 @@ exports[`prints useful error for requires after test is done 1`] = ` | ^ 12 | 13 | expect(double(5)).toBe(10); - 14 | }, 100);" + 14 | }, 0); + + at Runtime._execModule (../../packages/jest-runtime/build/index.js:1380:13) + at Timeout.require [as _onTimeout] (__tests__/lateRequire.test.js:11:20) + +Test Suites: 1 failed, 1 total +Tests: 1 failed, 1 total +Snapshots: 0 total +Time: 0.332 s, estimated 1 s +Ran all test suites." `; diff --git a/e2e/__tests__/environmentAfterTeardown.test.ts b/e2e/__tests__/environmentAfterTeardown.test.ts index 1216014a2af4..46a3508bdff1 100644 --- a/e2e/__tests__/environmentAfterTeardown.test.ts +++ b/e2e/__tests__/environmentAfterTeardown.test.ts @@ -9,10 +9,6 @@ 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'); - expect(interestingLines).toMatchSnapshot(); - 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).toMatchSnapshot(); }); diff --git a/e2e/__tests__/requireAfterTeardown.test.ts b/e2e/__tests__/requireAfterTeardown.test.ts index cb9607549b85..b6b24211c39f 100644 --- a/e2e/__tests__/requireAfterTeardown.test.ts +++ b/e2e/__tests__/requireAfterTeardown.test.ts @@ -10,10 +10,5 @@ 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'); - - expect(interestingLines).toMatchSnapshot(); - expect(stderr.split('\n')[19]).toMatch( - new RegExp('(__tests__/lateRequire.test.js:11:20)'), - ); + expect(stderr).toMatchSnapshot(); }); diff --git a/e2e/environment-after-teardown/__tests__/afterTeardown.test.js b/e2e/environment-after-teardown/__tests__/afterTeardown.test.js index dbc9bb1231f1..fb23021297b3 100644 --- a/e2e/environment-after-teardown/__tests__/afterTeardown.test.js +++ b/e2e/environment-after-teardown/__tests__/afterTeardown.test.js @@ -9,5 +9,5 @@ test('access environment methods after done', () => { setTimeout(() => { jest.clearAllTimers(); - }, 100); + }, 0); }); diff --git a/e2e/require-after-teardown/__tests__/lateRequire.test.js b/e2e/require-after-teardown/__tests__/lateRequire.test.js index 1316a525ee2e..5ad13a131afd 100644 --- a/e2e/require-after-teardown/__tests__/lateRequire.test.js +++ b/e2e/require-after-teardown/__tests__/lateRequire.test.js @@ -11,5 +11,5 @@ test('require after done', () => { const double = require('../'); expect(double(5)).toBe(10); - }, 100); + }, 0); }); diff --git a/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts b/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts index 80b4503621b7..1aae32296423 100644 --- a/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts +++ b/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapter.ts @@ -33,6 +33,7 @@ const jestAdapter = async ( globalConfig, localRequire: runtime.requireModule.bind(runtime), parentProcess: process, + runtime, sendMessageToJest, setGlobalsForRuntime: runtime.setGlobalsForRuntime.bind(runtime), testPath, diff --git a/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts b/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts index f9ac8076bd55..416d6555ddbf 100644 --- a/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts +++ b/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts @@ -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, @@ -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 { @@ -39,6 +41,7 @@ interface RuntimeGlobals extends Global.TestFrameworkGlobals { export const initialize = async ({ config, environment, + runtime, globalConfig, localRequire, parentProcess, @@ -48,6 +51,7 @@ export const initialize = async ({ }: { config: Config.ProjectConfig; environment: JestEnvironment; + runtime: Runtime; globalConfig: Config.GlobalConfig; localRequire: (path: string) => T; testPath: string; @@ -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}; }; diff --git a/packages/jest-circus/src/state.ts b/packages/jest-circus/src/state.ts index 4c5141f3f6e4..5dbc152f8399 100644 --- a/packages/jest-circus/src/state.ts +++ b/packages/jest-circus/src/state.ts @@ -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 = [ eventHandler, - unhandledRejectionHandler, formatNodeAssertErrors, ]; diff --git a/packages/jest-circus/src/unhandledRejectionHandler.ts b/packages/jest-circus/src/unhandledRejectionHandler.ts index c8c6c842ac6d..309f9e0b2e10 100644 --- a/packages/jest-circus/src/unhandledRejectionHandler.ts +++ b/packages/jest-circus/src/unhandledRejectionHandler.ts @@ -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 @@ -18,55 +19,62 @@ const untilNextEventLoopTurn = async () => { }); }; -const unhandledRejectionHandler: Circus.EventHandler = async ( - event, - state, -): Promise => { - 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; diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index f9945f8e7f72..b2e1735fb488 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -208,6 +208,7 @@ export default class Runtime { private readonly esmConditions: Array; private readonly cjsConditions: Array; private isTornDown = false; + private isInsideTestCode: boolean | undefined; constructor( config: Config.ProjectConfig, @@ -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'); @@ -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 @@ -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(); @@ -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) { @@ -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!; };