From 998a02a53f53d503d8b4ba47437a4af6e9c582fc Mon Sep 17 00:00:00 2001 From: Adam Miskiewicz Date: Thu, 1 Sep 2016 17:02:44 -0700 Subject: [PATCH 1/3] Split internal module registry from main module registry --- packages/jest-runtime/src/index.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/jest-runtime/src/index.js b/packages/jest-runtime/src/index.js index 510f57949a79..98695477d51f 100644 --- a/packages/jest-runtime/src/index.js +++ b/packages/jest-runtime/src/index.js @@ -81,6 +81,7 @@ class Runtime { _mockRegistry: {[key: string]: any}; _mocksPattern: ?RegExp; _moduleRegistry: {[key: string]: Module}; + _internalModuleRegistry: {[key: string]: Module}; _resolver: Resolver; _shouldAutoMock: boolean; _shouldMockModuleCache: BooleanObject; @@ -96,6 +97,7 @@ class Runtime { resolver: Resolver, ) { this._moduleRegistry = Object.create(null); + this._internalModuleRegistry = Object.create(null); this._mockRegistry = Object.create(null); this._config = config; this._environment = environment; @@ -225,6 +227,10 @@ class Runtime { const moduleID = this._normalizeID(from, moduleName); let modulePath; + const moduleRegistry = (!options || !options.isInternalModule) ? + this._moduleRegistry : + this._internalModuleRegistry; + // Some old tests rely on this mocking behavior. Ideally we'll change this // to be more explicit. const moduleResource = moduleName && this._resolver.getModule(moduleName); @@ -249,7 +255,7 @@ class Runtime { modulePath = this._resolveModule(from, moduleName); } - if (!this._moduleRegistry[modulePath]) { + if (!moduleRegistry[modulePath]) { // We must register the pre-allocated module object first so that any // circular dependencies that may arise while evaluating the module can // be satisfied. @@ -257,7 +263,7 @@ class Runtime { filename: modulePath, exports: {}, }; - this._moduleRegistry[modulePath] = localModule; + moduleRegistry[modulePath] = localModule; if (path.extname(modulePath) === '.json') { localModule.exports = this._environment.global.JSON.parse( fs.readFileSync(modulePath, 'utf8'), @@ -269,7 +275,7 @@ class Runtime { this._execModule(localModule, options); } } - return this._moduleRegistry[modulePath].exports; + return moduleRegistry[modulePath].exports; } requireInternalModule(from: Path, to?: string) { From 882a1d2e09083793104561c38c8efbe46f59a2af Mon Sep 17 00:00:00 2001 From: Adam Miskiewicz Date: Fri, 2 Sep 2016 09:50:21 -0700 Subject: [PATCH 2/3] Add integration test for internal module registry --- .../runtime-internal-module-registry-test.js | 30 +++++++++++++++++++ .../__mocks__/fs.js | 25 ++++++++++++++++ .../runtime-internal-module-registry-test.js | 28 +++++++++++++++++ .../package.json | 5 ++++ 4 files changed, 88 insertions(+) create mode 100644 integration_tests/__tests__/runtime-internal-module-registry-test.js create mode 100644 integration_tests/runtime-internal-module-registry/__mocks__/fs.js create mode 100644 integration_tests/runtime-internal-module-registry/__tests__/runtime-internal-module-registry-test.js create mode 100644 integration_tests/runtime-internal-module-registry/package.json diff --git a/integration_tests/__tests__/runtime-internal-module-registry-test.js b/integration_tests/__tests__/runtime-internal-module-registry-test.js new file mode 100644 index 000000000000..54b7fe4a3079 --- /dev/null +++ b/integration_tests/__tests__/runtime-internal-module-registry-test.js @@ -0,0 +1,30 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails oncall+jsinfra + */ +'use strict'; + +const runJest = require('../runJest'); + +describe('Runtime Internal Module Registry', () => { + // Previously, if Jest required a module (e.g. requiring `mkdirp` from `jest-util`) + // and the project *using* Jest also required that module, Jest wouldn't + // re-require it and thus ignored any mocks that the module may have used. + // + // This test verifies that that behavior doesn't happen anymore, and correctly + // uses two module registries: an internal registry that's used specificly by + // Jest to require any internal modules used when setting up the test environment, + // and a "normal" module registry that's used by the actual test code (and can safely + // be cleared after every test) + it('correctly makes use of internal module registry when requiring modules used both in Jest and in a project', () => { + const {json} = runJest.json('runtime-internal-module-registry', []); + + expect(json.numTotalTests).toBe(1); + expect(json.numPassedTests).toBe(1); + }); +}); diff --git a/integration_tests/runtime-internal-module-registry/__mocks__/fs.js b/integration_tests/runtime-internal-module-registry/__mocks__/fs.js new file mode 100644 index 000000000000..e4935d7d5d9b --- /dev/null +++ b/integration_tests/runtime-internal-module-registry/__mocks__/fs.js @@ -0,0 +1,25 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +'use strict'; + +const fs = jest.genMockFromModule('fs'); + +let mkdirWasCalled = false; + +function mkdirSync() { + mkdirWasCalled = true; + return; +} + +fs.mkdirSync = mkdirSync; +fs.__wasMkdirCalled = function() { + return mkdirWasCalled; +} + +module.exports = fs; diff --git a/integration_tests/runtime-internal-module-registry/__tests__/runtime-internal-module-registry-test.js b/integration_tests/runtime-internal-module-registry/__tests__/runtime-internal-module-registry-test.js new file mode 100644 index 000000000000..3150859d81c6 --- /dev/null +++ b/integration_tests/runtime-internal-module-registry/__tests__/runtime-internal-module-registry-test.js @@ -0,0 +1,28 @@ +/** + * Copyright (c) 2014-present, Facebook, Inc. All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +'use strict'; + +jest.mock('fs'); + +describe('Runtime internal module registry', () => { + it('behaves correctly when requiring a module that is used by jest internals', () => { + const fs = require('fs'); + + // We require from this crazy path so that we can mimick Jest (and it's + // transitive deps) being installed along side a projects deps (e.g. with an + // NPM3 flat dep tree) + const jestUtil = require('../../../packages/jest-util'); + + // If FS is mocked correctly, this folder won't actually be created on the + // filesystem + jestUtil.createDirectory('./dont-create-this-folder'); + + expect(fs.__wasMkdirCalled()).toBe(true); + }); +}); diff --git a/integration_tests/runtime-internal-module-registry/package.json b/integration_tests/runtime-internal-module-registry/package.json new file mode 100644 index 000000000000..148788b25446 --- /dev/null +++ b/integration_tests/runtime-internal-module-registry/package.json @@ -0,0 +1,5 @@ +{ + "jest": { + "testEnvironment": "node" + } +} From 7dc0ddaa4436deea298a32218c4c36d4da07d979 Mon Sep 17 00:00:00 2001 From: Adam Miskiewicz Date: Fri, 2 Sep 2016 13:48:49 -0700 Subject: [PATCH 3/3] Fix lint --- .../runtime-internal-module-registry-test.js | 25 +++++++++++-------- .../__mocks__/fs.js | 2 +- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/integration_tests/__tests__/runtime-internal-module-registry-test.js b/integration_tests/__tests__/runtime-internal-module-registry-test.js index 54b7fe4a3079..89fbe6a44b11 100644 --- a/integration_tests/__tests__/runtime-internal-module-registry-test.js +++ b/integration_tests/__tests__/runtime-internal-module-registry-test.js @@ -12,19 +12,22 @@ const runJest = require('../runJest'); describe('Runtime Internal Module Registry', () => { - // Previously, if Jest required a module (e.g. requiring `mkdirp` from `jest-util`) - // and the project *using* Jest also required that module, Jest wouldn't - // re-require it and thus ignored any mocks that the module may have used. + // Previously, if Jest required a module (e.g. requiring `mkdirp` from + // `jest-util`) and the project *using* Jest also required that module, Jest + // wouldn't re-require it and thus ignored any mocks that the module may have + // used. // // This test verifies that that behavior doesn't happen anymore, and correctly // uses two module registries: an internal registry that's used specificly by - // Jest to require any internal modules used when setting up the test environment, - // and a "normal" module registry that's used by the actual test code (and can safely - // be cleared after every test) - it('correctly makes use of internal module registry when requiring modules used both in Jest and in a project', () => { - const {json} = runJest.json('runtime-internal-module-registry', []); + // Jest to require any internal modules used when setting up the test + // environment, and a "normal" module registry that's used by the actual test + // code (and can safely be cleared after every test) + it('correctly makes use of internal module registry when requiring modules', + () => { + const {json} = runJest.json('runtime-internal-module-registry', []); - expect(json.numTotalTests).toBe(1); - expect(json.numPassedTests).toBe(1); - }); + expect(json.numTotalTests).toBe(1); + expect(json.numPassedTests).toBe(1); + } + ); }); diff --git a/integration_tests/runtime-internal-module-registry/__mocks__/fs.js b/integration_tests/runtime-internal-module-registry/__mocks__/fs.js index e4935d7d5d9b..0f30b438a389 100644 --- a/integration_tests/runtime-internal-module-registry/__mocks__/fs.js +++ b/integration_tests/runtime-internal-module-registry/__mocks__/fs.js @@ -20,6 +20,6 @@ function mkdirSync() { fs.mkdirSync = mkdirSync; fs.__wasMkdirCalled = function() { return mkdirWasCalled; -} +}; module.exports = fs;