diff --git a/lib/command.js b/lib/command.js index 155f33c2d..2020e6c33 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1094,6 +1094,24 @@ Expecting one of '${allowedValues.join("', '")}'`); return this; } + /** + * Note: masked in some unit tests. + * @param {string} executableFile + * @param {string} executableDir + * @param {string} subcommandName + */ + _throwForMissingExecutable(executableFile, executableDir, subcommandName) { + if (fs.existsSync(executableFile)) return; + const executableDirMessage = executableDir + ? `searched for local subcommand relative to directory '${executableDir}'` + : 'no directory for search for local subcommand, use .executableDir() to supply a custom directory'; + const executableMissing = `'${executableFile}' does not exist + - if '${subcommandName}' is not meant to be an executable command, remove description parameter from '.command()' and use '.description()' instead + - if the default executable name is not suitable, use the executableFile option to supply a custom name or path + - ${executableDirMessage}`; + throw new Error(executableMissing); + } + /** * Execute a sub-command executable. * @@ -1165,21 +1183,9 @@ Expecting one of '${allowedValues.join("', '")}'`); launchWithNode = sourceExt.includes(path.extname(executableFile)); - function throwMissingExecutable() { - const executableDirMessage = executableDir - ? `searched for local subcommand relative to directory '${executableDir}'` - : 'no directory for search for local subcommand, use .executableDir() to supply a custom directory'; - const executableMissing = `'${executableFile}' does not exist - - if '${subcommand._name}' is not meant to be an executable command, remove description parameter from '.command()' and use '.description()' instead - - if the default executable name is not suitable, use the executableFile option to supply a custom name or path - - ${executableDirMessage}`; - throw new Error(executableMissing); - } - let proc; if (process.platform !== 'win32') { if (launchWithNode) { - if (!fs.existsSync(executableFile)) throwMissingExecutable(); args.unshift(executableFile); // add executable arguments to spawn args = incrementNodeInspectorPort(process.execArgv).concat(args); @@ -1189,7 +1195,11 @@ Expecting one of '${allowedValues.join("', '")}'`); proc = childProcess.spawn(executableFile, args, { stdio: 'inherit' }); } } else { - if (!fs.existsSync(executableFile)) throwMissingExecutable(); + this._throwForMissingExecutable( + executableFile, + executableDir, + subcommand._name, + ); args.unshift(executableFile); // add executable arguments to spawn args = incrementNodeInspectorPort(process.execArgv).concat(args); @@ -1228,7 +1238,11 @@ Expecting one of '${allowedValues.join("', '")}'`); proc.on('error', (err) => { // @ts-ignore: because err.code is an unknown property if (err.code === 'ENOENT') { - throwMissingExecutable(); + this._throwForMissingExecutable( + executableFile, + executableDir, + subcommand._name, + ); // @ts-ignore: because err.code is an unknown property } else if (err.code === 'EACCES') { throw new Error(`'${executableFile}' not executable`); diff --git a/tests/command.executableSubcommand.mock.test.js b/tests/command.executableSubcommand.mock.test.js index 2510ad4b0..1297040e6 100644 --- a/tests/command.executableSubcommand.mock.test.js +++ b/tests/command.executableSubcommand.mock.test.js @@ -12,12 +12,14 @@ function makeSystemError(code) { return err; } -// These tests are either not relevant to Windows, or don't failure in same way because fail early for missing executable. -const describeOrSkipOnWindows = - process.platform === 'win32' ? describe.skip : describe; +// Suppress false positive warnings due to use of testOrSkipOnWindows +/* eslint-disable jest/no-standalone-expect */ -describeOrSkipOnWindows('executable Subcommand failure modes', () => { - test('when subcommand executable missing (ENOENT) then throw custom message', () => { +const testOrSkipOnWindows = process.platform === 'win32' ? test.skip : test; + +testOrSkipOnWindows( + 'when subcommand executable missing (ENOENT) then throw custom message', + () => { // If the command is not found, we show a custom error with an explanation and offer // some advice for possible fixes. const mockProcess = new EventEmitter(); @@ -34,9 +36,12 @@ describeOrSkipOnWindows('executable Subcommand failure modes', () => { mockProcess.emit('error', makeSystemError('ENOENT')); }).toThrow('use the executableFile option to supply a custom name'); // part of custom message spawnSpy.mockRestore(); - }); + }, +); - test('when subcommand executable not executable (EACCES) then throw custom message', () => { +testOrSkipOnWindows( + 'when subcommand executable not executable (EACCES) then throw custom message', + () => { // Side note: this error does not actually happen on Windows! But we can still simulate the behaviour on other platforms. const mockProcess = new EventEmitter(); const spawnSpy = jest @@ -52,50 +57,48 @@ describeOrSkipOnWindows('executable Subcommand failure modes', () => { mockProcess.emit('error', makeSystemError('EACCES')); }).toThrow('not executable'); // part of custom message spawnSpy.mockRestore(); - }); + }, +); - test('when subcommand executable fails with other error and exitOverride then return in custom wrapper', () => { - // The existing behaviour is to just silently fail for unexpected errors, as it is happening - // asynchronously in spawned process and client can not catch errors. - const mockProcess = new EventEmitter(); - const spawnSpy = jest - .spyOn(childProcess, 'spawn') - .mockImplementation(() => { - return mockProcess; - }); - const program = new commander.Command(); - program.exitOverride((err) => { - throw err; - }); - program.command('executable', 'executable description'); - program.parse(['executable'], { from: 'user' }); - let caughtErr; - try { - mockProcess.emit('error', makeSystemError('OTHER')); - } catch (err) { - caughtErr = err; - } - expect(caughtErr.code).toEqual('commander.executeSubCommandAsync'); - expect(caughtErr.nestedError.code).toEqual('OTHER'); - spawnSpy.mockRestore(); +test('when subcommand executable fails with other error and exitOverride then return in custom wrapper', () => { + // The existing behaviour is to just silently fail for unexpected errors, as it is happening + // asynchronously in spawned process and client can not catch errors. + const mockProcess = new EventEmitter(); + const spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => { + return mockProcess; }); - - test('when subcommand executable fails with other error then exit', () => { - // The existing behaviour is to just silently fail for unexpected errors, as it is happening - // asynchronously in spawned process and client can not catch errors. - const mockProcess = new EventEmitter(); - const spawnSpy = jest - .spyOn(childProcess, 'spawn') - .mockImplementation(() => { - return mockProcess; - }); - const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => {}); - const program = new commander.Command(); - program.command('executable', 'executable description'); - program.parse(['executable'], { from: 'user' }); + const program = new commander.Command(); + program._throwForMissingExecutable = () => {}; // suppress error, call mocked spawn + program.exitOverride((err) => { + throw err; + }); + program.command('executable', 'executable description'); + program.parse(['executable'], { from: 'user' }); + let caughtErr; + try { mockProcess.emit('error', makeSystemError('OTHER')); - expect(exitSpy).toHaveBeenCalledWith(1); - exitSpy.mockRestore(); - spawnSpy.mockRestore(); + } catch (err) { + caughtErr = err; + } + expect(caughtErr.code).toEqual('commander.executeSubCommandAsync'); + expect(caughtErr.nestedError.code).toEqual('OTHER'); + spawnSpy.mockRestore(); +}); + +test('when subcommand executable fails with other error then exit', () => { + // The existing behaviour is to just silently fail for unexpected errors, as it is happening + // asynchronously in spawned process and client can not catch errors. + const mockProcess = new EventEmitter(); + const spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => { + return mockProcess; }); + const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => {}); + const program = new commander.Command(); + program._throwForMissingExecutable = () => {}; // suppress error, call mocked spawn + program.command('executable', 'executable description'); + program.parse(['executable'], { from: 'user' }); + mockProcess.emit('error', makeSystemError('OTHER')); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockRestore(); + spawnSpy.mockRestore(); }); diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index 40f25edc1..3c2c5eaab 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -51,13 +51,14 @@ describe('search for subcommand', () => { }); // fs.existsSync gets called on Windows outside the search, so skip the tests (or come up with a different way of checking). - describeOrSkipOnWindows('whether perform search for local files', () => { + describe('whether perform search for local files', () => { beforeEach(() => { existsSpy.mockImplementation(() => false); }); test('when no script arg or executableDir then no search for local file', () => { const program = new commander.Command(); + program._throwForMissingExecutable = () => {}; // suppress error, call mocked spawn program.name('pm'); program.command('sub', 'executable description'); program.parse(['sub'], { from: 'user' }); @@ -67,6 +68,7 @@ describe('search for subcommand', () => { test('when script arg then search for local files', () => { const program = new commander.Command(); + program._throwForMissingExecutable = () => {}; // suppress error, call mocked spawn program.name('pm'); program.command('sub', 'executable description'); program.parse(['node', 'script-name', 'sub']); @@ -76,6 +78,7 @@ describe('search for subcommand', () => { test('when executableDir then search for local files)', () => { const program = new commander.Command(); + program._throwForMissingExecutable = () => {}; // suppress error, call mocked spawn program.name('pm'); program.executableDir(__dirname); program.command('sub', 'executable description');