Skip to content

Commit d8faba2

Browse files
authored
Increase test coverage, including incrementNodeInspectorPort (#1428)
* Add tests for incrementNodeInspectorPort * Fill out incrementNodeInspectorPort * Remove unused code from optionMissingArgument * Add tests for exit and custom throw * Error if call storeOptionsAsProperties with options already stored * Add test for bad params to parse * Error if parse "from" is unsupported * Can set alias after adding external command * Add test for invalid position to addHelpText * Clarify throw rather than display error * Add special case which displays help * Add test for incorrect return type on deprecated callback * Add test for implicit electron args * Add test for help requested on unknown subcommand * Add test for help suggestion including command path
1 parent 5173665 commit d8faba2

10 files changed

+263
-13
lines changed

index.js

+6-11
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
const EventEmitter = require('events').EventEmitter;
6-
const spawn = require('child_process').spawn;
6+
const childProcess = require('child_process');
77
const path = require('path');
88
const fs = require('fs');
99

@@ -1335,15 +1335,15 @@ class Command extends EventEmitter {
13351335
// add executable arguments to spawn
13361336
args = incrementNodeInspectorPort(process.execArgv).concat(args);
13371337

1338-
proc = spawn(process.argv[0], args, { stdio: 'inherit' });
1338+
proc = childProcess.spawn(process.argv[0], args, { stdio: 'inherit' });
13391339
} else {
1340-
proc = spawn(bin, args, { stdio: 'inherit' });
1340+
proc = childProcess.spawn(bin, args, { stdio: 'inherit' });
13411341
}
13421342
} else {
13431343
args.unshift(bin);
13441344
// add executable arguments to spawn
13451345
args = incrementNodeInspectorPort(process.execArgv).concat(args);
1346-
proc = spawn(process.execPath, args, { stdio: 'inherit' });
1346+
proc = childProcess.spawn(process.execPath, args, { stdio: 'inherit' });
13471347
}
13481348

13491349
const signals = ['SIGUSR1', 'SIGUSR2', 'SIGTERM', 'SIGINT', 'SIGHUP'];
@@ -1672,13 +1672,8 @@ class Command extends EventEmitter {
16721672
* @api private
16731673
*/
16741674

1675-
optionMissingArgument(option, flag) {
1676-
let message;
1677-
if (flag) {
1678-
message = `error: option '${option.flags}' argument missing, got '${flag}'`;
1679-
} else {
1680-
message = `error: option '${option.flags}' argument missing`;
1681-
}
1675+
optionMissingArgument(option) {
1676+
const message = `error: option '${option.flags}' argument missing`;
16821677
this._displayError(1, 'commander.optionMissingArgument', message);
16831678
};
16841679

tests/command.addHelpText.test.js

+7
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ describe('program calls to addHelpText', () => {
7777
expect(writeSpy).toHaveBeenNthCalledWith(4, 'after\n');
7878
expect(writeSpy).toHaveBeenNthCalledWith(5, 'afterAll\n');
7979
});
80+
81+
test('when "silly" position then throw', () => {
82+
const program = new commander.Command();
83+
expect(() => {
84+
program.addHelpText('silly', 'text');
85+
}).toThrow();
86+
});
8087
});
8188

8289
describe('program and subcommand calls to addHelpText', () => {

tests/command.alias.test.js

+9
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,12 @@ test('when set aliases then can get aliases', () => {
8787
program.aliases(aliases);
8888
expect(program.aliases()).toEqual(aliases);
8989
});
90+
91+
test('when set alias on executable then can get alias', () => {
92+
const program = new commander.Command();
93+
const alias = 'abcde';
94+
program
95+
.command('external', 'external command')
96+
.alias(alias);
97+
expect(program.commands[0].alias()).toEqual(alias);
98+
});

tests/command.exitOverride.test.js

+23
Original file line numberDiff line numberDiff line change
@@ -278,3 +278,26 @@ describe('.exitOverride and error details', () => {
278278
expectCommanderError(caughtErr, 1, 'commander.invalidOptionArgument', "error: option '--colour <shade>' argument 'green' is invalid. NO");
279279
});
280280
});
281+
282+
test('when no override and error then exit(1)', () => {
283+
const exitSpy = jest.spyOn(process, 'exit').mockImplementation(() => { });
284+
const program = new commander.Command();
285+
program.configureOutput({ outputError: () => {} });
286+
program.parse(['--unknownOption'], { from: 'user' });
287+
expect(exitSpy).toHaveBeenCalledWith(1);
288+
exitSpy.mockRestore();
289+
});
290+
291+
test('when custom processing throws custom error then throw custom error', () => {
292+
function justSayNo(value) {
293+
throw new Error('custom');
294+
}
295+
const program = new commander.Command();
296+
program
297+
.exitOverride()
298+
.option('-s, --shade <value>', 'specify shade', justSayNo);
299+
300+
expect(() => {
301+
program.parse(['--shade', 'green'], { from: 'user' });
302+
}).toThrow('custom');
303+
});

tests/command.help.test.js

+7
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,13 @@ test('when call outputHelp(cb) then display cb output', () => {
8888
writeSpy.mockClear();
8989
});
9090

91+
test('when call deprecated outputHelp(cb) with wrong callback return type then throw', () => {
92+
const program = new commander.Command();
93+
expect(() => {
94+
program.outputHelp((helpInformation) => 3);
95+
}).toThrow();
96+
});
97+
9198
test('when command sets deprecated noHelp then not displayed in helpInformation', () => {
9299
const program = new commander.Command();
93100
program

tests/command.helpCommand.test.js

+26
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,22 @@ describe('help command listed in helpInformation', () => {
2121
expect(helpInformation).toMatch(/help \[command\]/);
2222
});
2323

24+
test('when program has subcommands and specify only unknown option then display help', () => {
25+
const program = new commander.Command();
26+
program
27+
.configureHelp({ formatHelp: () => '' })
28+
.exitOverride()
29+
.allowUnknownOption()
30+
.command('foo');
31+
let caughtErr;
32+
try {
33+
program.parse(['--unknown'], { from: 'user' });
34+
} catch (err) {
35+
caughtErr = err;
36+
}
37+
expect(caughtErr.code).toEqual('commander.help');
38+
});
39+
2440
test('when program has subcommands and suppress help command then no help command', () => {
2541
const program = new commander.Command();
2642
program.addHelpCommand(false);
@@ -67,6 +83,16 @@ describe('help command processed on correct command', () => {
6783
}).toThrow('program');
6884
});
6985

86+
test('when "program help unknown" then program', () => {
87+
const program = new commander.Command();
88+
program.exitOverride();
89+
program.command('sub1');
90+
program.exitOverride(() => { throw new Error('program'); });
91+
expect(() => {
92+
program.parse('node test.js help unknown'.split(' '));
93+
}).toThrow('program');
94+
});
95+
7096
test('when "program help sub1" then sub1', () => {
7197
const program = new commander.Command();
7298
program.exitOverride();

tests/command.parse.test.js

+26-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ const commander = require('../');
44
// https://github.com/electron/electron/issues/4690#issuecomment-217435222
55
// https://www.electronjs.org/docs/api/process#processdefaultapp-readonly
66

7-
describe('.parse() user args', () => {
7+
describe('.parse() args from', () => {
88
test('when no args then use process.argv and app/script/args', () => {
99
const program = new commander.Command();
1010
const hold = process.argv;
@@ -14,7 +14,16 @@ describe('.parse() user args', () => {
1414
expect(program.args).toEqual(['user']);
1515
});
1616

17-
// implicit also supports detecting electron but more implementation knowledge required than useful to test
17+
test('when no args and electron properties and not default app then use process.argv and app/args', () => {
18+
const program = new commander.Command();
19+
const holdArgv = process.argv;
20+
process.versions.electron = '1.2.3';
21+
process.argv = 'node user'.split(' ');
22+
program.parse();
23+
delete process.versions.electron;
24+
process.argv = holdArgv;
25+
expect(program.args).toEqual(['user']);
26+
});
1827

1928
test('when args then app/script/args', () => {
2029
const program = new commander.Command();
@@ -51,6 +60,13 @@ describe('.parse() user args', () => {
5160
program.parse('user'.split(' '), { from: 'user' });
5261
expect(program.args).toEqual(['user']);
5362
});
63+
64+
test('when args from "silly" then throw', () => {
65+
const program = new commander.Command();
66+
expect(() => {
67+
program.parse(['node', 'script.js'], { from: 'silly' });
68+
}).toThrow();
69+
});
5470
});
5571

5672
describe('return type', () => {
@@ -72,3 +88,11 @@ describe('return type', () => {
7288
expect(result).toBe(program);
7389
});
7490
});
91+
92+
// Easy mistake to make when writing unit tests
93+
test('when parse strings instead of array then throw', () => {
94+
const program = new commander.Command();
95+
expect(() => {
96+
program.parse('node', 'test');
97+
}).toThrow();
98+
});

tests/command.unknownCommand.test.js

+16
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,20 @@ describe('unknownOption', () => {
6262
}
6363
expect(caughtErr.code).toBe('commander.unknownCommand');
6464
});
65+
66+
test('when unknown subcommand then help suggestion includes command path', () => {
67+
const program = new commander.Command();
68+
program
69+
.exitOverride()
70+
.command('sub')
71+
.command('subsub');
72+
let caughtErr;
73+
try {
74+
program.parse('node test.js sub unknown'.split(' '));
75+
} catch (err) {
76+
caughtErr = err;
77+
}
78+
expect(caughtErr.code).toBe('commander.unknownCommand');
79+
expect(writeErrorSpy.mock.calls[0][0]).toMatch('test sub');
80+
});
6581
});

tests/commander.configureCommand.test.js

+8
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,11 @@ test('when storeOptionsAsProperties(false) then opts+command passed to action',
7676
program.parse(['node', 'test', 'value']);
7777
expect(callback).toHaveBeenCalledWith('value', program.opts(), program);
7878
});
79+
80+
test('when storeOptionsAsProperties() after adding option then throw', () => {
81+
const program = new commander.Command();
82+
program.option('--port <number>', 'port number', '80');
83+
expect(() => {
84+
program.storeOptionsAsProperties(false);
85+
}).toThrow();
86+
});
+135
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
const childProcess = require('child_process');
2+
const path = require('path');
3+
const commander = require('../');
4+
5+
describe('incrementNodeInspectorPort', () => {
6+
let spawnSpy;
7+
let signalSpy;
8+
const oldExecArgv = process.execArgv;
9+
10+
beforeAll(() => {
11+
spawnSpy = jest.spyOn(childProcess, 'spawn').mockImplementation(() => {
12+
return {
13+
on: () => {}
14+
};
15+
});
16+
signalSpy = jest.spyOn(process, 'on').mockImplementation(() => {});
17+
});
18+
19+
afterEach(() => {
20+
spawnSpy.mockClear();
21+
});
22+
23+
afterAll(() => {
24+
spawnSpy.mockRestore();
25+
signalSpy.mockRestore();
26+
process.execArgv = oldExecArgv;
27+
});
28+
29+
function makeProgram() {
30+
const program = new commander.Command();
31+
const fileWhichExists = path.join(__dirname, './fixtures/pm-cache.js');
32+
program.command('cache', 'stand-alone command', { executableFile: fileWhichExists });
33+
return program;
34+
}
35+
36+
function extractMockExecArgs(mock) {
37+
return mock.mock.calls[0][1].slice(0, -1);
38+
}
39+
40+
test('when --inspect then bump port', () => {
41+
const program = makeProgram();
42+
process.execArgv = ['--inspect'];
43+
program.parse(['node', 'test', 'cache']);
44+
const execArgs = extractMockExecArgs(spawnSpy);
45+
expect(execArgs).toEqual(['--inspect=127.0.0.1:9230']);
46+
});
47+
48+
test('when --inspect=100 then bump port', () => {
49+
const program = makeProgram();
50+
process.execArgv = ['--inspect=100'];
51+
program.parse(['node', 'test', 'cache']);
52+
const execArgs = extractMockExecArgs(spawnSpy);
53+
expect(execArgs).toEqual(['--inspect=127.0.0.1:101']);
54+
});
55+
56+
test('when --inspect=1.2.3.4:100 then bump port', () => {
57+
const program = makeProgram();
58+
process.execArgv = ['--inspect=1.2.3.4:100'];
59+
program.parse(['node', 'test', 'cache']);
60+
const execArgs = extractMockExecArgs(spawnSpy);
61+
expect(execArgs).toEqual(['--inspect=1.2.3.4:101']);
62+
});
63+
64+
test('when --inspect=1.2.3.4 then bump port', () => {
65+
const program = makeProgram();
66+
process.execArgv = ['--inspect=1.2.3.4'];
67+
program.parse(['node', 'test', 'cache']);
68+
const execArgs = extractMockExecArgs(spawnSpy);
69+
expect(execArgs).toEqual(['--inspect=1.2.3.4:9230']);
70+
});
71+
72+
test('when --inspect-brk then bump port', () => {
73+
const program = makeProgram();
74+
process.execArgv = ['--inspect-brk'];
75+
program.parse(['node', 'test', 'cache']);
76+
const execArgs = extractMockExecArgs(spawnSpy);
77+
expect(execArgs).toEqual(['--inspect-brk=127.0.0.1:9230']);
78+
});
79+
80+
test('when --inspect-brk=100 then bump port', () => {
81+
const program = makeProgram();
82+
process.execArgv = ['--inspect-brk=100'];
83+
program.parse(['node', 'test', 'cache']);
84+
const execArgs = extractMockExecArgs(spawnSpy);
85+
expect(execArgs).toEqual(['--inspect-brk=127.0.0.1:101']);
86+
});
87+
88+
test('when --inspect-brk=1.2.3.4 then bump port', () => {
89+
const program = makeProgram();
90+
process.execArgv = ['--inspect-brk=1.2.3.4'];
91+
program.parse(['node', 'test', 'cache']);
92+
const execArgs = extractMockExecArgs(spawnSpy);
93+
expect(execArgs).toEqual(['--inspect-brk=1.2.3.4:9230']);
94+
});
95+
96+
test('when --inspect-brk=1.2.3.4:100 then bump port', () => {
97+
const program = makeProgram();
98+
process.execArgv = ['--inspect-brk=1.2.3.4:100'];
99+
program.parse(['node', 'test', 'cache']);
100+
const execArgs = extractMockExecArgs(spawnSpy);
101+
expect(execArgs).toEqual(['--inspect-brk=1.2.3.4:101']);
102+
});
103+
104+
test('when --inspect-port=100 then bump port', () => {
105+
const program = makeProgram();
106+
process.execArgv = ['--inspect-port=100'];
107+
program.parse(['node', 'test', 'cache']);
108+
const execArgs = extractMockExecArgs(spawnSpy);
109+
expect(execArgs).toEqual(['--inspect-port=127.0.0.1:101']);
110+
});
111+
112+
test('when --inspect-port=1.2.3.4:100 then bump port', () => {
113+
const program = makeProgram();
114+
process.execArgv = ['--inspect-port=1.2.3.4:100'];
115+
program.parse(['node', 'test', 'cache']);
116+
const execArgs = extractMockExecArgs(spawnSpy);
117+
expect(execArgs).toEqual(['--inspect-port=1.2.3.4:101']);
118+
});
119+
120+
test('when --inspect-unexpected then unchanged', () => {
121+
const program = makeProgram();
122+
process.execArgv = ['--inspect-unexpected'];
123+
program.parse(['node', 'test', 'cache']);
124+
const execArgs = extractMockExecArgs(spawnSpy);
125+
expect(execArgs).toEqual(['--inspect-unexpected']);
126+
});
127+
128+
test('when --frozen-intrinsics then unchanged', () => {
129+
const program = makeProgram();
130+
process.execArgv = ['--frozen-intrinsics '];
131+
program.parse(['node', 'test', 'cache']);
132+
const execArgs = extractMockExecArgs(spawnSpy);
133+
expect(execArgs).toEqual(['--frozen-intrinsics ']);
134+
});
135+
});

0 commit comments

Comments
 (0)