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

Fixed code coverage on Windows #499

Closed
wants to merge 4 commits 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/HasteModuleLoader/HasteModuleLoader.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,11 +396,11 @@ Loader.prototype._getNormalizedModuleID = function(currPath, moduleName) {
}
}

return [moduleType, realAbsPath, mockAbsPath].join(':');
return [moduleType, realAbsPath, mockAbsPath].join(path.delimiter);
};

Loader.prototype._getRealPathFromNormalizedModuleID = function(moduleID) {
return moduleID.split(':')[1];
return moduleID.split(path.delimiter)[1];
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ describe('HasteModuleLoader', function() {
} catch (e) {
error = e;
} finally {
expect(error.message).toContain('NativeModule.node: file too short');
expect(error.message).toMatch(
/NativeModule.node\: file too short|not a valid Win\d+ application/
);
}
});
});
Expand Down
17 changes: 11 additions & 6 deletions src/TestRunner.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ var DEFAULT_OPTIONS = {
};

var HIDDEN_FILE_RE = /\/\.[^\/]*$/;
function optionPathToRegex(p) {
return utils.escapeStrForRegex(p.replace(/\//g, path.sep));
}

/**
* A class that takes a project's test config and provides various utilities for
Expand All @@ -66,6 +69,7 @@ var HIDDEN_FILE_RE = /\/\.[^\/]*$/;
* @param options See DEFAULT_OPTIONS for descriptions on the various options
* and their defaults.
*/

class TestRunner {

constructor(config, options) {
Expand All @@ -74,12 +78,12 @@ class TestRunner {
this._moduleLoaderResourceMap = null;
this._testPathDirsRegExp = new RegExp(
config.testPathDirs
.map(dir => utils.escapeStrForRegex(dir))
.map(dir => optionPathToRegex(dir))
.join('|')
);

this._nodeHasteTestRegExp = new RegExp(
'/' + utils.escapeStrForRegex(config.testDirectoryName) + '/' +
optionPathToRegex(path.sep + config.testDirectoryName + path.sep) +
'.*\\.(' +
config.testFileExtensions
.map(ext => utils.escapeStrForRegex(ext))
Expand Down Expand Up @@ -116,7 +120,8 @@ class TestRunner {
}

_isTestFilePath(filePath) {
filePath = utils.pathNormalize(filePath);
// get filePath into OS-appropriate format before testing patterns
filePath = path.normalize(filePath);
var testPathIgnorePattern =
this._config.testPathIgnorePatterns.length
? new RegExp(this._config.testPathIgnorePatterns.join('|'))
Expand Down Expand Up @@ -351,9 +356,9 @@ class TestRunner {
if (config.collectCoverage && !config.collectCoverageOnlyFrom) {
config.collectCoverageOnlyFrom = {};
moduleLoader.getDependenciesFromPath(testFilePath)
// Skip over built-in and node modules
.filter(path => /^\//.test(path) && !(/node_modules/.test(path)))
.forEach(path => config.collectCoverageOnlyFrom[path] = true);
// Skip over built-in (non-absolute paths) and node modules
.filter(p => path.isAbsolute(p) && !(/node_modules/.test(p)))
.forEach(p => config.collectCoverageOnlyFrom[p] = true);
}

if (config.setupEnvScriptFile) {
Expand Down
13 changes: 0 additions & 13 deletions src/__tests__/TestRunner-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,13 @@ describe('TestRunner', function() {
return expect(isTestFile).toEqual(true);
});

it('supports ../ paths and windows separators', function() {
var path = 'c:\\path\\to\\__tests__\\foo\\bar\\baz\\..\\..\\..\\test.js';
var isTestFile = runner._isTestFilePath(path);

return expect(isTestFile).toEqual(true);
});

it('supports unix separators', function() {
var path = '/path/to/__tests__/test.js';
var isTestFile = runner._isTestFilePath(path);

return expect(isTestFile).toEqual(true);
});

it('supports windows separators', function() {
var path = 'c:\\path\\to\\__tests__\\test.js';
var isTestFile = runner._isTestFilePath(path);

return expect(isTestFile).toEqual(true);
});
});

describe('streamTestPathsRelatedTo', function() {
Expand Down
39 changes: 24 additions & 15 deletions src/lib/__tests__/utils-normalizeConfig-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,22 @@ describe('utils-normalizeConfig', function() {
var expectedPathAbs;
var expectedPathAbsAnother;

// Windows uses backslashes for path separators, which need to be escaped in
// regular expressions. This little helper function helps us generate the
// expected strings for checking path patterns.
function joinForPattern() {
return Array.prototype.join.call(
arguments,utils.escapeStrForRegex(path.sep)
);
}

beforeEach(function() {
path = require('path');
root = path.resolve('/').replace(/[\\\/]/g, '');
expectedPathFooBar = root + '/root/path/foo/bar/baz';
expectedPathFooQux = root + '/root/path/foo/qux/quux';
expectedPathAbs = root + '/an/abs/path';
expectedPathAbsAnother = root + '/another/abs/path';
root = path.resolve('/');
expectedPathFooBar = path.join(root, 'root', 'path', 'foo', 'bar', 'baz');
expectedPathFooQux = path.join(root, 'root', 'path', 'foo', 'qux', 'quux');
expectedPathAbs = path.join(root, 'an', 'abs', 'path');
expectedPathAbsAnother = path.join(root, 'another', 'abs', 'path');
utils = require('../utils');
});

Expand Down Expand Up @@ -234,8 +243,8 @@ describe('utils-normalizeConfig', function() {
}, '/root/path');

expect(config.testPathIgnorePatterns).toEqual([
'bar/baz',
'qux/quux'
joinForPattern('bar','baz'),
joinForPattern('qux','quux')
]);
});

Expand All @@ -251,8 +260,8 @@ describe('utils-normalizeConfig', function() {
});

expect(config.testPathIgnorePatterns).toEqual([
'bar/baz',
'qux/quux/'
joinForPattern('bar','baz'),
joinForPattern('qux','quux','')
]);
});

Expand All @@ -267,7 +276,7 @@ describe('utils-normalizeConfig', function() {

expect(config.testPathIgnorePatterns).toEqual([
'hasNoToken',
'/root/path/foo/hasAToken'
joinForPattern('','root','path','foo','hasAToken')
]);
});
});
Expand All @@ -285,8 +294,8 @@ describe('utils-normalizeConfig', function() {
}, '/root/path');

expect(config.modulePathIgnorePatterns).toEqual([
'bar/baz',
'qux/quux'
joinForPattern('bar','baz'),
joinForPattern('qux','quux')
]);
});

Expand All @@ -302,8 +311,8 @@ describe('utils-normalizeConfig', function() {
});

expect(config.modulePathIgnorePatterns).toEqual([
'bar/baz',
'qux/quux/'
joinForPattern('bar','baz'),
joinForPattern('qux','quux','')
]);
});

Expand All @@ -318,7 +327,7 @@ describe('utils-normalizeConfig', function() {

expect(config.modulePathIgnorePatterns).toEqual([
'hasNoToken',
'/root/path/foo/hasAToken'
joinForPattern('','root','path','foo','hasAToken')
]);
});
});
Expand Down
46 changes: 0 additions & 46 deletions src/lib/__tests__/utils-pathNormalize-test.js

This file was deleted.

37 changes: 21 additions & 16 deletions src/lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,13 @@ var fs = require('graceful-fs');
var path = require('path');
var stringify = require('json-stable-stringify');

function replacePathSepForRegex(str) {
if (path.sep === '\\') {
return str.replace(/(\/|\\)/g,'\\\\');
}
return str;
}

var DEFAULT_CONFIG_VALUES = {
bail: false,
cacheDirectory: path.resolve(__dirname, '..', '..', '.haste_cache'),
Expand All @@ -29,7 +36,7 @@ var DEFAULT_CONFIG_VALUES = {
testEnvData: {},
testFileExtensions: ['js'],
testPathDirs: ['<rootDir>'],
testPathIgnorePatterns: ['/node_modules/'],
testPathIgnorePatterns: [replacePathSepForRegex('/node_modules/')],
testReporter: require.resolve('../IstanbulTestReporter'),
testRunner: require.resolve('../jasmineTestRunner/jasmineTestRunner'),
testURL: 'about:blank',
Expand Down Expand Up @@ -74,10 +81,10 @@ function _replaceRootDirTags(rootDir, config) {
return config;
}

return pathNormalize(path.resolve(
return path.resolve(
rootDir,
'./' + path.normalize(config.substr('<rootDir>'.length))
));
);
}
return config;
}
Expand All @@ -86,6 +93,7 @@ function escapeStrForRegex(str) {
return str.replace(/[-[\]{}()*+?.,\\^$|#\s]/g, '\\$&');
}


/**
* Given the coverage info for a single file (as output by
* CoverageCollector.js), return an array whose entries are bools indicating
Expand Down Expand Up @@ -173,40 +181,40 @@ function normalizeConfig(config) {
throw new Error('No rootDir config value found!');
}

config.rootDir = pathNormalize(config.rootDir);
config.rootDir = path.normalize(config.rootDir);

// Normalize user-supplied config options
Object.keys(config).reduce(function(newConfig, key) {
var value;
switch (key) {
case 'collectCoverageOnlyFrom':
value = Object.keys(config[key]).reduce(function(normObj, filePath) {
filePath = pathNormalize(path.resolve(
filePath = path.resolve(
config.rootDir,
_replaceRootDirTags(config.rootDir, filePath)
));
);
normObj[filePath] = true;
return normObj;
}, {});
break;

case 'testPathDirs':
value = config[key].map(function(scanDir) {
return pathNormalize(path.resolve(
return path.resolve(
config.rootDir,
_replaceRootDirTags(config.rootDir, scanDir)
));
);
});
break;

case 'cacheDirectory':
case 'scriptPreprocessor':
case 'setupEnvScriptFile':
case 'setupTestFrameworkScriptFile':
value = pathNormalize(path.resolve(
value = path.resolve(
config.rootDir,
_replaceRootDirTags(config.rootDir, config[key])
));
);
break;

case 'moduleNameMapper':
Expand All @@ -227,7 +235,9 @@ function normalizeConfig(config) {
// For patterns, direct global substitution is far more ideal, so we
// special case substitutions for patterns here.
value = config[key].map(function(pattern) {
return pattern.replace(/<rootDir>/g, config.rootDir);
return replacePathSepForRegex(
pattern.replace(/<rootDir>/g, config.rootDir)
);
});
break;
case 'bail':
Expand Down Expand Up @@ -316,10 +326,6 @@ function uniqueStrings(set) {
return newSet;
}

function pathNormalize(dir) {
return path.normalize(dir.replace(/\\/g, '/')).replace(/\\/g, '/');
}

function readFile(filePath) {
return new Promise(function(resolve, reject) {
fs.readFile(filePath, 'utf8', function(err, data) {
Expand Down Expand Up @@ -644,7 +650,6 @@ exports.getLinePercentCoverageFromCoverageInfo =
exports.loadConfigFromFile = loadConfigFromFile;
exports.loadConfigFromPackageJson = loadConfigFromPackageJson;
exports.normalizeConfig = normalizeConfig;
exports.pathNormalize = pathNormalize;
exports.readAndPreprocessFileContent = readAndPreprocessFileContent;
exports.runContentWithLocalBindings = runContentWithLocalBindings;
exports.formatFailureMessage = formatFailureMessage;