Another try at co-existing with unit tests!

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`);

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();
 });

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');