From 145344c9a94c2ef4b650d3dde4057a93c9d8d088 Mon Sep 17 00:00:00 2001 From: Aras Abbasi Date: Fri, 3 Oct 2025 09:10:07 +0200 Subject: [PATCH] `lookupFiles` repeats various operations over and over. We can avoid alot of this overhead and improve the performance of big test suites. To improve the performance, we normalize the file extensions once at the beginning and dont need to do it again. This means that we can avoid again normalizing the array in `hasMatchingFileExtension` (before: `hasMatchingExtname`). Also we avoid the performance bottleneck of Array.helper functions like `.map` and `.some()`. `fs.statSync` is called with `throwIfNoEntry`, which exists since node 14.17.0, set to `false`. We can avoid "expensive" errors and the try catch blocks. We just check if stat is undefined, and if so, well, the path has an issue. Using `recursive` and `withFileTypes` option of readdirsync, we can avoid using recursive calling of `lookupFiles`. Added `jsdoc` for better type resolution. Also if you would put `// @ts-check` at the beginning of the file, it would pass. --- lib/cli/lookup-files.js | 160 ++++++++++++++++------------ test/integration/file-utils.spec.js | 27 ++++- 2 files changed, 120 insertions(+), 67 deletions(-) diff --git a/lib/cli/lookup-files.js b/lib/cli/lookup-files.js index f5ad0ca844..84f2a865d5 100644 --- a/lib/cli/lookup-files.js +++ b/lib/cli/lookup-files.js @@ -1,18 +1,22 @@ 'use strict'; + /** * Contains `lookupFiles`, which takes some globs/dirs/options and returns a list of files. * @module * @private */ -var fs = require('node:fs'); -var path = require('node:path'); -var glob = require('glob'); -var errors = require('../errors'); -var createNoFilesMatchPatternError = errors.createNoFilesMatchPatternError; -var createMissingArgumentError = errors.createMissingArgumentError; +const fs = require('node:fs'); +const path = require('node:path'); +const glob = require('glob'); +const { + createNoFilesMatchPatternError, + createMissingArgumentError +} = require('../errors'); const debug = require('debug')('mocha:cli:lookup-files'); +/** @typedef {`.${string}`} HiddenUnixPathname */ + /** * Determines if pathname would be a "hidden" file (or directory) on UN*X. * @@ -24,30 +28,54 @@ const debug = require('debug')('mocha:cli:lookup-files'); * * @private * @param {string} pathname - Pathname to check for match. - * @return {boolean} whether pathname would be considered a hidden file. + * @returns {value is HiddenUnixPathname} whether pathname would be considered a hidden file. * @example * isHiddenOnUnix('.profile'); // => true */ -const isHiddenOnUnix = pathname => path.basename(pathname).startsWith('.'); +const isHiddenOnUnix = pathname => path.basename(pathname)[0] === '.'; + +/** @typedef {`.${string}`} FileExtension */ /** - * Determines if pathname has a matching file extension. + * Normalize file extensions to ensure they have a leading period character. * - * Supports multi-part extensions. + * @private + * @param {FileExtension[]|string[]|undefined|null} exts + * @returns {FileExtension[]} + */ +const normalizeFileExtensions = (exts) => { + if (!exts) { + return []; + } + + for (var i = 0; i < exts.length; i++) { + if (exts[i][0] !== '.') { + exts[i] = `.${exts[i]}`; + } + } + return /** @type {FileExtension[]} */ (exts); +} + +/** + * Determines if pathname has a matching file extension. * * @private * @param {string} pathname - Pathname to check for match. - * @param {string[]} exts - List of file extensions, w/-or-w/o leading period - * @return {boolean} `true` if file extension matches. + * @param {FileExtension[]} fileExtensions - List of file extensions, w/-or-w/o leading period + * @returns {boolean} `true` if file extension matches. * @example - * hasMatchingExtname('foo.html', ['js', 'css']); // false - * hasMatchingExtname('foo.js', ['.js']); // true - * hasMatchingExtname('foo.js', ['js']); // ture + * hasMatchingFileExtension('foo.html', ['js', 'css']); // false + * hasMatchingFileExtension('foo.js', ['.js']); // true + * hasMatchingFileExtension('foo.js', ['js']); // ture */ -const hasMatchingExtname = (pathname, exts = []) => - exts - .map(ext => (ext.startsWith('.') ? ext : `.${ext}`)) - .some(ext => pathname.endsWith(ext)); +const hasMatchingFileExtension = (pathname, fileExtensions) => { + for (var i = 0; i < fileExtensions.length; i++) { + if (pathname.endsWith(fileExtensions[i])) { + return true; + } + } + return false; +} /** * Lookup file names at the given `path`. @@ -59,31 +87,30 @@ const hasMatchingExtname = (pathname, exts = []) => * @public * @alias module:lib/cli.lookupFiles * @param {string} filepath - Base path to start searching from. - * @param {string[]} [extensions=[]] - File extensions to look for. + * @param {string[]} [fileExtensions=[]] - File extensions to look for. * @param {boolean} [recursive=false] - Whether to recurse into subdirectories. - * @return {string[]} An array of paths. + * @returns {string[]} An array of paths. * @throws {Error} if no files match pattern. - * @throws {TypeError} if `filepath` is directory and `extensions` not provided. + * @throws {TypeError} if `filepath` is directory and `fileExtensions` not + * provided or an empty array. */ module.exports = function lookupFiles( filepath, - extensions = [], + fileExtensions, recursive = false ) { const files = []; - let stat; + fileExtensions = normalizeFileExtensions(fileExtensions); + // Detect glob patterns by checking if the path does not exist as-is if (!fs.existsSync(filepath)) { let pattern; - if (glob.hasMagic(filepath, {windowsPathsNoEscape: true})) { + if (glob.hasMagic(filepath, { windowsPathsNoEscape: true })) { // Handle glob as is without extensions pattern = filepath; } else { // glob pattern e.g. 'filepath+(.js|.ts)' - const strExtensions = extensions - .map(ext => (ext.startsWith('.') ? ext : `.${ext}`)) - .join('|'); - pattern = `${filepath}+(${strExtensions})`; + pattern = `${filepath}+(${fileExtensions.join('|')})`; debug('looking for files using glob pattern: %s', pattern); } files.push( @@ -106,50 +133,51 @@ module.exports = function lookupFiles( return files; } - // Handle file - try { - stat = fs.statSync(filepath); - if (stat.isFile()) { - return filepath; - } - } catch (err) { - // ignore error - return; - } - - // Handle directory - fs.readdirSync(filepath).forEach(dirent => { - const pathname = path.join(filepath, dirent); - let stat; + const stat = fs.statSync(filepath, { + throwIfNoEntry: false + }); - try { - stat = fs.statSync(pathname); - if (stat.isDirectory()) { - if (recursive) { - files.push(...lookupFiles(pathname, extensions, recursive)); - } - return; - } - } catch (ignored) { - return; - } - if (!extensions.length) { + if (stat === undefined) { + // Unreachable because glob check already checks if path exists, but for + // completeness... + } else if (stat.isFile()) { + files.push(filepath); + } else if (stat.isDirectory()) { + if (fileExtensions.length === 0) { throw createMissingArgumentError( - `Argument '${extensions}' required when argument '${filepath}' is a directory`, + `Argument '${fileExtensions}' required when argument '${filepath}' is a directory`, 'extensions', 'array' - ); + ) } - if ( - !stat.isFile() || - !hasMatchingExtname(pathname, extensions) || - isHiddenOnUnix(pathname) - ) { - return; + // Handle directory + const dirEnts = fs.readdirSync(filepath, { recursive, withFileTypes: true }); + + for (var i = 0; i < dirEnts.length; i++) { + const dirEnt = dirEnts[i]; + + const pathname = dirEnt.parentPath + ? path.join(dirEnt.parentPath, dirEnt.name) + : path.join(filepath, dirEnt.name); + + if (dirEnt.isFile() || dirEnt.isSymbolicLink()) { + if (dirEnt.isSymbolicLink()) { + const stat = fs.statSync(pathname, { throwIfNoEntry: false }); + if (!stat || !stat.isFile()) { + continue; // Skip broken symlinks or symlinks to directories + } + } + + if ( + hasMatchingFileExtension(pathname, /** @type {FileExtension[]} */(fileExtensions)) && + !isHiddenOnUnix(pathname) + ) { + files.push(pathname); + } + } } - files.push(pathname); - }); + } return files; }; diff --git a/test/integration/file-utils.spec.js b/test/integration/file-utils.spec.js index 67ca1be424..f353261e60 100644 --- a/test/integration/file-utils.spec.js +++ b/test/integration/file-utils.spec.js @@ -17,7 +17,7 @@ describe('file utils', function () { tmpDir = result.dirpath; removeTempDir = result.removeTempDir; - tmpFile = filepath => path.join(tmpDir, filepath); + tmpFile = (...args) => path.join(...[tmpDir, ...args]); touchFile(tmpFile('mocha-utils.js')); if (SYMLINK_SUPPORT) { @@ -64,6 +64,22 @@ describe('file utils', function () { ex.and('to have length', expectedLength); }); + describe('when containing subdirectories', function () { + beforeEach(function () { + touchFile(tmpFile('types','mocha-utils.d.ts')); + }); + + it('should find files in subdirectories', function () { + expect( + lookupFiles(tmpDir, ['.d.ts'], true).map(foundFilepath => + path.normalize(foundFilepath) + ), + 'to contain', + tmpFile('types', 'mocha-utils.d.ts') + ).and('to have length', 1); + }); + }) + describe('when given `extension` option', function () { describe('when provided a directory for the filepath', function () { let filepath; @@ -253,6 +269,15 @@ describe('file utils', function () { }); }); + describe('when provided path to non existing file', function () { + it('should throw an exception', function () { + expect(() => lookupFiles(tmpFile('mocha-utils-faux')), 'to throw', { + name: 'Error', + code: 'ERR_MOCHA_NO_FILES_MATCH_PATTERN' + }); + }); + }); + describe('when no files match', function () { it('should throw an exception', function () { expect(() => lookupFiles(tmpFile('mocha-utils')), 'to throw', {