Skip to content

Windows: Found 0 matching tests #18

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jun 25, 2014
Merged

Windows: Found 0 matching tests #18

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3485d7b
fixed windows file paths normalization
kl3ryk May 15, 2014
42effc7
code style improvements
kl3ryk May 15, 2014
c78ddb9
pathNormalizer moved to utils
kl3ryk May 16, 2014
726a0eb
utils test compability with windows improvements
kl3ryk May 16, 2014
2da47ad
remove unnecessary pathNormalize call
kl3ryk May 26, 2014
f3c2680
utils path fixed
kl3ryk May 26, 2014
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
1 change: 1 addition & 0 deletions src/TestRunner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ TestRunner.prototype._getModuleLoaderResourceMap = function() {
};

TestRunner.prototype._isTestFilePath = function(filePath) {
filePath = utils.pathNormalize(filePath);
Copy link
Contributor

@jeffmo jeffmo May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Contributor Author

@kl3ryk kl3ryk May 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the same - because here: https://github.com/kl3ryk/jest/blob/master/src/TestRunner.js#L232 FileFinder returns not normalized paths

Copy link
Contributor

@jeffmo jeffmo Jun 1, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, you're right. In that case, maybe we should just normalize in the _onMatcherMatch() function before calling the onTestFound callback then.

That puts this normalization closer to the source for anyone else who ever winds up using TestRunner.findTestPathsMatching

var testPathIgnorePattern =
this._config.testPathIgnorePatterns
? new RegExp(this._config.testPathIgnorePatterns.join('|'))
Expand Down
41 changes: 41 additions & 0 deletions src/__tests__/TestRunner-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,47 @@ describe('TestRunner', function() {
TestRunner = require('../TestRunner');
});

describe('_isTestFilePath', function() {
var runner;
var utils;

beforeEach(function() {
utils = require('../lib/utils');
runner = new TestRunner(utils.normalizeConfig({
rootDir: '.',
testPathDirs: []
}));
});

pit('supports ../ paths and unix separators', function() {
var path = '/path/to/__tests__/foo/bar/baz/../../../test.js';
var isTestFile = runner._isTestFilePath(path);

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

pit('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);
});

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

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

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

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

describe('findTestsRelatedTo', function() {
var fakeDepsFromPath;
var fs;
Expand Down
103 changes: 74 additions & 29 deletions src/lib/__tests__/utils-normalizeConfig-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,58 @@

jest.autoMockOff();

describe('utils-pathNormalize', function() {
Copy link
Contributor

@jeffmo jeffmo Jun 1, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you throw this suite into a separate file (say, utils-pathNormalize-test.js) so that it runs in parallel with the other tests?

var utils;

beforeEach(function() {
utils = require('../utils');
});

it('supports ../ paths and unix separators', function() {
var path = '/path/to/__tests__/foo/bar/baz/../../../test.js';
var pathNormalized = utils.pathNormalize(path);

return expect(pathNormalized).toEqual('/path/to/__tests__/test.js');
});

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

return expect(pathNormalized).toEqual('c:/path/to/__tests__/test.js');
});

it('supports unix separators', function() {
var path = '/path/to/__tests__/test.js';
var pathNormalized = utils.pathNormalize(path);

return expect(pathNormalized).toEqual(path);
});

it('supports windows separators', function() {
var path = 'c:\\path\\to\\__tests__\\test.js';
var pathNormalized = utils.pathNormalize(path);

return expect(pathNormalized).toEqual('c:/path/to/__tests__/test.js');
});
});

describe('utils-normalizeConfig', function() {
Copy link
Contributor

@jeffmo jeffmo May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind rebasing + re-running your tests again just for sanity?
I just pushed a fix for a bug where jest was only reporting results for the last top-level describe() suite it encountered: 489565b

Copy link
Contributor Author

@kl3ryk kl3ryk May 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now i have only:

Using Jest CLI v0.1.13
Found 8 matching tests...
 PASS  __tests__\TestRunner-test.js (0.685s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-genMockFromModule-test.js (0.207s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-hasDependency-test.js (0.386s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireMock-test.js (0.994s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireModule-test.js (1.277s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireModuleOrMock-test.js (0.651s)
 PASS  lib\__tests__\FakeTimers-test.js (0.091s)
npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

without any info about tests

Copy link
Contributor Author

@kl3ryk kl3ryk May 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I think there is race condition somewhere:

d:\github.com\jest>npm test

> [email protected] test d:\github.com\jest
> jshint --config=.jshintrc --exclude=src/coverageTemplate.js src && node bin/jest.js

Using Jest CLI v0.1.13
Found 8 matching tests...
 PASS  __tests__\TestRunner-test.js (0.813s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-genMockFromModule-test.js (0.207s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-hasDependency-test.js (0.465s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireMock-test.js (1.301s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireModule-test.js (1.593s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireModuleOrMock-test.js (0.784s)
 PASS  lib\__tests__\FakeTimers-test.js (0.123s)
 PASS  lib\__tests__\utils-normalizeConfig-test.js (0.544s)

d:\github.com\jest>npm test

> [email protected] test d:\github.com\jest
> jshint --config=.jshintrc --exclude=src/coverageTemplate.js src && node bin/jest.js

Using Jest CLI v0.1.13
Found 8 matching tests...
 PASS  __tests__\TestRunner-test.js (0.773s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-genMockFromModule-test.js (0.225s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-hasDependency-test.js (0.576s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireMock-test.js (1.297s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireModule-test.js (1.63s)
 PASS  HasteModuleLoader\__tests__\HasteModuleLoader-requireModuleOrMock-test.js (0.75s)
 PASS  lib\__tests__\FakeTimers-test.js (0.119s)

var path;
var root;
var utils;
var expectedPathFooBar;
var expectedPathFooQux;
var expectedPathAbs;
var expectedPathAbsAnother;

beforeEach(function() {
path = require('path');
Copy link
Contributor

@jeffmo jeffmo May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Man, it's too bad the path core module doesn't itself use path.sep internally for all of its APIs (it doesn't, I just checked).

If it did, we might've been able to just stub out path.sep with our own crazy test delimiter and make assertions that would fail in our tests if ever something the test was testing ever did a non-platform-independent path resolution...

Anyway, I digressed

Copy link
Contributor Author

@kl3ryk kl3ryk May 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO there should be canonical function in path module to handle this. I checked this also - nodejs/node-v0.x-archive#7635, but without any discussion. So as I said before I will prepare alternative path library with this canonical modificator.

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';
utils = require('../utils');
});

Expand Down Expand Up @@ -43,10 +91,11 @@ describe('utils-normalizeConfig', function() {
}
}, '/root/path');

expect(config.collectCoverageOnlyFrom).toEqual({
'/root/path/foo/bar/baz': true,
'/root/path/foo/qux/quux': true
});
var expected = {};
expected[expectedPathFooBar] = true;
expected[expectedPathFooQux] = true;

expect(config.collectCoverageOnlyFrom).toEqual(expected);
});

it('does not change absolute paths', function() {
Expand All @@ -58,10 +107,11 @@ describe('utils-normalizeConfig', function() {
}
});

expect(config.collectCoverageOnlyFrom).toEqual({
'/an/abs/path': true,
'/another/abs/path': true
});
var expected = {};
expected[expectedPathAbs] = true;
expected[expectedPathAbsAnother] = true;

expect(config.collectCoverageOnlyFrom).toEqual(expected);
});

it('substitutes <rootDir> tokens', function() {
Expand All @@ -72,9 +122,10 @@ describe('utils-normalizeConfig', function() {
}
});

expect(config.collectCoverageOnlyFrom).toEqual({
'/root/path/foo/bar/baz': true
});
var expected = {};
expected[expectedPathFooBar] = true;

expect(config.collectCoverageOnlyFrom).toEqual(expected);
});
});

Expand All @@ -89,8 +140,7 @@ describe('utils-normalizeConfig', function() {
}, '/root/path');

expect(config.testPathDirs).toEqual([
'/root/path/foo/bar/baz',
'/root/path/foo/qux/quux'
expectedPathFooBar, expectedPathFooQux
]);
});

Expand All @@ -104,8 +154,7 @@ describe('utils-normalizeConfig', function() {
});

expect(config.testPathDirs).toEqual([
'/an/abs/path',
'/another/abs/path'
expectedPathAbs, expectedPathAbsAnother
]);
});

Expand All @@ -117,7 +166,7 @@ describe('utils-normalizeConfig', function() {
]
});

expect(config.testPathDirs).toEqual(['/root/path/foo/bar/baz']);
expect(config.testPathDirs).toEqual([expectedPathFooBar]);
});
});

Expand All @@ -128,7 +177,7 @@ describe('utils-normalizeConfig', function() {
scriptPreprocessor: 'bar/baz'
}, '/root/path');

expect(config.scriptPreprocessor).toEqual('/root/path/foo/bar/baz');
expect(config.scriptPreprocessor).toEqual(expectedPathFooBar);
});

it('does not change absolute paths', function() {
Expand All @@ -137,7 +186,7 @@ describe('utils-normalizeConfig', function() {
scriptPreprocessor: '/an/abs/path'
});

expect(config.scriptPreprocessor).toEqual('/an/abs/path');
expect(config.scriptPreprocessor).toEqual(expectedPathAbs);
});

it('substitutes <rootDir> tokens', function() {
Expand All @@ -146,7 +195,7 @@ describe('utils-normalizeConfig', function() {
scriptPreprocessor: '<rootDir>/bar/baz'
});

expect(config.scriptPreprocessor).toEqual('/root/path/foo/bar/baz');
expect(config.scriptPreprocessor).toEqual(expectedPathFooBar);
});
});

Expand All @@ -157,7 +206,7 @@ describe('utils-normalizeConfig', function() {
setupEnvScriptFile: 'bar/baz'
}, '/root/path');

expect(config.setupEnvScriptFile).toEqual('/root/path/foo/bar/baz');
expect(config.setupEnvScriptFile).toEqual(expectedPathFooBar);
});

it('does not change absolute paths', function() {
Expand All @@ -166,7 +215,7 @@ describe('utils-normalizeConfig', function() {
setupEnvScriptFile: '/an/abs/path'
});

expect(config.setupEnvScriptFile).toEqual('/an/abs/path');
expect(config.setupEnvScriptFile).toEqual(expectedPathAbs);
});

it('substitutes <rootDir> tokens', function() {
Expand All @@ -175,7 +224,7 @@ describe('utils-normalizeConfig', function() {
setupEnvScriptFile: '<rootDir>/bar/baz'
});

expect(config.setupEnvScriptFile).toEqual('/root/path/foo/bar/baz');
expect(config.setupEnvScriptFile).toEqual(expectedPathFooBar);
});
});

Expand All @@ -186,9 +235,7 @@ describe('utils-normalizeConfig', function() {
setupTestFrameworkScriptFile: 'bar/baz'
}, '/root/path');

expect(config.setupTestFrameworkScriptFile).toEqual(
'/root/path/foo/bar/baz'
);
expect(config.setupTestFrameworkScriptFile).toEqual(expectedPathFooBar);
});

it('does not change absolute paths', function() {
Expand All @@ -197,7 +244,7 @@ describe('utils-normalizeConfig', function() {
setupTestFrameworkScriptFile: '/an/abs/path'
});

expect(config.setupTestFrameworkScriptFile).toEqual('/an/abs/path');
expect(config.setupTestFrameworkScriptFile).toEqual(expectedPathAbs);
});

it('substitutes <rootDir> tokens', function() {
Expand All @@ -206,9 +253,7 @@ describe('utils-normalizeConfig', function() {
setupTestFrameworkScriptFile: '<rootDir>/bar/baz'
});

expect(config.setupTestFrameworkScriptFile).toEqual(
'/root/path/foo/bar/baz'
);
expect(config.setupTestFrameworkScriptFile).toEqual(expectedPathFooBar);
});
});

Expand Down
23 changes: 15 additions & 8 deletions src/lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ function _replaceRootDirTags(rootDir, config) {
return config;
}

return path.resolve(
return pathNormalize(path.resolve(
Copy link
Contributor

@jeffmo jeffmo May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, not all config strings are paths (and thus we shouldn't necessarily normalize them as if they were).
Was there a specific case you were trying to cover with this instead of the places you updated inside normalizeConfig?

Copy link
Contributor Author

@kl3ryk kl3ryk May 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree but there is path.resolve used so it will be path after it. Also that line https://github.com/kl3ryk/jest/blob/master/src/lib/utils.js#L50 says that there is <rootDir> inside that string, so we can say this string is a path.

Copy link
Contributor

@jeffmo jeffmo Jun 1, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah you're right -- this whole function should only be called on path strings anyway.
Nevermind my previous comments

rootDir,
'./' + path.normalize(config.substr('<rootDir>'.length))
);
));
}
return config;
}
Expand Down Expand Up @@ -152,38 +152,40 @@ function normalizeConfig(config) {
throw new Error('No rootDir config value found!');
}

config.rootDir = pathNormalize(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 = path.resolve(
filePath = pathNormalize(path.resolve(
config.rootDir,
_replaceRootDirTags(config.rootDir, filePath)
Copy link
Contributor

@jeffmo jeffmo May 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we only need to pathNormalize() the filePath var here (rather than the whole outer result)?

Copy link
Contributor Author

@kl3ryk kl3ryk May 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because path.resolve also returns not normalized path.
eg.:

path.resolve('/', 'foo');

gives

C:\foo

);
));
normObj[filePath] = true;
return normObj;
}, {});
break;

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

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

case 'testPathIgnorePatterns':
Expand Down Expand Up @@ -231,6 +233,10 @@ function normalizeConfig(config) {
return _replaceRootDirTags(newConfig.rootDir, newConfig);
}

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

function loadConfigFromFile(filePath) {
var fileDir = path.dirname(filePath);
return Q.nfcall(fs.readFile, filePath, 'utf8').then(function(fileData) {
Expand Down Expand Up @@ -422,6 +428,7 @@ exports.getLinePercentCoverageFromCoverageInfo =
exports.loadConfigFromFile = loadConfigFromFile;
exports.loadConfigFromPackageJson = loadConfigFromPackageJson;
exports.normalizeConfig = normalizeConfig;
exports.pathNormalize = pathNormalize;
exports.readAndPreprocessFileContent = readAndPreprocessFileContent;
exports.runContentWithLocalBindings = runContentWithLocalBindings;
exports.serializeConsoleArgValue = serializeConsoleArgValue;
Expand Down