Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix npm3 module resolution + more caching. #732

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
*.swp
.DS_STORE
.haste_cache
node_modules
/node_modules
/examples/**/node_modules/
/website/node_modules
npm-debug.log
build
website/core/metadata.js
Expand Down
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
* Added a "no tests found" message if no tests can be found.
* Debounce `--watch` re-runs to not trigger test runs during a
branch switch in version control.
* Fixed test runtime error reporting and stack traces
* Fixed mocking of boolean values.
* Fixed unmocking behavior with npm3.
* Fixed test runtime error reporting and stack traces.

## 0.8.2

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "jest-cli",
"description": "Painless JavaScript Unit Testing.",
"version": "0.9.0-fb2",
"version": "0.9.0-fb3",
"main": "src/jest.js",
"dependencies": {
"chalk": "^1.1.1",
Expand Down
115 changes: 45 additions & 70 deletions src/HasteModuleLoader/HasteModuleLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ const transform = require('../lib/transform');

const NODE_PATH =
(process.env.NODE_PATH ? process.env.NODE_PATH.split(path.delimiter) : null);
const NODE_MODULES = path.sep + 'node_modules' + path.sep;
const IS_PATH_BASED_MODULE_NAME = /^(?:\.\.?\/|\/)/;
const VENDOR_PATH = path.resolve(__dirname, '../../vendor');

const mockParentModule = {
id: 'mockParent',
Expand All @@ -27,20 +27,9 @@ const mockParentModule = {

const normalizedIDCache = Object.create(null);
const moduleNameCache = Object.create(null);
const shouldMockModuleCache = Object.create(null);

const isFile = file => {
let stat;
try {
stat = fs.statSync(file);
} catch (err) {
if (err && err.code === 'ENOENT') {
return false;
}
}
return stat.isFile() || stat.isFIFO();
};

let _configUnmockListRegExpCache = null;
const unmockRegExpCache = new WeakMap();

class Loader {
constructor(config, environment, moduleMap) {
Expand All @@ -53,7 +42,6 @@ class Loader {
this._isCurrentlyExecutingManualMock = null;
this._mockMetaDataCache = Object.create(null);
this._shouldAutoMock = true;
this._configShouldMockModuleNames = Object.create(null);
this._extensions = config.moduleFileExtensions.map(ext => '.' + ext);

this._modules = moduleMap.modules;
Expand All @@ -63,22 +51,11 @@ class Loader {
this._CoverageCollector = require(config.coverageCollector);
}

if (_configUnmockListRegExpCache === null) {
_configUnmockListRegExpCache = new WeakMap();
}

if (
!config.unmockedModulePathPatterns ||
config.unmockedModulePathPatterns.length === 0
) {
this._unmockListRegExps = [];
} else {
this._unmockListRegExps = _configUnmockListRegExpCache.get(config);
if (!this._unmockListRegExps) {
this._unmockListRegExps = config.unmockedModulePathPatterns
.map(unmockPathRe => new RegExp(unmockPathRe));
_configUnmockListRegExpCache.set(config, this._unmockListRegExps);
}
this._unmockedList = unmockRegExpCache.get(config);
if (!this._unmockedList && config.unmockedModulePathPatterns) {
this._unmockedList =
new RegExp(config.unmockedModulePathPatterns.join('|'));
unmockRegExpCache.set(config, this._unmockedList);
}

// Workers communicate the config as JSON so we have to create a regex
Expand Down Expand Up @@ -135,11 +112,6 @@ class Loader {
modulePath = this._resolveModuleName(currPath, moduleName);
}

// Always natively require the jasmine runner.
if (modulePath.indexOf(VENDOR_PATH) === 0) {
return require(modulePath);
}

if (!modulePath) {
throw new Error(`Cannot find module '${moduleName}' from '${currPath}'`);
}
Expand Down Expand Up @@ -289,7 +261,7 @@ class Loader {
);
let moduleContent = transform(filename, this._config);
let collectorStore;
if (shouldCollectCoverage && !filename.includes('/node_modules/')) {
if (shouldCollectCoverage && !filename.includes(NODE_MODULES)) {
if (!collectors[filename]) {
collectors[filename] = new this._CoverageCollector(
moduleContent,
Expand Down Expand Up @@ -373,15 +345,15 @@ class Loader {
// Check if the resolver knows about this module
if (this._modules[moduleName]) {
return this._modules[moduleName];
} else {
// Otherwise it is likely a node_module.
const key = currPath + ' : ' + moduleName;
if (moduleNameCache[key]) {
return moduleNameCache[key];
}
moduleNameCache[key] = this._resolveNodeModule(currPath, moduleName);
}

// Otherwise it is likely a node_module.
const key = currPath + ' : ' + moduleName;
if (moduleNameCache[key]) {
return moduleNameCache[key];
}
moduleNameCache[key] = this._resolveNodeModule(currPath, moduleName);
return moduleNameCache[key];
}

_resolveNodeModule(currPath, moduleName) {
Expand All @@ -390,9 +362,7 @@ class Loader {
return resolve.sync(moduleName, {
basedir,
extensions: this._extensions,
isFile,
paths: NODE_PATH,
readFileSync: fs.readFileSync,
});
} catch (e) {
const parts = moduleName.split('/');
Expand Down Expand Up @@ -447,10 +417,7 @@ class Loader {
moduleType = 'user';
if (
IS_PATH_BASED_MODULE_NAME.test(moduleName) ||
(
this._getModule(moduleName) === undefined &&
this._getMockModule(moduleName) === undefined
)
(!this._getModule(moduleName) && !this._getMockModule(moduleName))
) {
realAbsPath = this._resolveModuleName(currPath, moduleName);
if (realAbsPath == null) {
Expand Down Expand Up @@ -491,16 +458,18 @@ class Loader {
const moduleID = this._getNormalizedModuleID(currPath, moduleName);
if (moduleID in this._explicitShouldMock) {
return this._explicitShouldMock[moduleID];
} else if (resolve.isCore(moduleName)) {
}

if (resolve.isCore(moduleName)) {
return false;
} else if (this._shouldAutoMock) {
}

if (this._shouldAutoMock) {
// See if the module is specified in the config as a module that should
// never be mocked
if (moduleName in this._configShouldMockModuleNames) {
return this._configShouldMockModuleNames[moduleName];
} else if (this._unmockListRegExps.length > 0) {
this._configShouldMockModuleNames[moduleName] = true;

if (moduleName in shouldMockModuleCache) {
return shouldMockModuleCache[moduleName];
} else if (this._unmockedList) {
const manualMockResource = this._getMockModule(moduleName);
let modulePath;
try {
Expand All @@ -521,27 +490,33 @@ class Loader {
// all module environments are complete (meaning you can't just
// write a manual mock as a substitute for a real module).
if (manualMockResource) {
shouldMockModuleCache[moduleName] = true;
return true;
}
throw e;
}
let unmockRegExp;

// Never mock the jasmine environment.
if (modulePath.indexOf(VENDOR_PATH) === 0) {
const realPath = fs.realpathSync(modulePath);
if (this._unmockedList.test(realPath)) {
shouldMockModuleCache[moduleName] = false;
} else if (
currPath.includes(NODE_MODULES) &&
realPath.includes(NODE_MODULES) &&
(
!shouldMockModuleCache[moduleName] ||
this._unmockedList.test(currPath)
)
) {
// If the dependency should normally be mocked but the parent
// module is a node module that is not being mocked, the target module
// should be unmocked too. This transitive behavior is useful for flat
// package managers, like npm3.
return false;
} else {
shouldMockModuleCache[moduleName] = true;
}

const realPath = fs.realpathSync(modulePath);
this._configShouldMockModuleNames[moduleName] = true;
for (let i = 0; i < this._unmockListRegExps.length; i++) {
unmockRegExp = this._unmockListRegExps[i];
if (unmockRegExp.test(modulePath) ||
unmockRegExp.test(realPath)) {
return this._configShouldMockModuleNames[moduleName] = false;
}
}
return this._configShouldMockModuleNames[moduleName];
return shouldMockModuleCache[moduleName];
}
return true;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('HasteModuleLoader', function() {
cacheDirectory: global.CACHE_DIRECTORY,
name: 'HasteModuleLoader-requireModule-tests',
rootDir,
unmockedModulePathPatterns: ['npm3-main-dep'],
});

function buildLoader() {
Expand Down Expand Up @@ -143,51 +144,38 @@ describe('HasteModuleLoader', function() {
});
});

describe('features I want to remove, but must exist for now', function() {
/**
* I'd like to kill this and make all tests use something more explicit
* when they want a manual mock, like:
*
* require.mock('MyManualMock');
* const ManuallyMocked = require('ManuallyMocked');
*
* --or--
*
* const ManuallyMocked = require.manualMock('ManuallyMocked');
*
* For now, however, this is built-in and many tests rely on it, so we
* must support it until we can do some cleanup.
*/
pit('provides manual mock when real module doesnt exist', function() {
return buildLoader().then(function(loader) {
const exports = loader.requireModule(
rootPath,
'ExclusivelyManualMock'
);
expect(exports.isExclusivelyManualMockModule).toBe(true);
});
pit('provides manual mock when real module doesnt exist', function() {
return buildLoader().then(function(loader) {
const exports = loader.requireModule(
rootPath,
'ExclusivelyManualMock'
);
expect(exports.isExclusivelyManualMockModule).toBe(true);
});
});

pit(
'doesnt override real modules with manual mocks when explicitly ' +
'marked with .dontMock()',
function() {
return buildLoader().then(function(loader) {
const root = loader.requireModule(rootPath, './root.js');
root.jest.resetModuleRegistry();
root.jest.dontMock('ManuallyMocked');
const exports = loader.requireModule(rootPath, 'ManuallyMocked');
expect(exports.isManualMockModule).toBe(false);
});
}
);

/**
* requireModule() should *always* return the real module. Mocks should
* only be returned by requireMock().
*
* See the 'overrides real modules with manual mock when one exists' test
* for more info on why I want to kill this feature.
*/
pit(
'doesnt override real modules with manual mocks when explicitly ' +
'marked with .dontMock()',
function() {
return buildLoader().then(function(loader) {
const root = loader.requireModule(rootPath, './root.js');
root.jest.resetModuleRegistry();
root.jest.dontMock('ManuallyMocked');
const exports = loader.requireModule(rootPath, 'ManuallyMocked');
expect(exports.isManualMockModule).toBe(false);
});
}
);
pit('unmocks transitive dependencies in node_modules by default', () => {
return buildLoader().then(loader => {
const nodeModule = loader.requireModule(rootPath, 'npm3-main-dep');
expect(nodeModule()).toEqual({
isMocked: false,
transitiveNPM3Dep: 'npm3-transitive-dep',
});
});
});
});
});
2 changes: 2 additions & 0 deletions src/HasteModuleLoader/__tests__/test_root/ManuallyMocked.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@
* @providesModule ManuallyMocked
*/

'use strict';

exports.isManualMockModule = false;
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@
* 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';

exports.isExclusivelyManualMockModule = true;
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 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';

let OnlyRequiredFromMock;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions src/HasteModuleLoader/__tests__/test_root/root.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
* @providesModule root
*/

'use strict';

require('ManuallyMocked');
require('ModuleWithSideEffects');
require('RegularModule');
Expand Down
3 changes: 3 additions & 0 deletions src/HasteModuleLoader/__tests__/test_root/subdir1/MyModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,7 @@
* 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';

exports.modulePath = 'subdir1/MyModule.js';