diff --git a/index.js b/index.js index 73deef361..d946f3ffe 100644 --- a/index.js +++ b/index.js @@ -3,7 +3,7 @@ */ const EventEmitter = require('events').EventEmitter; -const spawn = require('child_process').spawn; +const childProcess = require('child_process'); const path = require('path'); const fs = require('fs'); @@ -1335,15 +1335,15 @@ class Command extends EventEmitter { // add executable arguments to spawn args = incrementNodeInspectorPort(process.execArgv).concat(args); - proc = spawn(process.argv[0], args, { stdio: 'inherit' }); + proc = childProcess.spawn(process.argv[0], args, { stdio: 'inherit' }); } else { - proc = spawn(bin, args, { stdio: 'inherit' }); + proc = childProcess.spawn(bin, args, { stdio: 'inherit' }); } } else { args.unshift(bin); // add executable arguments to spawn args = incrementNodeInspectorPort(process.execArgv).concat(args); - proc = spawn(process.execPath, args, { stdio: 'inherit' }); + proc = childProcess.spawn(process.execPath, args, { stdio: 'inherit' }); } const signals = ['SIGUSR1', 'SIGUSR2', 'SIGTERM', 'SIGINT', 'SIGHUP']; @@ -1672,13 +1672,8 @@ class Command extends EventEmitter { * @api private */ - optionMissingArgument(option, flag) { - let message; - if (flag) { - message = `error: option '${option.flags}' argument missing, got '${flag}'`; - } else { - message = `error: option '${option.flags}' argument missing`; - } + optionMissingArgument(option) { + const message = `error: option '${option.flags}' argument missing`; this._displayError(1, 'commander.optionMissingArgument', message); }; diff --git a/tests/command.addHelpText.test.js b/tests/command.addHelpText.test.js index ca50ccc4e..e0bac2895 100644 --- a/tests/command.addHelpText.test.js +++ b/tests/command.addHelpText.test.js @@ -77,6 +77,13 @@ describe('program calls to addHelpText', () => { expect(writeSpy).toHaveBeenNthCalledWith(4, 'after\n'); expect(writeSpy).toHaveBeenNthCalledWith(5, 'afterAll\n'); }); + + test('when "silly" position then throw', () => { + const program = new commander.Command(); + expect(() => { + program.addHelpText('silly', 'text'); + }).toThrow(); + }); }); describe('program and subcommand calls to addHelpText', () => { diff --git a/tests/command.alias.test.js b/tests/command.alias.test.js index 5b90d110c..73a9b8f29 100644 --- a/tests/command.alias.test.js +++ b/tests/command.alias.test.js @@ -87,3 +87,12 @@ test('when set aliases then can get aliases', () => { program.aliases(aliases); expect(program.aliases()).toEqual(aliases); }); + +test('when set alias on executable then can get alias', () => { + const program = new commander.Command(); + const alias = 'abcde'; + program + .command('external', 'external command') + .alias(alias); + expect(program.commands[0].alias()).toEqual(alias); +}); diff --git a/tests/command.exitOverride.test.js b/tests/command.exitOverride.test.js index 27b7c45c5..a951baeef 100644 --- a/tests/command.exitOverride.test.js +++ b/tests/command.exitOverride.test.js @@ -278,3 +278,26 @@ describe('.exitOverride and error details', () => { expectCommanderError(caughtErr, 1, 'commander.invalidOptionArgument', "error: option '--colour ' argument 'green' is invalid. NO"); }); }); + +test('when no override and error then exit(1)', () => { + const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => { }); + const program = new commander.Command(); + program.configureOutput({ outputError: () => {} }); + program.parse(['--unknownOption'], { from: 'user' }); + expect(exitSpy).toHaveBeenCalledWith(1); + exitSpy.mockRestore(); +}); + +test('when custom processing throws custom error then throw custom error', () => { + function justSayNo(value) { + throw new Error('custom'); + } + const program = new commander.Command(); + program + .exitOverride() + .option('-s, --shade ', 'specify shade', justSayNo); + + expect(() => { + program.parse(['--shade', 'green'], { from: 'user' }); + }).toThrow('custom'); +}); diff --git a/tests/command.help.test.js b/tests/command.help.test.js index a2f673642..069f00d20 100644 --- a/tests/command.help.test.js +++ b/tests/command.help.test.js @@ -88,6 +88,13 @@ test('when call outputHelp(cb) then display cb output', () => { writeSpy.mockClear(); }); +test('when call deprecated outputHelp(cb) with wrong callback return type then throw', () => { + const program = new commander.Command(); + expect(() => { + program.outputHelp((helpInformation) => 3); + }).toThrow(); +}); + test('when command sets deprecated noHelp then not displayed in helpInformation', () => { const program = new commander.Command(); program diff --git a/tests/command.helpCommand.test.js b/tests/command.helpCommand.test.js index a42048be8..8699dff30 100644 --- a/tests/command.helpCommand.test.js +++ b/tests/command.helpCommand.test.js @@ -21,6 +21,22 @@ describe('help command listed in helpInformation', () => { expect(helpInformation).toMatch(/help \[command\]/); }); + test('when program has subcommands and specify only unknown option then display help', () => { + const program = new commander.Command(); + program + .configureHelp({ formatHelp: () => '' }) + .exitOverride() + .allowUnknownOption() + .command('foo'); + let caughtErr; + try { + program.parse(['--unknown'], { from: 'user' }); + } catch (err) { + caughtErr = err; + } + expect(caughtErr.code).toEqual('commander.help'); + }); + test('when program has subcommands and suppress help command then no help command', () => { const program = new commander.Command(); program.addHelpCommand(false); @@ -67,6 +83,16 @@ describe('help command processed on correct command', () => { }).toThrow('program'); }); + test('when "program help unknown" then program', () => { + const program = new commander.Command(); + program.exitOverride(); + program.command('sub1'); + program.exitOverride(() => { throw new Error('program'); }); + expect(() => { + program.parse('node test.js help unknown'.split(' ')); + }).toThrow('program'); + }); + test('when "program help sub1" then sub1', () => { const program = new commander.Command(); program.exitOverride(); diff --git a/tests/command.parse.test.js b/tests/command.parse.test.js index 75845198e..796c3b67d 100644 --- a/tests/command.parse.test.js +++ b/tests/command.parse.test.js @@ -4,7 +4,7 @@ const commander = require('../'); // https://github.com/electron/electron/issues/4690#issuecomment-217435222 // https://www.electronjs.org/docs/api/process#processdefaultapp-readonly -describe('.parse() user args', () => { +describe('.parse() args from', () => { test('when no args then use process.argv and app/script/args', () => { const program = new commander.Command(); const hold = process.argv; @@ -14,7 +14,16 @@ describe('.parse() user args', () => { expect(program.args).toEqual(['user']); }); - // implicit also supports detecting electron but more implementation knowledge required than useful to test + test('when no args and electron properties and not default app then use process.argv and app/args', () => { + const program = new commander.Command(); + const holdArgv = process.argv; + process.versions.electron = '1.2.3'; + process.argv = 'node user'.split(' '); + program.parse(); + delete process.versions.electron; + process.argv = holdArgv; + expect(program.args).toEqual(['user']); + }); test('when args then app/script/args', () => { const program = new commander.Command(); @@ -51,6 +60,13 @@ describe('.parse() user args', () => { program.parse('user'.split(' '), { from: 'user' }); expect(program.args).toEqual(['user']); }); + + test('when args from "silly" then throw', () => { + const program = new commander.Command(); + expect(() => { + program.parse(['node', 'script.js'], { from: 'silly' }); + }).toThrow(); + }); }); describe('return type', () => { @@ -72,3 +88,11 @@ describe('return type', () => { expect(result).toBe(program); }); }); + +// Easy mistake to make when writing unit tests +test('when parse strings instead of array then throw', () => { + const program = new commander.Command(); + expect(() => { + program.parse('node', 'test'); + }).toThrow(); +}); diff --git a/tests/command.unknownCommand.test.js b/tests/command.unknownCommand.test.js index 83bf87dc8..844c8455a 100644 --- a/tests/command.unknownCommand.test.js +++ b/tests/command.unknownCommand.test.js @@ -62,4 +62,20 @@ describe('unknownOption', () => { } expect(caughtErr.code).toBe('commander.unknownCommand'); }); + + test('when unknown subcommand then help suggestion includes command path', () => { + const program = new commander.Command(); + program + .exitOverride() + .command('sub') + .command('subsub'); + let caughtErr; + try { + program.parse('node test.js sub unknown'.split(' ')); + } catch (err) { + caughtErr = err; + } + expect(caughtErr.code).toBe('commander.unknownCommand'); + expect(writeErrorSpy.mock.calls[0][0]).toMatch('test sub'); + }); }); diff --git a/tests/commander.configureCommand.test.js b/tests/commander.configureCommand.test.js index 3595d820a..5388a1866 100644 --- a/tests/commander.configureCommand.test.js +++ b/tests/commander.configureCommand.test.js @@ -76,3 +76,11 @@ test('when storeOptionsAsProperties(false) then opts+command passed to action', program.parse(['node', 'test', 'value']); expect(callback).toHaveBeenCalledWith('value', program.opts(), program); }); + +test('when storeOptionsAsProperties() after adding option then throw', () => { + const program = new commander.Command(); + program.option('--port ', 'port number', '80'); + expect(() => { + program.storeOptionsAsProperties(false); + }).toThrow(); +}); diff --git a/tests/incrementNodeInspectorPort.test.js b/tests/incrementNodeInspectorPort.test.js new file mode 100644 index 000000000..5f5ac76a5 --- /dev/null +++ b/tests/incrementNodeInspectorPort.test.js @@ -0,0 +1,135 @@ +const childProcess = require('child_process'); +const path = require('path'); +const commander = require('../'); + +describe('incrementNodeInspectorPort', () => { + let spawnSpy; + let signalSpy; + const oldExecArgv = process.execArgv; + + beforeAll(() => { + spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => { + return { + on: () => {} + }; + }); + signalSpy = jest.spyOn(process, 'on').mockImplementation(() => {}); + }); + + afterEach(() => { + spawnSpy.mockClear(); + }); + + afterAll(() => { + spawnSpy.mockRestore(); + signalSpy.mockRestore(); + process.execArgv = oldExecArgv; + }); + + function makeProgram() { + const program = new commander.Command(); + const fileWhichExists = path.join(__dirname, './fixtures/pm-cache.js'); + program.command('cache', 'stand-alone command', { executableFile: fileWhichExists }); + return program; + } + + function extractMockExecArgs(mock) { + return mock.mock.calls[0][1].slice(0, -1); + } + + test('when --inspect then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect=127.0.0.1:9230']); + }); + + test('when --inspect=100 then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect=100']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect=127.0.0.1:101']); + }); + + test('when --inspect=1.2.3.4:100 then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect=1.2.3.4:100']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect=1.2.3.4:101']); + }); + + test('when --inspect=1.2.3.4 then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect=1.2.3.4']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect=1.2.3.4:9230']); + }); + + test('when --inspect-brk then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect-brk']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect-brk=127.0.0.1:9230']); + }); + + test('when --inspect-brk=100 then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect-brk=100']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect-brk=127.0.0.1:101']); + }); + + test('when --inspect-brk=1.2.3.4 then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect-brk=1.2.3.4']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect-brk=1.2.3.4:9230']); + }); + + test('when --inspect-brk=1.2.3.4:100 then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect-brk=1.2.3.4:100']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect-brk=1.2.3.4:101']); + }); + + test('when --inspect-port=100 then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect-port=100']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect-port=127.0.0.1:101']); + }); + + test('when --inspect-port=1.2.3.4:100 then bump port', () => { + const program = makeProgram(); + process.execArgv = ['--inspect-port=1.2.3.4:100']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect-port=1.2.3.4:101']); + }); + + test('when --inspect-unexpected then unchanged', () => { + const program = makeProgram(); + process.execArgv = ['--inspect-unexpected']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--inspect-unexpected']); + }); + + test('when --frozen-intrinsics then unchanged', () => { + const program = makeProgram(); + process.execArgv = ['--frozen-intrinsics ']; + program.parse(['node', 'test', 'cache']); + const execArgs = extractMockExecArgs(spawnSpy); + expect(execArgs).toEqual(['--frozen-intrinsics ']); + }); +});