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: Add error handling for nonexistent file case with --file option #5086

Merged
merged 29 commits into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
bd47d79
feat: handle nonexistent files passed to --file
khoaHyh Jan 24, 2024
4aad184
refactor: remove path.resolve()
khoaHyh Jan 24, 2024
8983b84
add comment to new code in collect-files.js
khoaHyh Jan 24, 2024
b6d6dc2
fix: add back absolute path resolving
khoaHyh Jan 24, 2024
9b3d5d2
refactor: log warning and remove call stack
khoaHyh Feb 7, 2024
645e0b0
revert changes to bin/mocha.js
khoaHyh Feb 7, 2024
0e1e84a
improve test case
khoaHyh Feb 7, 2024
9685b82
Merge branch 'master' into issue/4047
khoaHyh Feb 11, 2024
f8aedff
Merge branch 'master' into issue/4047
khoaHyh Feb 21, 2024
a6bdef4
Merge branch 'master' into issue/4047
khoaHyh Mar 24, 2024
909834d
throw error and have handler work with it
khoaHyh Mar 24, 2024
cf69df1
change collectFiles to return object
khoaHyh Mar 25, 2024
b905d21
clean up
khoaHyh Mar 25, 2024
ea092be
exit mocha immediately on missing file
khoaHyh Mar 25, 2024
6d4170e
new log message
khoaHyh Mar 26, 2024
bad240d
add tests
khoaHyh Mar 26, 2024
21e566d
Merge branch 'master' into issue/4047
khoaHyh Mar 26, 2024
dce26f4
Merge branch 'master' into issue/4047
khoaHyh Mar 26, 2024
4c9f318
Merge branch 'master' into issue/4047
khoaHyh Mar 27, 2024
1a9f155
Merge remote-tracking branch 'upstream/master' into issue/4047
khoaHyh May 29, 2024
ff58b19
code quality improvements
khoaHyh Jun 5, 2024
d6f49b9
Merge branch 'master' into issue/4047
khoaHyh Jun 5, 2024
8423875
Merge branch 'master' into issue/4047
khoaHyh Jun 18, 2024
c82c874
add comments
khoaHyh Jun 18, 2024
118fa4c
docs: update to new link name
voxpelli Jun 18, 2024
5c076cd
pass mocha instance to helper function
khoaHyh Jun 18, 2024
3ec3dcd
Merge branch 'master' into issue/4047
khoaHyh Jun 20, 2024
0d6f9c4
Merge branch 'main' into issue/4047
khoaHyh Jun 23, 2024
ffdf2d6
Merge branch 'main' into issue/4047
khoaHyh Jun 25, 2024
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
46 changes: 39 additions & 7 deletions lib/cli/collect-files.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
'use strict';

const fs = require('fs');
const path = require('path');
const ansi = require('ansi-colors');
const debug = require('debug')('mocha:cli:run:helpers');
Expand All @@ -19,7 +20,7 @@ const {castArray} = require('../utils');
/**
* Smash together an array of test files in the correct order
* @param {FileCollectionOptions} [opts] - Options
* @returns {string[]} List of files to test
* @returns {FileCollectionResponse} An object containing a list of files to test and unmatched files.
* @private
*/
module.exports = ({
Expand All @@ -30,7 +31,7 @@ module.exports = ({
sort,
spec
} = {}) => {
const unmatched = [];
const unmatchedSpecFiles = [];
khoaHyh marked this conversation as resolved.
Show resolved Hide resolved
const specFiles = spec.reduce((specFiles, arg) => {
try {
const moreSpecFiles = castArray(lookupFiles(arg, extension, recursive))
Expand All @@ -44,14 +45,34 @@ module.exports = ({
return [...specFiles, ...moreSpecFiles];
} catch (err) {
if (err.code === NO_FILES_MATCH_PATTERN) {
unmatched.push({message: err.message, pattern: err.pattern});
unmatchedSpecFiles.push({message: err.message, pattern: err.pattern});
return specFiles;
}

throw err;
}
}, []);

// check that each file passed in to --file exists

const unmatchedFiles = [];
fileArgs.forEach(file => {
const fileAbsolutePath = path.resolve(file);
try {
require.resolve(fileAbsolutePath);
voxpelli marked this conversation as resolved.
Show resolved Hide resolved
} catch (err) {
if (err.code === 'MODULE_NOT_FOUND') {
unmatchedFiles.push({
pattern: file,
absolutePath: fileAbsolutePath
});
return;
}

throw err;
}
});

// ensure we don't sort the stuff from fileArgs; order is important!
if (sort) {
specFiles.sort();
Expand All @@ -67,19 +88,24 @@ module.exports = ({
if (!files.length) {
// give full message details when only 1 file is missing
const noneFoundMsg =
unmatched.length === 1
? `Error: No test files found: ${JSON.stringify(unmatched[0].pattern)}` // stringify to print escaped characters raw
unmatchedSpecFiles.length === 1
? `Error: No test files found: ${JSON.stringify(
unmatchedSpecFiles[0].pattern
)}` // stringify to print escaped characters raw
: 'Error: No test files found';
console.error(ansi.red(noneFoundMsg));
process.exit(1);
} else {
// print messages as a warning
unmatched.forEach(warning => {
unmatchedSpecFiles.forEach(warning => {
console.warn(ansi.yellow(`Warning: ${warning.message}`));
});
}

return files;
return {
files,
unmatchedFiles
};
};

/**
Expand All @@ -92,4 +118,10 @@ module.exports = ({
* @property {string[]} file - List of additional files to include
* @property {boolean} recursive - Find files recursively
* @property {boolean} sort - Sort test files
* @typedef {Object} UnmatchedFile - Diagnostic object containing unmatched files
voxpelli marked this conversation as resolved.
Show resolved Hide resolved
* @property {string} absolutePath - A list of unmatched files derived from the file arguments passed in.
* @property {string} pattern - A list of unmatched files derived from the file arguments passed in.
* @typedef {Object} FileCollectionResponse
* @property {string[]} files - A list of files to test
* @property {UnmatchedFile[]} unmatchedFiles - A list of unmatched files derived from the file arguments passed in.
*/
47 changes: 41 additions & 6 deletions lib/cli/run-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

const fs = require('fs');
const path = require('path');
const ansi = require('ansi-colors');
const debug = require('debug')('mocha:cli:run:helpers');
const {watchRun, watchParallelRun} = require('./watch-run');
const collectFiles = require('./collect-files');
Expand Down Expand Up @@ -117,9 +118,25 @@ exports.handleRequires = async (requires = [], {ignoredPlugins = []} = {}) => {
* @private
*/
const singleRun = async (mocha, {exit}, fileCollectParams) => {
const files = collectFiles(fileCollectParams);
debug('single run with %d file(s)', files.length);
mocha.files = files;
const fileCollectionObj = collectFiles(fileCollectParams);

if (fileCollectionObj.unmatchedFiles.length > 0) {
fileCollectionObj.unmatchedFiles.forEach(({pattern, absolutePath}) => {
console.error(
ansi.yellow(
`Warning: Cannot find any files matching pattern "${pattern}" at the absolute path "${absolutePath}"`
)
);
});
console.log(
'No test file(s) found with the given pattern, exiting with code 1'
);

return mocha.run(exitMocha(1));
}

debug('single run with %d file(s)', fileCollectionObj.files.length);
mocha.files = fileCollectionObj.files;

// handles ESM modules
await mocha.loadFilesAsync();
Expand All @@ -140,9 +157,27 @@ const singleRun = async (mocha, {exit}, fileCollectParams) => {
* @private
*/
const parallelRun = async (mocha, options, fileCollectParams) => {
const files = collectFiles(fileCollectParams);
debug('executing %d test file(s) in parallel mode', files.length);
mocha.files = files;
const fileCollectionObj = collectFiles(fileCollectParams);

if (fileCollectionObj.unmatchedFiles.length > 0) {
fileCollectionObj.unmatchedFiles.forEach(({pattern, absolutePath}) => {
console.error(
ansi.yellow(
`Warning: Cannot find any files matching pattern "${pattern}" at the absolute path "${absolutePath}"`
)
);
});
console.log(
'No test file(s) found with the given pattern, exiting with code 1'
);
return mocha.run(exitMocha(1));
}
voxpelli marked this conversation as resolved.
Show resolved Hide resolved

debug(
'executing %d test file(s) in parallel mode',
fileCollectionObj.files.length
);
mocha.files = fileCollectionObj.files;

// note that we DO NOT load any files here; this is handled by the worker
return mocha.run(options.exit ? exitMocha : exitMochaLater);
Expand Down
4 changes: 2 additions & 2 deletions lib/cli/watch-run.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ exports.watchParallelRun = (
newMocha.suite.ctx = new Context();

// reset the list of files
newMocha.files = collectFiles(fileCollectParams);
newMocha.files = collectFiles(fileCollectParams).files;

// because we've swapped out the root suite (see the `run` inner function
// in `createRerunner`), we need to call `mocha.ui()` again to set up the context/globals.
Expand Down Expand Up @@ -120,7 +120,7 @@ exports.watchRun = (mocha, {watchFiles, watchIgnore}, fileCollectParams) => {
newMocha.suite.ctx = new Context();

// reset the list of files
newMocha.files = collectFiles(fileCollectParams);
newMocha.files = collectFiles(fileCollectParams).files;

// because we've swapped out the root suite (see the `run` inner function
// in `createRerunner`), we need to call `mocha.ui()` again to set up the context/globals.
Expand Down
7 changes: 7 additions & 0 deletions test/integration/fixtures/collect-files.fixture.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
var obj = {foo: 'bar'};

describe('mjs', function () {
it('should work', function () {
expect(obj, 'to equal', {foo: 'bar'});
});
});
86 changes: 83 additions & 3 deletions test/integration/options/file.spec.js
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
'use strict';

var path = require('path').posix;
var helpers = require('../helpers');
var runMochaJSON = helpers.runMochaJSON;
var resolvePath = helpers.resolveFixturePath;
const {
runMochaJSON,
resolveFixturePath: resolvePath,
runMocha
} = require('../helpers');

describe('--file', function () {
var args = [];
Expand Down Expand Up @@ -64,4 +66,82 @@ describe('--file', function () {
done();
});
});

it('should run esm tests passed via file', function (done) {
const esmFile = 'collect-files.fixture.mjs';
const testArgs = ['--file', resolvePath(esmFile)];

runMochaJSON(esmFile, testArgs, function (err, res) {
if (err) {
return done(err);
}
expect(res, 'to have passed');
done();
});
});

it('should log a warning if a nonexistent file is specified', function (done) {
voxpelli marked this conversation as resolved.
Show resolved Hide resolved
const nonexistentTestFileArg = 'nonexistent.test.ts';
runMocha(
nonexistentTestFileArg,
['--file'],
function (err, res) {
if (err) {
return done(err);
}

expect(
res.output,
'to contain',
`Warning: Cannot find any files matching pattern`
).and('to contain', nonexistentTestFileArg);
done();
},
{stdio: 'pipe'}
);
});

it('should provide warning for nonexistent cjs file extensions', function (done) {
const nonexistentCjsArg = 'nonexistent.test.js';
voxpelli marked this conversation as resolved.
Show resolved Hide resolved

runMocha(
nonexistentCjsArg,
['--file'],
function (err, res) {
if (err) {
return done(err);
}

expect(
res.output,
'to contain',
`Warning: Cannot find any files matching pattern`
).and('to contain', nonexistentCjsArg);
done();
},
{stdio: 'pipe'}
);
});

it('should provide warning for nonexistent esm file extensions', function (done) {
const nonexistentEsmArg = 'nonexistent.test.mjs';

runMocha(
nonexistentEsmArg,
['--file'],
function (err, res) {
if (err) {
return done(err);
}

expect(
res.output,
'to contain',
`Warning: Cannot find any files matching pattern`
).and('to contain', nonexistentEsmArg);
done();
},
{stdio: 'pipe'}
);
});
});
Loading