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.

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

Jump to bottom

Fix regression/bug in "lookupFiles()" #3905

Merged
merged 3 commits into from May 17, 2019
Merged

Fix regression/bug in "lookupFiles()" #3905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc5068e
lookupFiles(): fix regression and glob
juergba May 9, 2019
10cfd40
more glob
juergba May 9, 2019
0a7b830
tests
juergba May 15, 2019
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
37 changes: 21 additions & 16 deletions lib/utils.js
View file
Open in desktop
Expand Up @@ -563,33 +563,38 @@ function isHiddenOnUnix(pathname) {
* @public
* @memberof Mocha.utils
* @param {string} filepath - Base path to start searching from.
* @param {string[]} extensions - File extensions to look for.
* @param {boolean} recursive - Whether to recurse into subdirectories.
* @param {string[]} [extensions=[]] - File extensions to look for.
* @param {boolean} [recursive=false] - Whether to recurse into subdirectories.
* @return {string[]} An array of paths.
* @throws {Error} if no files match pattern.
* @throws {TypeError} if `filepath` is directory and `extensions` not provided.
*/
exports.lookupFiles = function lookupFiles(filepath, extensions, recursive) {
extensions = extensions || [];
recursive = recursive || false;
Copy link
Contributor

@plroebuck plroebuck May 12, 2019 •
edited

Choose a reason for hiding this comment

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

Don't change the input argument values unless they are not provided (i.e., undefined). The provided code did this correctly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Any falsy input (0, "", null, undefined) or no argument provided (=undefined) will be changed to its default value. What should be wrong about changing those falsy values?

var files = [];
var stat;

if (!fs.existsSync(filepath)) {
// check all extensions
extensions.forEach(function(ext) {
if (fs.existsSync(filepath + '.' + ext)) {
files.push(filepath + '.' + ext);
}
});
var pattern;
if (glob.hasMagic(filepath)) {
// Handle glob as is without extensions
pattern = filepath;
} else {
// glob pattern e.g. 'filepath+(.js|.ts)'
var strExtensions = extensions
.map(function(v) {
return '.' + v;
})
.join('|');
pattern = filepath + '+(' + strExtensions + ')';
}
files = glob.sync(pattern, {nodir: true});
if (!files.length) {
// Handle glob
files = glob.sync(filepath);
if (!files.length) {
throw createNoFilesMatchPatternError(
'Cannot find any files matching pattern ' + exports.dQuote(filepath),
filepath
);
}
throw createNoFilesMatchPatternError(
'Cannot find any files matching pattern ' + exports.dQuote(filepath),
filepath
);
}
return files;
}
Expand Down
28 changes: 28 additions & 0 deletions test/integration/file-utils.spec.js
View file
Open in desktop
Expand Up @@ -66,6 +66,34 @@ describe('file utils', function() {
expect(res, 'to contain', nonJsFile).and('to have length', 1);
});

it('should return only the ".js" file', function() {
var TsFile = tmpFile('mocha-utils.ts');
fs.writeFileSync(TsFile, 'yippy skippy ying yang yow');

var res = utils
.lookupFiles(tmpFile('mocha-utils'), ['js'], false)
.map(path.normalize.bind(path));
expect(res, 'to contain', tmpFile('mocha-utils.js')).and(
'to have length',
1
);
});

it('should return ".js" and ".ts" files', function() {
var TsFile = tmpFile('mocha-utils.ts');
fs.writeFileSync(TsFile, 'yippy skippy ying yang yow');

var res = utils
.lookupFiles(tmpFile('mocha-utils'), ['js', 'ts'], false)
.map(path.normalize.bind(path));
expect(
res,
'to contain',
tmpFile('mocha-utils.js'),
tmpFile('mocha-utils.ts')
).and('to have length', 2);
});

it('should require the extensions parameter when looking up a file', function() {
var dirLookup = function() {
return utils.lookupFiles(tmpFile('mocha-utils'), undefined, false);
Expand Down