From bb858716abe96dc5c668117b233341daec9685cf Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 28 Mar 2018 12:13:08 +0200 Subject: [PATCH 1/3] Throw a more useful error when trying to require modules after the test environment is torn down --- CHANGELOG.md | 3 +++ .../require_after_teardown.test.js.snap | 15 ++++++++++++ .../__tests__/require_after_teardown.test.js | 23 +++++++++++++++++++ .../__tests__/late-require.test.js | 15 ++++++++++++ .../require-after-teardown/index.js | 10 ++++++++ .../require-after-teardown/package.json | 5 ++++ packages/jest-runtime/package.json | 1 + packages/jest-runtime/src/index.js | 22 +++++++++++++++--- 8 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap create mode 100644 integration-tests/__tests__/require_after_teardown.test.js create mode 100644 integration-tests/require-after-teardown/__tests__/late-require.test.js create mode 100644 integration-tests/require-after-teardown/index.js create mode 100644 integration-tests/require-after-teardown/package.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 47daaf340e1a..20bcb3e1a7ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,9 @@ ### Fixes +* `[jest-runtime]` Throw a more useful error when trying to require modules + after the test environment is torn down + ([#5888](https://github.com/facebook/jest/pull/5888)) * `[jest-jasmine2]` Install `sourcemap-support` into normal runtime to catch runtime errors ([#5945](https://github.com/facebook/jest/pull/5945)) * `[jest-jasmine2]` Added assertion error handling inside `afterAll hook` diff --git a/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap b/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap new file mode 100644 index 000000000000..e8d1929f72a2 --- /dev/null +++ b/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap @@ -0,0 +1,15 @@ +// 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. + + 9 | test('require after done', () => { + 10 | setTimeout(() => { + > 11 | const double = require('../'); + 12 | + 13 | expect(double(5)).toBe(10); + 14 | }, 0); + + at Runtime._execModule (../../packages/jest-runtime/build/index.js:585:9) + at Timeout.setTimeout [as _onTimeout] (__tests__/late-require.test.js:11:20)" +`; diff --git a/integration-tests/__tests__/require_after_teardown.test.js b/integration-tests/__tests__/require_after_teardown.test.js new file mode 100644 index 000000000000..66a4dcbdf046 --- /dev/null +++ b/integration-tests/__tests__/require_after_teardown.test.js @@ -0,0 +1,23 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. 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. + * + * @flow + */ + +'use strict'; + +const runJest = require('../runJest'); + +test('prints useful error for requires after test is done', () => { + const {stderr} = runJest('require-after-teardown'); + + const interestingLines = stderr + .split('\n') + .slice(9, 20) + .join('\n'); + + expect(interestingLines).toMatchSnapshot(); +}); diff --git a/integration-tests/require-after-teardown/__tests__/late-require.test.js b/integration-tests/require-after-teardown/__tests__/late-require.test.js new file mode 100644 index 000000000000..116a9ddde81e --- /dev/null +++ b/integration-tests/require-after-teardown/__tests__/late-require.test.js @@ -0,0 +1,15 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. 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. + */ +'use strict'; + +test('require after done', () => { + setTimeout(() => { + const double = require('../'); + + expect(double(5)).toBe(10); + }, 0); +}); diff --git a/integration-tests/require-after-teardown/index.js b/integration-tests/require-after-teardown/index.js new file mode 100644 index 000000000000..39261f1e45e9 --- /dev/null +++ b/integration-tests/require-after-teardown/index.js @@ -0,0 +1,10 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. 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. + */ + +module.exports = function double(input) { + return input * 2; +}; diff --git a/integration-tests/require-after-teardown/package.json b/integration-tests/require-after-teardown/package.json new file mode 100644 index 000000000000..148788b25446 --- /dev/null +++ b/integration-tests/require-after-teardown/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +} diff --git a/packages/jest-runtime/package.json b/packages/jest-runtime/package.json index c4cb00c48713..0469662a3163 100644 --- a/packages/jest-runtime/package.json +++ b/packages/jest-runtime/package.json @@ -16,6 +16,7 @@ "graceful-fs": "^4.1.11", "jest-config": "^22.4.2", "jest-haste-map": "^22.4.2", + "jest-message-util": "^22.4.0", "jest-regex-util": "^22.1.0", "jest-resolve": "^22.4.2", "jest-snapshot": "^22.4.0", diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index d179cdfbb81b..259b4cddad33 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -19,6 +19,7 @@ import type {SourceMapRegistry} from 'types/SourceMaps'; import path from 'path'; import HasteMap from 'jest-haste-map'; +import {formatStackTrace, separateMessageFromStack} from 'jest-message-util'; import Resolver from 'jest-resolve'; import {createDirectory, deepCyclicCopy} from 'jest-util'; import {escapePathForRegex} from 'jest-regex-util'; @@ -551,9 +552,24 @@ class Runtime { } } - const wrapper = this._environment.runScript(transformedFile.script)[ - ScriptTransformer.EVAL_RESULT_VARIABLE - ]; + const runScript = this._environment.runScript(transformedFile.script); + + if (runScript === null) { + const {message, stack} = separateMessageFromStack( + new ReferenceError( + 'You are trying to `import` a file after the Jest environment has been torn down.', + ).stack, + ); + + console.error( + `\n${message}\n` + + formatStackTrace(stack, this._config, {noStackTrace: false}), + ); + process.exitCode = 1; + return; + } + + const wrapper = runScript[ScriptTransformer.EVAL_RESULT_VARIABLE]; wrapper.call( localModule.exports, // module context localModule, // module object From 8dd85f42994158338338bbfd7f6a6d97b6da3818 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 28 Mar 2018 12:25:00 +0200 Subject: [PATCH 2/3] remove jest-runtime from stack entirely --- .../require_after_teardown.test.js.snap | 1 - .../__tests__/require_after_teardown.test.js | 2 +- packages/jest-runtime/src/index.js | 14 +++++++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap b/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap index e8d1929f72a2..9918115429f3 100644 --- a/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap +++ b/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap @@ -10,6 +10,5 @@ exports[`prints useful error for requires after test is done 1`] = ` 13 | expect(double(5)).toBe(10); 14 | }, 0); - at Runtime._execModule (../../packages/jest-runtime/build/index.js:585:9) at Timeout.setTimeout [as _onTimeout] (__tests__/late-require.test.js:11:20)" `; diff --git a/integration-tests/__tests__/require_after_teardown.test.js b/integration-tests/__tests__/require_after_teardown.test.js index 66a4dcbdf046..eab7b494c0e5 100644 --- a/integration-tests/__tests__/require_after_teardown.test.js +++ b/integration-tests/__tests__/require_after_teardown.test.js @@ -16,7 +16,7 @@ test('prints useful error for requires after test is done', () => { const interestingLines = stderr .split('\n') - .slice(9, 20) + .slice(9, 19) .join('\n'); expect(interestingLines).toMatchSnapshot(); diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index 259b4cddad33..b21208815648 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -555,11 +555,15 @@ class Runtime { const runScript = this._environment.runScript(transformedFile.script); if (runScript === null) { - const {message, stack} = separateMessageFromStack( - new ReferenceError( - 'You are trying to `import` a file after the Jest environment has been torn down.', - ).stack, - ); + const originalStack = new ReferenceError( + 'You are trying to `import` a file after the Jest environment has been torn down.', + ).stack + .split('\n') + // Remove this file from the stack (jest-message-utils will keep one line) + .filter(line => line.indexOf(__filename) === -1) + .join('\n'); + + const {message, stack} = separateMessageFromStack(originalStack); console.error( `\n${message}\n` + From 21818f5e89318366711fc27cbd631e9cec32958d Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 28 Mar 2018 13:29:57 +0200 Subject: [PATCH 3/3] fix test in node 6 --- .../__snapshots__/require_after_teardown.test.js.snap | 4 +--- integration-tests/__tests__/require_after_teardown.test.js | 5 ++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap b/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap index 9918115429f3..e200d37782aa 100644 --- a/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap +++ b/integration-tests/__tests__/__snapshots__/require_after_teardown.test.js.snap @@ -8,7 +8,5 @@ exports[`prints useful error for requires after test is done 1`] = ` > 11 | const double = require('../'); 12 | 13 | expect(double(5)).toBe(10); - 14 | }, 0); - - at Timeout.setTimeout [as _onTimeout] (__tests__/late-require.test.js:11:20)" + 14 | }, 0);" `; diff --git a/integration-tests/__tests__/require_after_teardown.test.js b/integration-tests/__tests__/require_after_teardown.test.js index eab7b494c0e5..1a13b9ac5d38 100644 --- a/integration-tests/__tests__/require_after_teardown.test.js +++ b/integration-tests/__tests__/require_after_teardown.test.js @@ -16,8 +16,11 @@ test('prints useful error for requires after test is done', () => { const interestingLines = stderr .split('\n') - .slice(9, 19) + .slice(9, 17) .join('\n'); expect(interestingLines).toMatchSnapshot(); + expect(stderr.split('\n')[18]).toMatch( + new RegExp('(__tests__/late-require.test.js:11:20)'), + ); });