From 297224ddf49906d192fb94664169bd28f9af7696 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 17 Jul 2021 14:55:50 +1200 Subject: [PATCH 01/22] Simplify how default program name found. Just use args. --- lib/command.js | 28 +++++++++++++++++++++------- tests/command.chain.test.js | 6 ++++++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/command.js b/lib/command.js index 4d080b172..086efc077 100644 --- a/lib/command.js +++ b/lib/command.js @@ -768,8 +768,8 @@ Expecting one of '${allowedValues.join("', '")}'`); }; /** - * Get user arguments implied or explicit arguments. - * Side-effects: set _scriptPath if args included application, and use that to set implicit command name. + * Get user arguments from implied or explicit arguments. + * Side-effects: set _scriptPath if args included script, and use that to set implicit command name. * * @api private */ @@ -813,12 +813,10 @@ Expecting one of '${allowedValues.join("', '")}'`); default: throw new Error(`unexpected parse option { from: '${parseOptions.from}' }`); } - if (!this._scriptPath && require.main) { - this._scriptPath = require.main.filename; - } - // Guess name, used in usage in help. - this._name = this._name || (this._scriptPath && path.basename(this._scriptPath, path.extname(this._scriptPath))); + // Find a name for program, in case not supplied. + if (!this._name && this._scriptPath) this.nameFromFilename(this._scriptPath); + this._name = this._name || 'program'; return userArgs; } @@ -1613,6 +1611,22 @@ Expecting one of '${allowedValues.join("', '")}'`); return this; }; + /** + * Set the name of the command from script filename, say from process.argv[1], + * or require.main.filename, or __filename. (Or import.meta.url?) + * + * Note: not documented in README, but public for client use if proves useful. + * + * @param {string} filename + * @return {Command} + */ + + nameFromFilename(filename) { + this._name = path.basename(filename, path.extname(filename)); + + return this; + } + /** * Return program help documentation. * diff --git a/tests/command.chain.test.js b/tests/command.chain.test.js index e80c2b292..564e0834b 100644 --- a/tests/command.chain.test.js +++ b/tests/command.chain.test.js @@ -190,4 +190,10 @@ describe('Command methods that should return this for chaining', () => { const result = cmd.copyInheritedSettings(program); expect(result).toBe(cmd); }); + + test('when call .nameFromFilename() then returns this', () => { + const program = new Command(); + const result = program.nameFromFilename('name'); + expect(result).toBe(program); + }); }); From 995afa6b13d336b6b02ff9519160120a2ffa233a Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 17 Jul 2021 22:01:16 +1200 Subject: [PATCH 02/22] Rework search for executable files --- lib/command.js | 81 +++++++++++++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 37 deletions(-) diff --git a/lib/command.js b/lib/command.js index 086efc077..47b048476 100644 --- a/lib/command.js +++ b/lib/command.js @@ -814,7 +814,7 @@ Expecting one of '${allowedValues.join("', '")}'`); throw new Error(`unexpected parse option { from: '${parseOptions.from}' }`); } - // Find a name for program, in case not supplied. + // Find default name for program from arguments. if (!this._name && this._scriptPath) this.nameFromFilename(this._scriptPath); this._name = this._name || 'program'; @@ -882,57 +882,63 @@ Expecting one of '${allowedValues.join("', '")}'`); let launchWithNode = false; // Use node for source targets so do not need to get permissions correct, and on Windows. const sourceExt = ['.js', '.ts', '.tsx', '.mjs', '.cjs']; - // Not checking for help first. Unlikely to have mandatory and executable, and can't robustly test for help flags in external command. - this._checkForMissingMandatoryOptions(); + function findFile(baseName, baseDir) { + // Look for specified file + const localBin = path.resolve(baseDir, baseName); + if (fs.existsSync(localBin)) return localBin; - // Want the entry script as the reference for command name and directory for searching for other files. - let scriptPath = this._scriptPath; - // Fallback in case not set, due to how Command created or called. - if (!scriptPath && require.main) { - scriptPath = require.main.filename; - } + // Stop looking if candidate already has an expected extension. + if (sourceExt.includes(path.extname(baseName))) return undefined; - let baseDir; - try { - const resolvedLink = fs.realpathSync(scriptPath); - baseDir = path.dirname(resolvedLink); - } catch (e) { - baseDir = '.'; // dummy, probably not going to find executable! - } + // Try all the extensions. + const foundExt = sourceExt.find(ext => fs.existsSync(`${localBin}${ext}`)); + if (foundExt) return `${localBin}${foundExt}`; - // name of the subcommand, like `pm-install` - let bin = path.basename(scriptPath, path.extname(scriptPath)) + '-' + subcommand._name; - if (subcommand._executableFile) { - bin = subcommand._executableFile; + return undefined; } - const localBin = path.join(baseDir, bin); - if (fs.existsSync(localBin)) { - // prefer local `./` to bin in the $PATH - bin = localBin; - } else { - // Look for source files. - sourceExt.forEach((ext) => { - if (fs.existsSync(`${localBin}${ext}`)) { - bin = `${localBin}${ext}`; + // Not checking for help first. Unlikely to have mandatory and executable, and can't robustly test for help flags in external command. + this._checkForMissingMandatoryOptions(); + + let executableFile = subcommand._executableFile || `${this._name}-${subcommand._name}`; + let scriptDir; + // Look for a local file in preference to a command in PATH. + if (this._scriptPath) { + let resolvedScriptPath; // Follow links for installed npm bin on non-Windows. + try { + resolvedScriptPath = fs.realpathSync(this._scriptPath); + } catch (err) { + resolvedScriptPath = this._scriptPath; + } + scriptDir = path.dirname(resolvedScriptPath); + + let localFile = findFile(executableFile, scriptDir); + if (!localFile) { + const legacyName = `${path.basename(this._scriptPath, path.extname(this._scriptPath))}-${subcommand._name}`; + if (legacyName !== executableFile) { + localFile = findFile(legacyName, scriptDir); } - }); + } + executableFile = localFile || executableFile; + } else { + scriptDir = '[script not found in arguments]'; } - launchWithNode = sourceExt.includes(path.extname(bin)); + + launchWithNode = sourceExt.includes(path.extname(executableFile)); let proc; if (process.platform !== 'win32') { if (launchWithNode) { - args.unshift(bin); + args.unshift(executableFile); // add executable arguments to spawn args = incrementNodeInspectorPort(process.execArgv).concat(args); proc = childProcess.spawn(process.argv[0], args, { stdio: 'inherit' }); } else { - proc = childProcess.spawn(bin, args, { stdio: 'inherit' }); + proc = childProcess.spawn(executableFile, args, { stdio: 'inherit' }); } } else { - args.unshift(bin); + args.unshift(executableFile); // add executable arguments to spawn args = incrementNodeInspectorPort(process.execArgv).concat(args); proc = childProcess.spawn(process.execPath, args, { stdio: 'inherit' }); @@ -961,13 +967,14 @@ Expecting one of '${allowedValues.join("', '")}'`); proc.on('error', (err) => { // @ts-ignore if (err.code === 'ENOENT') { - const executableMissing = `'${bin}' does not exist + 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`; + - if the default executable name is not suitable, use the executableFile option to supply a custom name + - searched for subcommand relative to directory ${scriptDir}`; throw new Error(executableMissing); // @ts-ignore } else if (err.code === 'EACCES') { - throw new Error(`'${bin}' not executable`); + throw new Error(`'${executableFile}' not executable`); } if (!exitCallback) { process.exit(1); From 5136f8bf54aecc27ec738d7bc1ecac609fa703ae Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 12:53:56 +1200 Subject: [PATCH 03/22] Adjust error messaging, still WIP --- lib/command.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/command.js b/lib/command.js index 47b048476..2e1b6029c 100644 --- a/lib/command.js +++ b/lib/command.js @@ -920,8 +920,6 @@ Expecting one of '${allowedValues.join("', '")}'`); } } executableFile = localFile || executableFile; - } else { - scriptDir = '[script not found in arguments]'; } launchWithNode = sourceExt.includes(path.extname(executableFile)); @@ -967,10 +965,13 @@ Expecting one of '${allowedValues.join("', '")}'`); proc.on('error', (err) => { // @ts-ignore if (err.code === 'ENOENT') { + const scriptDirMessage = scriptDir + ? `searched for subcommand relative to directory ${scriptDir}` + : 'no script argument supplied, did not perform relative search for subcommand'; 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 - - searched for subcommand relative to directory ${scriptDir}`; + - ${scriptDirMessage}`; throw new Error(executableMissing); // @ts-ignore } else if (err.code === 'EACCES') { From f3e8f47b74b25ae4ad939d996193da4b3589f83f Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 15:55:20 +1200 Subject: [PATCH 04/22] Add executableDir and use searching for subcommands --- lib/command.js | 68 ++++++++++++++++++-------------- tests/command.addCommand.test.js | 9 ----- tests/command.chain.test.js | 2 +- 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/lib/command.js b/lib/command.js index 2e1b6029c..9a2d439a7 100644 --- a/lib/command.js +++ b/lib/command.js @@ -39,6 +39,7 @@ class Command extends EventEmitter { this._actionHandler = null; this._executableHandler = false; this._executableFile = null; // custom name for executable + this._executableDir = null; // custom search directory for subcommands this._defaultCommandName = null; this._exitCallback = null; this._aliases = []; @@ -243,19 +244,10 @@ class Command extends EventEmitter { */ addCommand(cmd, opts) { - if (!cmd._name) throw new Error('Command passed to .addCommand() must have a name'); - - // To keep things simple, block automatic name generation for deeply nested executables. - // Fail fast and detect when adding rather than later when parsing. - function checkExplicitNames(commandArray) { - commandArray.forEach((cmd) => { - if (cmd._executableHandler && !cmd._executableFile) { - throw new Error(`Must specify executableFile for deeply nested executable: ${cmd.name()}`); - } - checkExplicitNames(cmd.commands); - }); + if (!cmd._name) { + throw new Error(`Command passed to .addCommand() must have a name +- specify the name in Command constructor or using .name()`); } - checkExplicitNames(cmd.commands); opts = opts || {}; if (opts.isDefault) this._defaultCommandName = cmd._name; @@ -882,7 +874,7 @@ Expecting one of '${allowedValues.join("', '")}'`); let launchWithNode = false; // Use node for source targets so do not need to get permissions correct, and on Windows. const sourceExt = ['.js', '.ts', '.tsx', '.mjs', '.cjs']; - function findFile(baseName, baseDir) { + function findFile(baseDir, baseName) { // Look for specified file const localBin = path.resolve(baseDir, baseName); if (fs.existsSync(localBin)) return localBin; @@ -900,23 +892,28 @@ Expecting one of '${allowedValues.join("', '")}'`); // Not checking for help first. Unlikely to have mandatory and executable, and can't robustly test for help flags in external command. this._checkForMissingMandatoryOptions(); + // executableFile and executableDir might be full path, or just a name let executableFile = subcommand._executableFile || `${this._name}-${subcommand._name}`; - let scriptDir; - // Look for a local file in preference to a command in PATH. + let executableDir = this._executableDir || ''; if (this._scriptPath) { - let resolvedScriptPath; // Follow links for installed npm bin on non-Windows. + let resolvedScriptPath; // resolve possible symlink for installed npm binary try { resolvedScriptPath = fs.realpathSync(this._scriptPath); } catch (err) { resolvedScriptPath = this._scriptPath; } - scriptDir = path.dirname(resolvedScriptPath); + executableDir = path.resolve(path.dirname(resolvedScriptPath), executableDir); + } - let localFile = findFile(executableFile, scriptDir); - if (!localFile) { - const legacyName = `${path.basename(this._scriptPath, path.extname(this._scriptPath))}-${subcommand._name}`; - if (legacyName !== executableFile) { - localFile = findFile(legacyName, scriptDir); + // Look for a local file in preference to a command in PATH. + if (executableDir) { + let localFile = findFile(executableDir, executableFile); + + // Legacy search using prefix of script name instead of command name + if (!localFile && !subcommand._executableFile && this._scriptPath) { + const legacyName = path.basename(this._scriptPath, path.extname(this._scriptPath)); + if (legacyName !== this._name) { + localFile = findFile(executableDir, `${legacyName}-${subcommand._name}`); } } executableFile = localFile || executableFile; @@ -965,13 +962,13 @@ Expecting one of '${allowedValues.join("', '")}'`); proc.on('error', (err) => { // @ts-ignore if (err.code === 'ENOENT') { - const scriptDirMessage = scriptDir - ? `searched for subcommand relative to directory ${scriptDir}` - : 'no script argument supplied, did not perform relative search for subcommand'; + const executableDirMessage = executableDir + ? `searched for 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 - - ${scriptDirMessage}`; + - if the default executable name is not suitable, use the executableFile option to supply a custom name or path + - ${executableDirMessage}`; throw new Error(executableMissing); // @ts-ignore } else if (err.code === 'EACCES') { @@ -1621,9 +1618,9 @@ Expecting one of '${allowedValues.join("', '")}'`); /** * Set the name of the command from script filename, say from process.argv[1], - * or require.main.filename, or __filename. (Or import.meta.url?) + * or require.main.filename, or __filename. * - * Note: not documented in README, but public for client use if proves useful. + * (Used internally and public although not documented in README.) * * @param {string} filename * @return {Command} @@ -1635,6 +1632,19 @@ Expecting one of '${allowedValues.join("', '")}'`); return this; } + /** + * Get or set the directory for searching for executable subcommands of this command. + * + * @param {string} [path] + * @return {string|Command} + */ + + executableDir(path) { + if (path === undefined) return this._executableDir; + this._executableDir = path; + return this; + }; + /** * Return program help documentation. * diff --git a/tests/command.addCommand.test.js b/tests/command.addCommand.test.js index b20324c38..f8f86b682 100644 --- a/tests/command.addCommand.test.js +++ b/tests/command.addCommand.test.js @@ -51,15 +51,6 @@ test('when command without name passed to .addCommand then throw', () => { }).toThrow(); }); -test('when executable command without custom executableFile passed to .addCommand then throw', () => { - const program = new commander.Command(); - const cmd = new commander.Command('sub'); - cmd.command('exec', 'exec description'); - expect(() => { - program.addCommand(cmd); - }).toThrow(); -}); - test('when executable command with custom executableFile passed to .addCommand then ok', () => { const program = new commander.Command(); const cmd = new commander.Command('sub'); diff --git a/tests/command.chain.test.js b/tests/command.chain.test.js index 564e0834b..9c25a3db4 100644 --- a/tests/command.chain.test.js +++ b/tests/command.chain.test.js @@ -191,7 +191,7 @@ describe('Command methods that should return this for chaining', () => { expect(result).toBe(cmd); }); - test('when call .nameFromFilename() then returns this', () => { + test('when set .nameFromFilename() then returns this', () => { const program = new Command(); const result = program.nameFromFilename('name'); expect(result).toBe(program); From 243162604dcf58368c9236becc641c6be291e310 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 17:57:04 +1200 Subject: [PATCH 05/22] Wording improvements --- lib/command.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/command.js b/lib/command.js index 9a2d439a7..5d1051c42 100644 --- a/lib/command.js +++ b/lib/command.js @@ -761,7 +761,7 @@ Expecting one of '${allowedValues.join("', '")}'`); /** * Get user arguments from implied or explicit arguments. - * Side-effects: set _scriptPath if args included script, and use that to set implicit command name. + * Side-effects: set _scriptPath if args included script. Used for default program name, and subcommand searches. * * @api private */ @@ -963,7 +963,7 @@ Expecting one of '${allowedValues.join("', '")}'`); // @ts-ignore if (err.code === 'ENOENT') { const executableDirMessage = executableDir - ? `searched for subcommand relative to directory '${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 @@ -1604,7 +1604,7 @@ Expecting one of '${allowedValues.join("', '")}'`); }; /** - * Get or set the name of the command + * Get or set the name of the command. * * @param {string} [str] * @return {string|Command} @@ -1617,11 +1617,14 @@ Expecting one of '${allowedValues.join("', '")}'`); }; /** - * Set the name of the command from script filename, say from process.argv[1], + * Set the name of the command from script filename, such as process.argv[1], * or require.main.filename, or __filename. * * (Used internally and public although not documented in README.) * + * @example + * program.nameFromFilename(require.main.filename); + * * @param {string} filename * @return {Command} */ From 12aebd9a9b4d6681cbf50f5d7c3b6c2cae12318d Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 18:29:34 +1200 Subject: [PATCH 06/22] Add TypeScript for nameFromFilename --- typings/index.d.ts | 15 +++++++++++++++ typings/index.test-d.ts | 3 +++ 2 files changed, 18 insertions(+) diff --git a/typings/index.d.ts b/typings/index.d.ts index 2889e30e0..97043f849 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -691,6 +691,21 @@ export class Command { name(): string; /** + * Set the name of the command from script filename, such as process.argv[1], + * or require.main.filename, or __filename. + * + * (Used internally and public although not documented in README.) + * + * @example + * ```ts + * program.nameFromFilename(require.main.filename); + * ``` + * + * @returns `this` command for chaining + */ + nameFromFilename(filename: string): this; + + /** * Output help information for this command. * * Outputs built-in help, and custom text added using `.addHelpText()`. diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 237fb37ee..17cfce1f2 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -228,6 +228,9 @@ expectType(program.usage()); expectType(program.name('my-name')); expectType(program.name()); +// nameFromFilename +expectType(program.nameFromFilename(__filename)); + // outputHelp expectType(program.outputHelp()); expectType(program.outputHelp((str: string) => { return str; })); From 278053bad715a947c1db75a8a6e238dfe639b61a Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 18:49:59 +1200 Subject: [PATCH 07/22] Add new tests for name (and nameFromFilename) --- tests/command.name.test.js | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/tests/command.name.test.js b/tests/command.name.test.js index 290e32ffc..f0d39a0e1 100644 --- a/tests/command.name.test.js +++ b/tests/command.name.test.js @@ -1,5 +1,11 @@ +const path = require('path'); const commander = require('../'); +test('when construct with name then name is set', () => { + const program = new commander.Command('foo'); + expect(program.name()).toBe('foo'); +}); + test('when set program name and parse then name is as assigned', () => { const program = new commander.Command(); program.name('custom'); @@ -7,10 +13,16 @@ test('when set program name and parse then name is as assigned', () => { expect(program.name()).toBe('custom'); }); -test('when program name not set and parse then name is found from arguments', () => { +test('when program name not set and parse with script argument then plain name is found from script name', () => { const program = new commander.Command(); - program.parse(['node', 'test']); - expect(program.name()).toBe('test'); + program.parse(['node', path.resolve(process.cwd(), 'script.js')], { from: 'node' }); + expect(program.name()).toBe('script'); +}); + +test('when command name not set and no script argument in parse then name is program', () => { + const program = new commander.Command(); + program.parse([], { from: 'user' }); + expect(program.name()).toBe('program'); }); test('when add command then command is named', () => { @@ -26,3 +38,15 @@ test('when set program name then name appears in help', () => { const helpInformation = program.helpInformation(); expect(helpInformation).toMatch(/^Usage: custom-name/); }); + +test('when pass path to nameFromFilename then name is plain name', () => { + const program = new commander.Command(); + program.nameFromFilename(path.resolve(process.cwd(), 'foo.js')); + expect(program.name()).toBe('foo'); +}); + +test('when pass __filename to nameFromFilename then name is plain name of this file', () => { + const program = new commander.Command(); + program.nameFromFilename(__filename); + expect(program.name()).toBe('command.name.test'); +}); From 635c23ee62382bc27d15383899eaa57a76436eec Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 19:01:22 +1200 Subject: [PATCH 08/22] Add TypeScript for executableDir --- lib/command.js | 5 +++++ typings/index.d.ts | 20 +++++++++++++++++++- typings/index.test-d.ts | 4 ++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/command.js b/lib/command.js index 5d1051c42..53e9f087d 100644 --- a/lib/command.js +++ b/lib/command.js @@ -1638,6 +1638,11 @@ Expecting one of '${allowedValues.join("', '")}'`); /** * Get or set the directory for searching for executable subcommands of this command. * + * @example + * program.executableDir(__dirname); + * // or + * program.executableDir('subcommands'); + * * @param {string} [path] * @return {string|Command} */ diff --git a/typings/index.d.ts b/typings/index.d.ts index 97043f849..d6e9f5b5b 100644 --- a/typings/index.d.ts +++ b/typings/index.d.ts @@ -703,7 +703,25 @@ export class Command { * * @returns `this` command for chaining */ - nameFromFilename(filename: string): this; + nameFromFilename(filename: string): this; + + /** + * Set the directory for searching for executable subcommands of this command. + * + * @example + * ```ts + * program.executableDir(__dirname); + * // or + * program.executableDir('subcommands'); + * ``` + * + * @returns `this` command for chaining + */ + executableDir(path: string): this; + /** + * Get the executable search directory. + */ + executableDir(): string; /** * Output help information for this command. diff --git a/typings/index.test-d.ts b/typings/index.test-d.ts index 17cfce1f2..59a8c9d17 100644 --- a/typings/index.test-d.ts +++ b/typings/index.test-d.ts @@ -231,6 +231,10 @@ expectType(program.name()); // nameFromFilename expectType(program.nameFromFilename(__filename)); +// executableDir +expectType(program.executableDir(__dirname)); +expectType(program.executableDir()); + // outputHelp expectType(program.outputHelp()); expectType(program.outputHelp((str: string) => { return str; })); From 6620757f3caf97fb8c622b227704c5169edc0b6f Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 20:04:07 +1200 Subject: [PATCH 09/22] Start tests for subcommand search --- ...ommand.executableSubcommand.lookup.test.js | 2 + ...ommand.executableSubcommand.search.test.js | 110 ++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 tests/command.executableSubcommand.search.test.js diff --git a/tests/command.executableSubcommand.lookup.test.js b/tests/command.executableSubcommand.lookup.test.js index 419d0a537..d87a18257 100644 --- a/tests/command.executableSubcommand.lookup.test.js +++ b/tests/command.executableSubcommand.lookup.test.js @@ -4,6 +4,8 @@ const util = require('util'); const execFileAsync = util.promisify(childProcess.execFile); // Calling node explicitly so pm works without file suffix cross-platform. +// This file does end-to-end tests actually spawning program. +// See also command.executableSubcommand.search.test.js // Get false positives due to use of testOrSkipOnWindows /* eslint-disable jest/no-standalone-expect */ diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js new file mode 100644 index 000000000..b2f86053e --- /dev/null +++ b/tests/command.executableSubcommand.search.test.js @@ -0,0 +1,110 @@ +const childProcess = require('child_process'); +const fs = require('fs'); +const commander = require('../'); + +// This file does in-process mocking. +// See also command.executableSubcommand.lookup.test.js + +function extractMockSpawnArgs(mock) { + expect(mock).toHaveBeenCalled(); + // child_process.spawn(command[, args][, options]) + return mock.mock.calls[0][1]; +} + +function extractMockSpawnCommand(mock) { + expect(mock).toHaveBeenCalled(); + // child_process.spawn(command[, args][, options]) + return mock.mock.calls[0][0]; +} + +describe('search tests', () => { + let spawnSpy; + let existsSpy; + + beforeAll(() => { + spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => { + return { + on: () => {} + }; + }); + }); + + beforeEach(() => { + existsSpy = jest.spyOn(fs, 'existsSync'); + }); + + afterEach(() => { + spawnSpy.mockClear(); + existsSpy.mockRestore(); + }); + + afterAll(() => { + spawnSpy.mockRestore(); + }); + + test('when no script arg or executableDir then no search for local file', () => { + existsSpy.mockImplementation(() => false); + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + program.parse(['sub'], { from: 'user' }); + + expect(existsSpy).not.toHaveBeenCalled(); + }); + + test('when script arg then search for local files', () => { + existsSpy.mockImplementation(() => false); + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + program.parse(['node', 'script-name', 'sub']); + + expect(existsSpy).toHaveBeenCalled(); + }); + + test('when executableDir then search for local files)', () => { + existsSpy.mockImplementation(() => false); + const program = new commander.Command(); + program.name('pm'); + program.executableDir(__dirname); + program.command('sub', 'executable description'); + program.parse(['sub'], { from: 'user' }); + + expect(existsSpy).toHaveBeenCalled(); + }); + + test('when no script arg or executableDir then spawn program-sub (in PATH)', () => { + existsSpy.mockImplementation(() => false); + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + program.parse(['sub'], { from: 'user' }); + + expect(extractMockSpawnCommand(spawnSpy)).toEqual('pm-sub'); + }); + + test('when program named and script arg then spawn program-sub (in PATH)', () => { + existsSpy.mockImplementation(() => false); + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + program.parse(['node', 'script-name', 'sub']); + + expect(extractMockSpawnCommand(spawnSpy)).toEqual('pm-sub'); + }); + + test('when program not named and script path arg then spawn script-sub (in PATH)', () => { + existsSpy.mockImplementation(() => false); + const program = new commander.Command(); + program.command('sub', 'executable description'); + program.parse(['node', 'script.js', 'sub']); + + expect(extractMockSpawnCommand(spawnSpy)).toEqual('script-sub'); + }); + + // search directory + // - script path + // - script path symlink + // - executableDir + // legacy lookup +}); From 9128aa326fb68dee738877e4e10a32895965d6f5 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 20:20:08 +1200 Subject: [PATCH 10/22] Comment-out unused routine for lint, WIP --- tests/command.executableSubcommand.search.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index b2f86053e..f628eb23e 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -5,11 +5,11 @@ const commander = require('../'); // This file does in-process mocking. // See also command.executableSubcommand.lookup.test.js -function extractMockSpawnArgs(mock) { - expect(mock).toHaveBeenCalled(); - // child_process.spawn(command[, args][, options]) - return mock.mock.calls[0][1]; -} +// function extractMockSpawnArgs(mock) { +// expect(mock).toHaveBeenCalled(); +// // child_process.spawn(command[, args][, options]) +// return mock.mock.calls[0][1]; +// } function extractMockSpawnCommand(mock) { expect(mock).toHaveBeenCalled(); From fe764fda2489cb03173a78773f35d98b47eb9612 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 20:59:49 +1200 Subject: [PATCH 11/22] Refactor tests and skip launchable executable tests on Windows --- ...ommand.executableSubcommand.search.test.js | 111 ++++++++++-------- 1 file changed, 64 insertions(+), 47 deletions(-) diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index f628eb23e..67b9509c6 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -17,7 +17,9 @@ function extractMockSpawnCommand(mock) { return mock.mock.calls[0][0]; } -describe('search tests', () => { +const describeOrSkipOnWindows = (process.platform === 'win32') ? describe.skip : describe; + +describe('search for subcommand', () => { let spawnSpy; let existsSpy; @@ -42,64 +44,79 @@ describe('search tests', () => { spawnSpy.mockRestore(); }); - test('when no script arg or executableDir then no search for local file', () => { - existsSpy.mockImplementation(() => false); - const program = new commander.Command(); - program.name('pm'); - program.command('sub', 'executable description'); - program.parse(['sub'], { from: 'user' }); + describe('whether perform search for local files', () => { + beforeEach(() => { + existsSpy.mockImplementation(() => false); + }); - expect(existsSpy).not.toHaveBeenCalled(); - }); + test('when no script arg or executableDir then no search for local file', () => { + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + program.parse(['sub'], { from: 'user' }); - test('when script arg then search for local files', () => { - existsSpy.mockImplementation(() => false); - const program = new commander.Command(); - program.name('pm'); - program.command('sub', 'executable description'); - program.parse(['node', 'script-name', 'sub']); + expect(existsSpy).not.toHaveBeenCalled(); + }); - expect(existsSpy).toHaveBeenCalled(); - }); + test('when script arg then search for local files', () => { + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + program.parse(['node', 'script-name', 'sub']); - test('when executableDir then search for local files)', () => { - existsSpy.mockImplementation(() => false); - const program = new commander.Command(); - program.name('pm'); - program.executableDir(__dirname); - program.command('sub', 'executable description'); - program.parse(['sub'], { from: 'user' }); + expect(existsSpy).toHaveBeenCalled(); + }); + + test('when executableDir then search for local files)', () => { + const program = new commander.Command(); + program.name('pm'); + program.executableDir(__dirname); + program.command('sub', 'executable description'); + program.parse(['sub'], { from: 'user' }); - expect(existsSpy).toHaveBeenCalled(); + expect(existsSpy).toHaveBeenCalled(); + }); }); - test('when no script arg or executableDir then spawn program-sub (in PATH)', () => { - existsSpy.mockImplementation(() => false); - const program = new commander.Command(); - program.name('pm'); - program.command('sub', 'executable description'); - program.parse(['sub'], { from: 'user' }); + // We always use node on Windows, and don't spawn executable as the command. + describeOrSkipOnWindows('subcommand command name with no matching local file', () => { + beforeEach(() => { + existsSpy.mockImplementation(() => false); + }); + + test('when named pm and no script arg or executableDir then spawn pm-sub as command', () => { + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + program.parse(['sub'], { from: 'user' }); - expect(extractMockSpawnCommand(spawnSpy)).toEqual('pm-sub'); - }); + expect(extractMockSpawnCommand(spawnSpy)).toEqual('pm-sub'); + }); - test('when program named and script arg then spawn program-sub (in PATH)', () => { - existsSpy.mockImplementation(() => false); - const program = new commander.Command(); - program.name('pm'); - program.command('sub', 'executable description'); - program.parse(['node', 'script-name', 'sub']); + test('when named pm and script arg then still spawn pm-sub as command', () => { + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + program.parse(['node', 'script-name', 'sub']); - expect(extractMockSpawnCommand(spawnSpy)).toEqual('pm-sub'); - }); + expect(extractMockSpawnCommand(spawnSpy)).toEqual('pm-sub'); + }); - test('when program not named and script path arg then spawn script-sub (in PATH)', () => { - existsSpy.mockImplementation(() => false); - const program = new commander.Command(); - program.command('sub', 'executable description'); - program.parse(['node', 'script.js', 'sub']); + test('when no name and script arg then spawn script-sub as command', () => { + const program = new commander.Command(); + program.command('sub', 'executable description'); + program.parse(['node', 'script.js', 'sub']); - expect(extractMockSpawnCommand(spawnSpy)).toEqual('script-sub'); + expect(extractMockSpawnCommand(spawnSpy)).toEqual('script-sub'); + }); + + test('when named pm and script arg and executableFile then spawn executableFile', () => { + const program = new commander.Command(); + program.command('sub', 'executable description', { executableFile: 'myExecutable' }); + program.parse(['node', 'script.js', 'sub']); + + expect(extractMockSpawnCommand(spawnSpy)).toEqual('myExecutable'); + }); }); // search directory From 23285648f60cfbb92f09b47ae0e1a70a8813227c Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 18 Jul 2021 21:56:15 +1200 Subject: [PATCH 12/22] Add notes on further tests --- ...ommand.executableSubcommand.search.test.js | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index 67b9509c6..8493ebbe0 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -78,7 +78,7 @@ describe('search for subcommand', () => { }); }); - // We always use node on Windows, and don't spawn executable as the command. + // We always use node on Windows, and don't spawn executable as the command (which may be a feature or a shortcoming!?). describeOrSkipOnWindows('subcommand command name with no matching local file', () => { beforeEach(() => { existsSpy.mockImplementation(() => false); @@ -110,7 +110,7 @@ describe('search for subcommand', () => { expect(extractMockSpawnCommand(spawnSpy)).toEqual('script-sub'); }); - test('when named pm and script arg and executableFile then spawn executableFile', () => { + test('when named pm and script arg and executableFile then spawn executableFile as command', () => { const program = new commander.Command(); program.command('sub', 'executable description', { executableFile: 'myExecutable' }); program.parse(['node', 'script.js', 'sub']); @@ -119,9 +119,15 @@ describe('search for subcommand', () => { }); }); - // search directory - // - script path - // - script path symlink - // - executableDir - // legacy lookup + // search (needs a directory) + // - arg pwd/script searches in pwd + // - executableDir searches in executableDir + // - named pm, dir, local pm-sub (Mac/Win difference in whether uses node) + // - named pm, dir, local pm-sub.js + // - arg pwd/script, local script-sub.js + // - arg pwd/script.js, local script-sub.js + // - arg symlink to pwd/script.js, local symlink-sub.js [LEGACY] + // - relative executableDir + scriptDir + // - relative scriptDir alone (no) + // - relative executableDir (no) }); From aa0ede402c3effe1e9e2043a6b2ce05d404619a1 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sat, 24 Jul 2021 20:01:05 +1200 Subject: [PATCH 13/22] Filling out mock executable tests --- ...ommand.executableSubcommand.lookup.test.js | 2 +- ...ommand.executableSubcommand.search.test.js | 108 ++++++++++++++++-- 2 files changed, 102 insertions(+), 8 deletions(-) diff --git a/tests/command.executableSubcommand.lookup.test.js b/tests/command.executableSubcommand.lookup.test.js index d87a18257..7ff011c89 100644 --- a/tests/command.executableSubcommand.lookup.test.js +++ b/tests/command.executableSubcommand.lookup.test.js @@ -7,7 +7,7 @@ const execFileAsync = util.promisify(childProcess.execFile); // This file does end-to-end tests actually spawning program. // See also command.executableSubcommand.search.test.js -// Get false positives due to use of testOrSkipOnWindows +// Suppress false positive warnings due to use of testOrSkipOnWindows /* eslint-disable jest/no-standalone-expect */ const testOrSkipOnWindows = (process.platform === 'win32') ? test.skip : test; diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index 8493ebbe0..ecb7c0423 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -1,15 +1,19 @@ const childProcess = require('child_process'); const fs = require('fs'); +const path = require('path'); const commander = require('../'); // This file does in-process mocking. // See also command.executableSubcommand.lookup.test.js -// function extractMockSpawnArgs(mock) { -// expect(mock).toHaveBeenCalled(); -// // child_process.spawn(command[, args][, options]) -// return mock.mock.calls[0][1]; -// } +const gLocalDirectory = path.resolve(__dirname, 'fixtures'); // Real directory, although not actually searching for files in it. + +function extractMockSpawnArgs(mock) { + expect(mock).toHaveBeenCalled(); + // non-Win, launchWithNode: childProcess.spawn(process.argv[0], args, { stdio: 'inherit' }); + // Win always: childProcess.spawn(process.execPath, args, { stdio: 'inherit' }); + return mock.mock.calls[0][1]; +} function extractMockSpawnCommand(mock) { expect(mock).toHaveBeenCalled(); @@ -119,8 +123,93 @@ describe('search for subcommand', () => { }); }); - // search (needs a directory) - // - arg pwd/script searches in pwd + describe('subcommand command name with matching local file', () => { + test('when construct with name pm and script arg then spawn local pm-sub.js as command', () => { + const program = new commander.Command('pm'); + program.command('sub', 'executable description'); + + const localPath = path.resolve(gLocalDirectory, 'pm-sub.js'); + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['node', path.resolve(gLocalDirectory, 'script.js'), 'sub']); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); + + test('when name pm and script arg then spawn local pm-sub.js as command', () => { + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + + const localPath = path.resolve(gLocalDirectory, 'pm-sub.js'); + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['node', path.resolve(gLocalDirectory, 'script.js'), 'sub']); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); + + test('when script arg then spawn local script-sub.js as command', () => { + const program = new commander.Command(); + program.command('sub', 'executable description'); + + const localPath = path.resolve(gLocalDirectory, 'script-sub.js'); + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['node', path.resolve(gLocalDirectory, 'script.js'), 'sub']); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); + + test('when name pm and executableDir then spawn local pm-sub.js as command', () => { + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + + const execDir = path.resolve(gLocalDirectory, 'exec-dir'); + program.executableDir(execDir); + const localPath = path.resolve(execDir, 'pm-sub.js'); + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['sub'], { from: 'user' }); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); + }); + + describe('search for local file', () => { + test('when script arg then search for local script-sub.js, .ts, .tsx, .mpjs, .cjs', () => { + existsSpy.mockImplementation((path) => false); + const program = new commander.Command(); + program.command('sub', 'executable description'); + const scriptPath = path.resolve(gLocalDirectory, 'script'); + program.parse(['node', scriptPath, 'sub']); + const sourceExt = ['.js', '.ts', '.tsx', '.mjs', '.cjs']; + sourceExt.forEach((ext) => { + expect(existsSpy).toHaveBeenCalledWith(path.resolve(gLocalDirectory, `script-sub${ext}`)); + }); + }); + + test('when script.js arg and local script-sub.js then spawns local script-sub.js', () => { + const program = new commander.Command(); + program.command('sub', 'executable description'); + const scriptPath = path.resolve(gLocalDirectory, 'script.js'); + const subPath = path.resolve(gLocalDirectory, 'script-sub.js'); + existsSpy.mockImplementation((path) => path === subPath); + program.parse(['node', scriptPath, 'sub']); + expect(existsSpy).toHaveBeenCalledWith(subPath); + expect(extractMockSpawnArgs(spawnSpy)).toEqual([subPath]); + }); + + test('when script.js arg and local script-sub.js then spawns local script-sub', () => { + const program = new commander.Command(); + program.command('sub', 'executable description'); + const scriptPath = path.resolve(gLocalDirectory, 'script.js'); + const subPath = path.resolve(gLocalDirectory, 'script-sub.js'); + existsSpy.mockImplementation((path) => path === subPath); + program.parse(['node', scriptPath, 'sub']); + expect(existsSpy).toHaveBeenCalledWith(subPath); + expect(extractMockSpawnArgs(spawnSpy)).toEqual([subPath]); + }); + }); + + // search (which needs a directory) // - executableDir searches in executableDir // - named pm, dir, local pm-sub (Mac/Win difference in whether uses node) // - named pm, dir, local pm-sub.js @@ -130,4 +219,9 @@ describe('search for subcommand', () => { // - relative executableDir + scriptDir // - relative scriptDir alone (no) // - relative executableDir (no) + // relative executableFile (relative to search dir) + // absolute executableFile (overrides search dir) + + // local over global? Known global? non-Windows only? Perhaps not bother, out of scope. + // test for "node" being used to run script? i.e. process.execArg or process.argv[0]. Tricky and legacy, out-of-scope for current work. }); From a46828d6d4aa6dc549e117edd6a515a1c242de4d Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 26 Jul 2021 15:41:49 +1200 Subject: [PATCH 14/22] Expand tests --- ...ommand.executableSubcommand.search.test.js | 92 +++++++++++++++---- 1 file changed, 74 insertions(+), 18 deletions(-) diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index ecb7c0423..a3bee5cc5 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -3,8 +3,8 @@ const fs = require('fs'); const path = require('path'); const commander = require('../'); -// This file does in-process mocking. -// See also command.executableSubcommand.lookup.test.js +// This file does in-process mocking. Bit clumsy but faster and less external clutter than using fixtures. +// See also command.executableSubcommand.lookup.test.js for test using fixtures. const gLocalDirectory = path.resolve(__dirname, 'fixtures'); // Real directory, although not actually searching for files in it. @@ -83,7 +83,7 @@ describe('search for subcommand', () => { }); // We always use node on Windows, and don't spawn executable as the command (which may be a feature or a shortcoming!?). - describeOrSkipOnWindows('subcommand command name with no matching local file', () => { + describeOrSkipOnWindows('subcommand command name with no matching local file (non-Windows)', () => { beforeEach(() => { existsSpy.mockImplementation(() => false); }); @@ -124,7 +124,7 @@ describe('search for subcommand', () => { }); describe('subcommand command name with matching local file', () => { - test('when construct with name pm and script arg then spawn local pm-sub.js as command', () => { + test('when construct with name pm and script arg then spawn local pm-sub.js', () => { const program = new commander.Command('pm'); program.command('sub', 'executable description'); @@ -135,7 +135,7 @@ describe('search for subcommand', () => { expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); }); - test('when name pm and script arg then spawn local pm-sub.js as command', () => { + test('when name pm and script arg then spawn local pm-sub.js', () => { const program = new commander.Command(); program.name('pm'); program.command('sub', 'executable description'); @@ -147,7 +147,7 @@ describe('search for subcommand', () => { expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); }); - test('when script arg then spawn local script-sub.js as command', () => { + test('when script arg then spawn local script-sub.js', () => { const program = new commander.Command(); program.command('sub', 'executable description'); @@ -158,7 +158,20 @@ describe('search for subcommand', () => { expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); }); - test('when name pm and executableDir then spawn local pm-sub.js as command', () => { + test('when name pm and script arg and only script-sub.js then fallback to spawn local script-sub.js', () => { + const program = new commander.Command(); + program.name('pm'); + program.command('sub', 'executable description'); + + // Fallback for compatibility with Commander <= v8 + const localPath = path.resolve(gLocalDirectory, 'script-sub.js'); + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['node', path.resolve(gLocalDirectory, 'script.js'), 'sub']); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); + + test('when name pm and executableDir then spawn local pm-sub.js', () => { const program = new commander.Command(); program.name('pm'); program.command('sub', 'executable description'); @@ -171,6 +184,60 @@ describe('search for subcommand', () => { expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); }); + + test('when script arg and executableDir then spawn local execDir/script-sub.js', () => { + const program = new commander.Command(); + program.command('sub', 'executable description'); + + const execDir = path.resolve(gLocalDirectory, 'exec-dir'); + program.executableDir(execDir); + const localPath = path.resolve(execDir, 'script-sub.js'); + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['node', path.resolve(gLocalDirectory, 'scriptDir', 'script'), 'sub']); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); + + test('when script arg is link and and link-sub relative to link target then spawn local link-sub', () => { + const program = new commander.Command(); + program.command('sub', 'executable description'); + console.error('hello'); + + const linkPath = path.resolve(gLocalDirectory, 'link', 'link'); + const scriptPath = path.resolve(gLocalDirectory, 'script', 'script.js'); + const scriptSubPath = path.resolve(gLocalDirectory, 'script', 'link-sub.js'); + const realPathSyncSpy = jest.spyOn(fs, 'realpathSync').mockImplementation((path) => { + console.error(`Calling realPathSync with ${path}`); + return path === linkPath ? scriptPath : linkPath; + }); + existsSpy.mockImplementation((path) => path === scriptSubPath); + program.parse(['node', linkPath, 'sub']); + realPathSyncSpy.mockRestore(); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([scriptSubPath]); + }); + + test('when name pm and script arg and relative executableFile then spawn local exec.js', () => { + const program = new commander.Command('pm'); + const localPath = path.join('relative', 'exec.js'); + program.command('sub', 'executable description', { executableFile: localPath }); + + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['node', path.resolve(gLocalDirectory, 'script.js'), 'sub']); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); + + test('when name pm and script arg and absolute executableFile then spawn local exec.js', () => { + const program = new commander.Command('pm'); + const localPath = path.resolve(gLocalDirectory, 'absolute', 'exec.js'); + program.command('sub', 'executable description', { executableFile: localPath }); + + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['node', path.resolve(gLocalDirectory, 'script.js'), 'sub']); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); }); describe('search for local file', () => { @@ -196,17 +263,6 @@ describe('search for subcommand', () => { expect(existsSpy).toHaveBeenCalledWith(subPath); expect(extractMockSpawnArgs(spawnSpy)).toEqual([subPath]); }); - - test('when script.js arg and local script-sub.js then spawns local script-sub', () => { - const program = new commander.Command(); - program.command('sub', 'executable description'); - const scriptPath = path.resolve(gLocalDirectory, 'script.js'); - const subPath = path.resolve(gLocalDirectory, 'script-sub.js'); - existsSpy.mockImplementation((path) => path === subPath); - program.parse(['node', scriptPath, 'sub']); - expect(existsSpy).toHaveBeenCalledWith(subPath); - expect(extractMockSpawnArgs(spawnSpy)).toEqual([subPath]); - }); }); // search (which needs a directory) From 597e894f7a2f14add675c27c0861fd579791c147 Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 26 Jul 2021 15:50:22 +1200 Subject: [PATCH 15/22] Remove debugging and ToDo --- ...ommand.executableSubcommand.search.test.js | 29 ------------------- 1 file changed, 29 deletions(-) diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index a3bee5cc5..d51a84985 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -201,13 +201,11 @@ describe('search for subcommand', () => { test('when script arg is link and and link-sub relative to link target then spawn local link-sub', () => { const program = new commander.Command(); program.command('sub', 'executable description'); - console.error('hello'); const linkPath = path.resolve(gLocalDirectory, 'link', 'link'); const scriptPath = path.resolve(gLocalDirectory, 'script', 'script.js'); const scriptSubPath = path.resolve(gLocalDirectory, 'script', 'link-sub.js'); const realPathSyncSpy = jest.spyOn(fs, 'realpathSync').mockImplementation((path) => { - console.error(`Calling realPathSync with ${path}`); return path === linkPath ? scriptPath : linkPath; }); existsSpy.mockImplementation((path) => path === scriptSubPath); @@ -252,32 +250,5 @@ describe('search for subcommand', () => { expect(existsSpy).toHaveBeenCalledWith(path.resolve(gLocalDirectory, `script-sub${ext}`)); }); }); - - test('when script.js arg and local script-sub.js then spawns local script-sub.js', () => { - const program = new commander.Command(); - program.command('sub', 'executable description'); - const scriptPath = path.resolve(gLocalDirectory, 'script.js'); - const subPath = path.resolve(gLocalDirectory, 'script-sub.js'); - existsSpy.mockImplementation((path) => path === subPath); - program.parse(['node', scriptPath, 'sub']); - expect(existsSpy).toHaveBeenCalledWith(subPath); - expect(extractMockSpawnArgs(spawnSpy)).toEqual([subPath]); - }); }); - - // search (which needs a directory) - // - executableDir searches in executableDir - // - named pm, dir, local pm-sub (Mac/Win difference in whether uses node) - // - named pm, dir, local pm-sub.js - // - arg pwd/script, local script-sub.js - // - arg pwd/script.js, local script-sub.js - // - arg symlink to pwd/script.js, local symlink-sub.js [LEGACY] - // - relative executableDir + scriptDir - // - relative scriptDir alone (no) - // - relative executableDir (no) - // relative executableFile (relative to search dir) - // absolute executableFile (overrides search dir) - - // local over global? Known global? non-Windows only? Perhaps not bother, out of scope. - // test for "node" being used to run script? i.e. process.execArg or process.argv[0]. Tricky and legacy, out-of-scope for current work. }); From ae39f4f6c8425dc4b484dcb6005b6e62c475f705 Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 26 Jul 2021 15:57:25 +1200 Subject: [PATCH 16/22] Relative and absolute test for executableDir --- .../command.executableSubcommand.search.test.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index d51a84985..67d66eb86 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -185,7 +185,20 @@ describe('search for subcommand', () => { expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); }); - test('when script arg and executableDir then spawn local execDir/script-sub.js', () => { + test('when script arg and relative executableDir then spawn relative script-sub.js', () => { + const program = new commander.Command(); + program.command('sub', 'executable description'); + + const execDir = 'exec-dir'; + program.executableDir(execDir); + const localPath = path.resolve(gLocalDirectory, execDir, 'script-sub.js'); + existsSpy.mockImplementation((path) => path === localPath); + program.parse(['node', path.resolve(gLocalDirectory, 'script'), 'sub']); + + expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + }); + + test('when script arg and absolute executableDir then spawn absolute script-sub.js', () => { const program = new commander.Command(); program.command('sub', 'executable description'); @@ -193,7 +206,7 @@ describe('search for subcommand', () => { program.executableDir(execDir); const localPath = path.resolve(execDir, 'script-sub.js'); existsSpy.mockImplementation((path) => path === localPath); - program.parse(['node', path.resolve(gLocalDirectory, 'scriptDir', 'script'), 'sub']); + program.parse(['node', path.resolve(gLocalDirectory, 'script-Dir', 'script'), 'sub']); expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); }); From eab72f532adb00e175c2992458874d3c9f08fecc Mon Sep 17 00:00:00 2001 From: John Gee Date: Mon, 26 Jul 2021 16:31:59 +1200 Subject: [PATCH 17/22] Avoid memory leak warning --- lib/command.js | 18 ++++++++++-------- ...command.executableSubcommand.search.test.js | 3 ++- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/command.js b/lib/command.js index 53e9f087d..fa7b9d535 100644 --- a/lib/command.js +++ b/lib/command.js @@ -939,15 +939,17 @@ Expecting one of '${allowedValues.join("', '")}'`); proc = childProcess.spawn(process.execPath, args, { stdio: 'inherit' }); } - const signals = ['SIGUSR1', 'SIGUSR2', 'SIGTERM', 'SIGINT', 'SIGHUP']; - signals.forEach((signal) => { - // @ts-ignore - process.on(signal, () => { - if (proc.killed === false && proc.exitCode === null) { - proc.kill(signal); - } + if (!proc.killed) { // testing mainly to avoid leak warnings during unit tests with mocked spawn + const signals = ['SIGUSR1', 'SIGUSR2', 'SIGTERM', 'SIGINT', 'SIGHUP']; + signals.forEach((signal) => { + // @ts-ignore + process.on(signal, () => { + if (proc.killed === false && proc.exitCode === null) { + proc.kill(signal); + } + }); }); - }); + } // By default terminate process when spawned process terminates. // Suppressing the exit if exitCallback defined is a bit messy and of limited use, but does allow process to stay running! diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index 67d66eb86..5b9e3ed7f 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -30,7 +30,8 @@ describe('search for subcommand', () => { beforeAll(() => { spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => { return { - on: () => {} + on: () => {}, + killed: true }; }); }); From 48838f506e7139b446e2e122d7063ae59ada8016 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 1 Aug 2021 11:08:35 +1200 Subject: [PATCH 18/22] Fix relative search test --- tests/command.executableSubcommand.search.test.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/command.executableSubcommand.search.test.js b/tests/command.executableSubcommand.search.test.js index 5b9e3ed7f..74180560e 100644 --- a/tests/command.executableSubcommand.search.test.js +++ b/tests/command.executableSubcommand.search.test.js @@ -4,7 +4,7 @@ const path = require('path'); const commander = require('../'); // This file does in-process mocking. Bit clumsy but faster and less external clutter than using fixtures. -// See also command.executableSubcommand.lookup.test.js for test using fixtures. +// See also command.executableSubcommand.lookup.test.js for tests using fixtures. const gLocalDirectory = path.resolve(__dirname, 'fixtures'); // Real directory, although not actually searching for files in it. @@ -232,12 +232,13 @@ describe('search for subcommand', () => { test('when name pm and script arg and relative executableFile then spawn local exec.js', () => { const program = new commander.Command('pm'); const localPath = path.join('relative', 'exec.js'); + const absolutePath = path.resolve(gLocalDirectory, localPath); program.command('sub', 'executable description', { executableFile: localPath }); - existsSpy.mockImplementation((path) => path === localPath); + existsSpy.mockImplementation((path) => path === absolutePath); program.parse(['node', path.resolve(gLocalDirectory, 'script.js'), 'sub']); - expect(extractMockSpawnArgs(spawnSpy)).toEqual([localPath]); + expect(extractMockSpawnArgs(spawnSpy)).toEqual([absolutePath]); }); test('when name pm and script arg and absolute executableFile then spawn local exec.js', () => { From 107f1f53b24cb8c49e23b3f498540804e3244a02 Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 1 Aug 2021 12:57:34 +1200 Subject: [PATCH 19/22] Separate and expand .name section in README. And some lint, remove trailing spaces et al --- Readme.md | 50 +++++++++++++++++++++++++++++-------------------- Readme_zh-CN.md | 8 +++----- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/Readme.md b/Readme.md index 1c0129d2b..c8ced20dc 100644 --- a/Readme.md +++ b/Readme.md @@ -32,7 +32,8 @@ Read this in other languages: English | [简体中文](./Readme_zh-CN.md) - [Custom help](#custom-help) - [Display help after errors](#display-help-after-errors) - [Display help from code](#display-help-from-code) - - [.usage and .name](#usage-and-name) + - [.name](#name) + - [.usage](#usage) - [.helpOption(flags, description)](#helpoptionflags-description) - [.addHelpCommand()](#addhelpcommand) - [More configuration](#more-configuration-2) @@ -66,7 +67,6 @@ This is used in the examples in this README for brevity. ```js const { program } = require('commander'); -program.version('0.0.1'); ``` For larger programs which may use commander in multiple ways, including unit testing, it is better to create a local Command object to use. @@ -74,7 +74,6 @@ For larger programs which may use commander in multiple ways, including unit tes ```js const { Command } = require('commander'); const program = new Command(); -program.version('0.0.1'); ``` For named imports in ECMAScript modules, import from `commander/esm.mjs`. @@ -93,7 +92,6 @@ import { Command } from 'commander'; const program = new Command(); ``` - ## Options Options are defined with the `.option()` method, also serving as documentation for the options. Each option can have a short flag (single character) and a long name, separated by a comma or space or vertical bar ('|'). @@ -113,7 +111,7 @@ By default options on the command line are not positional, and can be specified ### Common option types, boolean and value The two most used option types are a boolean option, and an option which takes its value -from the following argument (declared with angle brackets like `--expect `). Both are `undefined` unless specified on command line. +from the following argument (declared with angle brackets like `--expect `). Both are `undefined` unless specified on command line. Example file: [options-common.js](./examples/options-common.js) @@ -426,7 +424,7 @@ program .addCommand(build.makeBuildCommand()); ``` -Configuration options can be passed with the call to `.command()` and `.addCommand()`. Specifying `hidden: true` will +Configuration options can be passed with the call to `.command()` and `.addCommand()`. Specifying `hidden: true` will remove the command from the generated help output. Specifying `isDefault: true` will run the subcommand if no other subcommand is specified ([example](./examples/defaultCommand.js)). @@ -436,7 +434,7 @@ For subcommands, you can specify the argument syntax in the call to `.command()` is the only method usable for subcommands implemented using a stand-alone executable, but for other subcommands you can instead use the following method. -To configure a command, you can use `.argument()` to specify each expected command-argument. +To configure a command, you can use `.argument()` to specify each expected command-argument. You supply the argument name and an optional description. The argument may be `` or `[optional]`. You can specify a default value for an optional command-argument. @@ -513,7 +511,7 @@ program ### Action handler The action handler gets passed a parameter for each command-argument you declared, and two additional parameters -which are the parsed options and the command object itself. +which are the parsed options and the command object itself. Example file: [thank.js](./examples/thank.js) @@ -630,7 +628,7 @@ shell spawn --help ### Custom help -You can add extra text to be displayed along with the built-in help. +You can add extra text to be displayed along with the built-in help. Example file: [custom-help](./examples/custom-help) @@ -664,7 +662,7 @@ The positions in order displayed are: - `after`: display extra information after built-in help - `afterAll`: add to the program for a global footer (epilog) -The positions "beforeAll" and "afterAll" apply to the command and all its subcommands. +The positions "beforeAll" and "afterAll" apply to the command and all its subcommands. The second parameter can be a string, or a function returning a string. The function is passed a context object for your convenience. The properties are: @@ -673,7 +671,7 @@ The second parameter can be a string, or a function returning a string. The func ### Display help after errors -The default behaviour for usage errors is to just display a short error message. +The default behaviour for usage errors is to just display a short error message. You can change the behaviour to show the full help or a custom help message after an error. ```js @@ -696,10 +694,23 @@ error: unknown option '--unknown' `.helpInformation()`: get the built-in command help information as a string for processing or displaying yourself. -### .usage and .name +### .name -These allow you to customise the usage description in the first line of the help. The name is otherwise -deduced from the (full) program arguments. Given: +The command name appears in the help, and is also used for locating stand-alone executable subcommands. +You may specify the name using `.name()` or in the constructor. + +```js +program.name('pizza'); +const pm = new Command('pm'); +``` + +For the program, Commander will +fallback to using the script name from the full arguments passed into `.parse()`. However, the script name varies +depending on how your program is launched so you may wish to specify it explicitly. + +### .usage + +This allows you to customise the usage description in the first line of the help. Given: ```js program @@ -715,7 +726,7 @@ Usage: my-command [global options] command ### .helpOption(flags, description) -By default every command has a help option. Override the default help flags and description. Pass false to disable the built-in help option. +By default every command has a help option. You may change the default help flags and description. Pass false to disable the built-in help option. ```js program @@ -747,7 +758,7 @@ There are methods getting the visible lists of arguments, options, and subcomman Example file: [configure-help.js](./examples/configure-help.js) -``` +```js program.configureHelp({ sortSubcommands: true, subcommandTerm: (cmd) => cmd.name() // Just show the name, instead of short usage. @@ -809,7 +820,7 @@ program subcommand -b By default options are recognised before and after command-arguments. To only process options that come before the command-arguments, use `.passThroughOptions()`. This lets you pass the arguments and following options through to another program -without needing to use `--` to end the option processing. +without needing to use `--` to end the option processing. To use pass through options in a subcommand, the program needs to enable positional options. Example file: [pass-through-options.js](./examples/pass-through-options.js) @@ -826,7 +837,7 @@ By default the option processing shows an error for an unknown option. To have a By default the argument processing does not display an error for more command-arguments than expected. To display an error for excess arguments, use`.allowExcessArguments(false)`. -### Legacy options as properties +### Legacy options as properties Before Commander 7, the option values were stored as properties on the command. This was convenient to code but the downside was possible clashes with @@ -903,7 +914,6 @@ You can modify this behaviour for custom applications. In addition, you can modi Example file: [configure-output.js](./examples/configure-output.js) - ```js function errorColor(str) { // Add ANSI escape codes to display text in red. @@ -986,7 +996,7 @@ Examples: $ deploy exec sequential $ deploy exec async` ); - + program.parse(process.argv); ``` diff --git a/Readme_zh-CN.md b/Readme_zh-CN.md index 8b9704e24..3d480d63a 100644 --- a/Readme_zh-CN.md +++ b/Readme_zh-CN.md @@ -64,7 +64,6 @@ npm install commander ```js const { program } = require('commander'); -program.version('0.0.1'); ``` 如果程序较为复杂，用户需要以多种方式来使用 Commander，如单元测试等。创建本地 Command 对象是一种更好的方式： @@ -72,7 +71,6 @@ program.version('0.0.1'); ```js const { Command } = require('commander'); const program = new Command(); -program.version('0.0.1'); ``` ## 选项 @@ -254,6 +252,7 @@ $ collect --letter -n 1 -n 2 3 -- operand Options: { number: [ '1', '2', '3' ], letter: true } Remaining arguments: [ 'operand' ] ``` + 关于可能有歧义的用例，请见[可变参数的选项](./docs/zh-CN/%E5%8F%AF%E5%8F%98%E5%8F%82%E6%95%B0%E7%9A%84%E9%80%89%E9%A1%B9.md)。 ### 版本选项 @@ -466,7 +465,6 @@ async function main() { } ``` - 在命令行上使用命令时，选项和命令参数必须是合法的，使用未知的选项，或缺少所需的命令参数，会提示异常。 如要允许使用未知的选项，可以调用`.allowUnknownOption()`。默认情况下，传入过多的参数并不报错，但也可以通过调用`.allowExcessArguments(false)`来启用过多参数的报错。 @@ -612,6 +610,7 @@ program.addHelpCommand('assist [command]', 'show assistance'); 内建帮助信息通过Help类进行格式化。如有需要，可以使用`.configureHelp()`来更改其数据属性和方法，或使用`.createHelp()`来创建子类，从而配置Help类的行为。 数据属性包括： + - `helpWidth`：指明帮助信息的宽度。可在单元测试中使用。 - `sortSubcommands`：以字母序排列子命令 - `sortOptions`：以字母序排列选项 @@ -620,7 +619,7 @@ program.addHelpCommand('assist [command]', 'show assistance'); 示例代码： [configure-help.js](./examples/configure-help.js) -``` +```js program.configureHelp({ sortSubcommands: true, subcommandTerm: (cmd) => cmd.name() // Just show the name, instead of short usage. @@ -779,7 +778,6 @@ Commander默认用作命令行应用，其输出写入stdout和stderr。 示例代码：[configure-output.js](./examples/configure-output.js) - ```js function errorColor(str) { // 添加ANSI转义字符，以将文本输出为红色 From 3b38d2f8734e216a93bc3688e440b74f6a66f9ee Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 1 Aug 2021 13:01:26 +1200 Subject: [PATCH 20/22] Add .name to multiple subcommand example --- Readme.md | 1 + Readme_zh-CN.md | 1 + examples/deploy | 1 + 3 files changed, 3 insertions(+) diff --git a/Readme.md b/Readme.md index c8ced20dc..09fcaeb2f 100644 --- a/Readme.md +++ b/Readme.md @@ -970,6 +970,7 @@ const { Command } = require('commander'); const program = new Command(); program + .name('deploy') .version('0.0.1') .option('-c, --config ', 'set config path', './deploy.conf'); diff --git a/Readme_zh-CN.md b/Readme_zh-CN.md index 3d480d63a..176f79c79 100644 --- a/Readme_zh-CN.md +++ b/Readme_zh-CN.md @@ -834,6 +834,7 @@ const { Command } = require('commander'); const program = new Command(); program + .name('deploy') .version('0.0.1') .option('-c, --config ', 'set config path', './deploy.conf'); diff --git a/examples/deploy b/examples/deploy index 9c769dfd0..d24b4b6ab 100755 --- a/examples/deploy +++ b/examples/deploy @@ -5,6 +5,7 @@ const { Command } = require('../'); // include commander in git clone of command const program = new Command(); program + .name('deploy') .version('0.0.1') .option('-c, --config ', 'set config path', './deploy.conf'); From 353248fae9963659063139a6080926894e674eff Mon Sep 17 00:00:00 2001 From: John Gee Date: Sun, 1 Aug 2021 13:25:33 +1200 Subject: [PATCH 21/22] Update description of executable search. Add mention of .executableDir --- Readme.md | 6 ++++-- Readme_zh-CN.md | 1 + examples/pm | 1 + examples/pm-install | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Readme.md b/Readme.md index 09fcaeb2f..394b5335f 100644 --- a/Readme.md +++ b/Readme.md @@ -548,8 +548,9 @@ pass more arguments than declared, but you can make this an error with `.allowEx ### Stand-alone executable (sub)commands When `.command()` is invoked with a description argument, this tells Commander that you're going to use stand-alone executables for subcommands. -Commander will search the executables in the directory of the entry script (like `./examples/pm`) with the name `program-subcommand`, like `pm-install`, `pm-search`. -You can specify a custom name with the `executableFile` configuration option. +Commander will search the files in the directory of the entry script for a file with the name combination `command-subcommand`, like `pm-install` or `pm-search` in the example below. The search includes trying common file extensions, like `.js`. +You may specify a custom name (and path) with the `executableFile` configuration option. +You may specify a custom search directory for subcommands with `.executableDir()`. You handle the options for an executable (sub)command in the executable, and don't declare them at the top-level. @@ -557,6 +558,7 @@ Example file: [pm](./examples/pm) ```js program + .name('pm') .version('0.1.0') .command('install [name]', 'install one or more packages') .command('search [query]', 'search with optional query') diff --git a/Readme_zh-CN.md b/Readme_zh-CN.md index 176f79c79..eab7b11d3 100644 --- a/Readme_zh-CN.md +++ b/Readme_zh-CN.md @@ -479,6 +479,7 @@ Commander 将会尝试在入口脚本（例如 `./examples/pm`）的目录中搜 ```js program + .name('pm') .version('0.1.0') .command('install [name]', 'install one or more packages') .command('search [query]', 'search with optional query') diff --git a/examples/pm b/examples/pm index fa838968e..262c36db8 100755 --- a/examples/pm +++ b/examples/pm @@ -5,6 +5,7 @@ const { Command } = require('../'); // include commander in git clone of command const program = new Command(); program + .name('pm') .version('0.0.1') .description('Fake package manager') .command('install [name]', 'install one or more packages').alias('i') diff --git a/examples/pm-install b/examples/pm-install index f4e6e9cfb..2648bd61c 100755 --- a/examples/pm-install +++ b/examples/pm-install @@ -17,7 +17,7 @@ if (!pkgs.length) { } console.log(); -if (program.force) console.log(' force: install'); +if (program.opts().force) console.log(' force: install'); pkgs.forEach(function(pkg) { console.log(' install : %s', pkg); }); From d10f87e5040cbccf7a44b47e8ca012491d43dad0 Mon Sep 17 00:00:00 2001 From: John Gee Date: Wed, 4 Aug 2021 19:11:32 +1200 Subject: [PATCH 22/22] Add coverage of naming subcommands. --- Readme.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Readme.md b/Readme.md index 394b5335f..85b7e02af 100644 --- a/Readme.md +++ b/Readme.md @@ -699,16 +699,18 @@ error: unknown option '--unknown' ### .name The command name appears in the help, and is also used for locating stand-alone executable subcommands. -You may specify the name using `.name()` or in the constructor. + +You may specify the program name using `.name()` or in the Command constructor. For the program, Commander will +fallback to using the script name from the full arguments passed into `.parse()`. However, the script name varies +depending on how your program is launched so you may wish to specify it explicitly. ```js program.name('pizza'); const pm = new Command('pm'); ``` -For the program, Commander will -fallback to using the script name from the full arguments passed into `.parse()`. However, the script name varies -depending on how your program is launched so you may wish to specify it explicitly. +Subcommands get a name when specified using `.command()`. If you create the subcommand yourself to use with `.addCommand()`, +then set the name using `.name()` or in the Command constructor. ### .usage