Skip to content

Commit a8ef5cf

Browse files
shadowspawnshadowspawn
authored
Add informative message for missing executable on Windows (#2291)
1 parent 02c603e commit a8ef5cf

3 files changed

Lines changed: 83 additions & 40 deletions

File tree

‎lib/command.js‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1103,6 +1103,26 @@ Expecting one of '${allowedValues.join("', '")}'`);
11031103
return this;
11041104
}
11051105

1106+
/**
1107+
* Throw if expected executable is missing. Add lots of help for author.
1108+
*
1109+
* @param {string} executableFile
1110+
* @param {string} executableDir
1111+
* @param {string} subcommandName
1112+
*/
1113+
_checkForMissingExecutable(executableFile, executableDir, subcommandName) {
1114+
if (fs.existsSync(executableFile)) return;
1115+
1116+
const executableDirMessage = executableDir
1117+
? `searched for local subcommand relative to directory '${executableDir}'`
1118+
: 'no directory for search for local subcommand, use .executableDir() to supply a custom directory';
1119+
const executableMissing = `'${executableFile}' does not exist
1120+
- if '${subcommandName}' is not meant to be an executable command, remove description parameter from '.command()' and use '.description()' instead
1121+
- if the default executable name is not suitable, use the executableFile option to supply a custom name or path
1122+
- ${executableDirMessage}`;
1123+
throw new Error(executableMissing);
1124+
}
1125+
11061126
/**
11071127
* Execute a sub-command executable.
11081128
*
@@ -1186,6 +1206,11 @@ Expecting one of '${allowedValues.join("', '")}'`);
11861206
proc = childProcess.spawn(executableFile, args, { stdio: 'inherit' });
11871207
}
11881208
} else {
1209+
this._checkForMissingExecutable(
1210+
executableFile,
1211+
executableDir,
1212+
subcommand._name,
1213+
);
11891214
args.unshift(executableFile);
11901215
// add executable arguments to spawn
11911216
args = incrementNodeInspectorPort(process.execArgv).concat(args);
@@ -1224,14 +1249,11 @@ Expecting one of '${allowedValues.join("', '")}'`);
12241249
proc.on('error', (err) => {
12251250
// @ts-ignore: because err.code is an unknown property
12261251
if (err.code === 'ENOENT') {
1227-
const executableDirMessage = executableDir
1228-
? `searched for local subcommand relative to directory '${executableDir}'`
1229-
: 'no directory for search for local subcommand, use .executableDir() to supply a custom directory';
1230-
const executableMissing = `'${executableFile}' does not exist
1231-
- if '${subcommand._name}' is not meant to be an executable command, remove description parameter from '.command()' and use '.description()' instead
1232-
- if the default executable name is not suitable, use the executableFile option to supply a custom name or path
1233-
- ${executableDirMessage}`;
1234-
throw new Error(executableMissing);
1252+
this._checkForMissingExecutable(
1253+
executableFile,
1254+
executableDir,
1255+
subcommand._name,
1256+
);
12351257
// @ts-ignore: because err.code is an unknown property
12361258
} else if (err.code === 'EACCES') {
12371259
throw new Error(`'${executableFile}' not executable`);

‎tests/command.executableSubcommand.mock.test.js‎

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,47 +12,62 @@ function makeSystemError(code) {
1212
return err;
1313
}
1414

15-
test('when subcommand executable missing (ENOENT) then throw custom message', () => {
16-
// If the command is not found, we show a custom error with an explanation and offer
17-
// some advice for possible fixes.
18-
const mockProcess = new EventEmitter();
19-
const spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => {
20-
return mockProcess;
21-
});
22-
const program = new commander.Command();
23-
program.exitOverride();
24-
program.command('executable', 'executable description');
25-
program.parse(['executable'], { from: 'user' });
26-
expect(() => {
27-
mockProcess.emit('error', makeSystemError('ENOENT'));
28-
}).toThrow('use the executableFile option to supply a custom name'); // part of custom message
29-
spawnSpy.mockRestore();
30-
});
15+
// Suppress false positive warnings due to use of testOrSkipOnWindows
16+
/* eslint-disable jest/no-standalone-expect */
3117

32-
test('when subcommand executable not executable (EACCES) then throw custom message', () => {
33-
// Side note: this error does not actually happen on Windows! But we can still simulate the behaviour on other platforms.
34-
const mockProcess = new EventEmitter();
35-
const spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => {
36-
return mockProcess;
37-
});
38-
const program = new commander.Command();
39-
program.exitOverride();
40-
program.command('executable', 'executable description');
41-
program.parse(['executable'], { from: 'user' });
42-
expect(() => {
43-
mockProcess.emit('error', makeSystemError('EACCES'));
44-
}).toThrow('not executable'); // part of custom message
45-
spawnSpy.mockRestore();
46-
});
18+
const testOrSkipOnWindows = process.platform === 'win32' ? test.skip : test;
19+
20+
testOrSkipOnWindows(
21+
'when subcommand executable missing (ENOENT) then throw custom message',
22+
() => {
23+
// If the command is not found, we show a custom error with an explanation and offer
24+
// some advice for possible fixes.
25+
const mockProcess = new EventEmitter();
26+
const spawnSpy = jest
27+
.spyOn(childProcess, 'spawn')
28+
.mockImplementation(() => {
29+
return mockProcess;
30+
});
31+
const program = new commander.Command();
32+
program.exitOverride();
33+
program.command('executable', 'executable description');
34+
program.parse(['executable'], { from: 'user' });
35+
expect(() => {
36+
mockProcess.emit('error', makeSystemError('ENOENT'));
37+
}).toThrow('use the executableFile option to supply a custom name'); // part of custom message
38+
spawnSpy.mockRestore();
39+
},
40+
);
41+
42+
testOrSkipOnWindows(
43+
'when subcommand executable not executable (EACCES) then throw custom message',
44+
() => {
45+
const mockProcess = new EventEmitter();
46+
const spawnSpy = jest
47+
.spyOn(childProcess, 'spawn')
48+
.mockImplementation(() => {
49+
return mockProcess;
50+
});
51+
const program = new commander.Command();
52+
program.exitOverride();
53+
program.command('executable', 'executable description');
54+
program.parse(['executable'], { from: 'user' });
55+
expect(() => {
56+
mockProcess.emit('error', makeSystemError('EACCES'));
57+
}).toThrow('not executable'); // part of custom message
58+
spawnSpy.mockRestore();
59+
},
60+
);
4761

48-
test('when subcommand executable fails with other error and exitOverride then return in custom wrapper', () => {
62+
test('when subcommand executable fails with other error and exitOverride then return in custom wrapper', () => {
4963
// The existing behaviour is to just silently fail for unexpected errors, as it is happening
5064
// asynchronously in spawned process and client can not catch errors.
5165
const mockProcess = new EventEmitter();
5266
const spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => {
5367
return mockProcess;
5468
});
5569
const program = new commander.Command();
70+
program._checkForMissingExecutable = () => {}; // suppress error, call mocked spawn
5671
program.exitOverride((err) => {
5772
throw err;
5873
});
@@ -78,6 +93,7 @@ test('when subcommand executable fails with other error then exit', () => {
7893
});
7994
const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => {});
8095
const program = new commander.Command();
96+
program._checkForMissingExecutable = () => {}; // suppress error, call mocked spawn
8197
program.command('executable', 'executable description');
8298
program.parse(['executable'], { from: 'user' });
8399
mockProcess.emit('error', makeSystemError('OTHER'));

‎tests/command.executableSubcommand.search.test.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,15 @@ describe('search for subcommand', () => {
5050
spawnSpy.mockRestore();
5151
});
5252

53+
// fs.existsSync gets called on Windows outside the search, so skip the tests (or come up with a different way of checking).
5354
describe('whether perform search for local files', () => {
5455
beforeEach(() => {
5556
existsSpy.mockImplementation(() => false);
5657
});
5758

5859
test('when no script arg or executableDir then no search for local file', () => {
5960
const program = new commander.Command();
61+
program._checkForMissingExecutable = () => {}; // suppress error, call mocked spawn
6062
program.name('pm');
6163
program.command('sub', 'executable description');
6264
program.parse(['sub'], { from: 'user' });
@@ -66,6 +68,7 @@ describe('search for subcommand', () => {
6668

6769
test('when script arg then search for local files', () => {
6870
const program = new commander.Command();
71+
program._checkForMissingExecutable = () => {}; // suppress error, call mocked spawn
6972
program.name('pm');
7073
program.command('sub', 'executable description');
7174
program.parse(['node', 'script-name', 'sub']);
@@ -75,6 +78,7 @@ describe('search for subcommand', () => {
7578

7679
test('when executableDir then search for local files)', () => {
7780
const program = new commander.Command();
81+
program._checkForMissingExecutable = () => {}; // suppress error, call mocked spawn
7882
program.name('pm');
7983
program.executableDir(__dirname);
8084
program.command('sub', 'executable description');
@@ -301,6 +305,7 @@ describe('search for subcommand', () => {
301305
test('when script arg then search for local script-sub.js, .ts, .tsx, .mpjs, .cjs', () => {
302306
existsSpy.mockImplementation((path) => false);
303307
const program = new commander.Command();
308+
program._checkForMissingExecutable = () => {}; // suppress error, call mocked spawn
304309
program.command('sub', 'executable description');
305310
const scriptPath = path.resolve(gLocalDirectory, 'script');
306311
program.parse(['node', scriptPath, 'sub']);

0 commit comments

Comments
 (0)