From 27cb94a66fcc1db89ac1b9ef99c2a75b1e16c2a3 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 18 Feb 2017 12:51:24 -0800 Subject: [PATCH 1/6] chore: add tests, get coverage back up to 100% --- test/command.js | 86 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/test/command.js b/test/command.js index dfa5c7060..b5fb37468 100644 --- a/test/command.js +++ b/test/command.js @@ -1286,4 +1286,90 @@ describe('Command', function () { }) }) }) + + describe('default commands', function () { + it('executes default command if no positional arguments given', function (done) { + yargs('--foo bar') + .command('*', 'default command', function () {}, function (argv) { + argv.foo.should.equal('bar') + return done() + }) + .argv + }) + + it('does not execute default command if another command is provided', function (done) { + yargs('run bcoe --foo bar') + .command('*', 'default command', function () {}, function (argv) {}) + .command('run ', 'run command', function () {}, function (argv) { + argv.name.should.equal('bcoe') + argv.foo.should.equal('bar') + return done() + }) + .argv + }) + + it('allows default command to be set as alias', function (done) { + yargs('bcoe --foo bar') + .command(['start ', '*'], 'start command', function () {}, function (argv) { + argv._.should.eql([]) + argv.name.should.equal('bcoe') + argv.foo.should.equal('bar') + return done() + }) + .argv + }) + + it('allows command to be run when alias is default command', function (done) { + yargs('start bcoe --foo bar') + .command(['start ', '*'], 'start command', function () {}, function (argv) { + argv._.should.eql(['start']) + argv.name.should.equal('bcoe') + argv.foo.should.equal('bar') + return done() + }) + .argv + }) + + it('the last default command set should take precedence', function (done) { + yargs('bcoe --foo bar') + .command(['first', '*'], 'override me', function () {}, function () {}) + .command(['second ', '*'], 'start command', function () {}, function (argv) { + argv._.should.eql([]) + argv.name.should.equal('bcoe') + argv.foo.should.equal('bar') + return done() + }) + .argv + }) + + describe('strict', function () { + it('executes default command when strict mode is enabled', function (done) { + yargs('--foo bar') + .command('*', 'default command', function () {}, function (argv) { + argv.foo.should.equal('bar') + return done() + }) + .option('foo', { + describe: 'a foo command' + }) + .strict() + .argv + }) + + it('allows default command aliases, when strict mode is enabled', function (done) { + yargs('bcoe --foo bar') + .command(['start ', '*'], 'start command', function () {}, function (argv) { + argv._.should.eql([]) + argv.name.should.equal('bcoe') + argv.foo.should.equal('bar') + return done() + }) + .strict() + .option('foo', { + describe: 'a foo command' + }) + .argv + }) + }) + }) }) From 6e0fd0ca722203f63164f796aa02dca5dcc12ac2 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sat, 25 Feb 2017 23:58:29 -0800 Subject: [PATCH 2/6] fix: switching invalid arguments to warn rather than throw --- lib/argsert.js | 87 +++++++++++++++++++++++-------------------- test/argsert.js | 24 ++++++++---- test/helpers/utils.js | 5 +++ 3 files changed, 68 insertions(+), 48 deletions(-) diff --git a/lib/argsert.js b/lib/argsert.js index c9a055a5d..013405fe7 100644 --- a/lib/argsert.js +++ b/lib/argsert.js @@ -1,54 +1,61 @@ const command = require('./command')() +const YError = require('./yerror') const positionName = ['first', 'second', 'third', 'fourth', 'fifth', 'sixth'] module.exports = function (expected, callerArguments, length) { - // preface the argument description with "cmd", so - // that we can run it through yargs' command parser. - var position = 0 - var parsed = {demanded: [], optional: []} - if (typeof expected === 'object') { - length = callerArguments - callerArguments = expected - } else { - parsed = command.parseCommand('cmd ' + expected) - } - const args = [].slice.call(callerArguments) + // TODO: should this eventually raise an exception. + try { + // preface the argument description with "cmd", so + // that we can run it through yargs' command parser. + var position = 0 + var parsed = {demanded: [], optional: []} + if (typeof expected === 'object') { + length = callerArguments + callerArguments = expected + } else { + parsed = command.parseCommand('cmd ' + expected) + } + const args = [].slice.call(callerArguments) - while (args.length && args[args.length - 1] === undefined) args.pop() - length = length || args.length + while (args.length && args[args.length - 1] === undefined) args.pop() + length = length || args.length - if (length < parsed.demanded.length) { - throw Error('Not enough arguments provided. Expected ' + parsed.demanded.length + - ' but received ' + args.length + '.') - } + if (length < parsed.demanded.length) { + throw new YError('Not enough arguments provided. Expected ' + parsed.demanded.length + + ' but received ' + args.length + '.') + } - const totalCommands = parsed.demanded.length + parsed.optional.length - if (length > totalCommands) { - throw Error('Too many arguments provided. Expected max ' + totalCommands + - ' but received ' + length + '.') - } + const totalCommands = parsed.demanded.length + parsed.optional.length + if (length > totalCommands) { + throw new YError('Too many arguments provided. Expected max ' + totalCommands + + ' but received ' + length + '.') + } - parsed.demanded.forEach(function (demanded) { - const arg = args.shift() - const observedType = guessType(arg) - const matchingTypes = demanded.cmd.filter(function (type) { - return type === observedType || type === '*' + parsed.demanded.forEach(function (demanded) { + const arg = args.shift() + const observedType = guessType(arg) + const matchingTypes = demanded.cmd.filter(function (type) { + return type === observedType || type === '*' + }) + if (matchingTypes.length === 0) argumentTypeError(observedType, demanded.cmd, position, false) + position += 1 }) - if (matchingTypes.length === 0) argumentTypeError(observedType, demanded.cmd, position, false) - position += 1 - }) - parsed.optional.forEach(function (optional) { - if (args.length === 0) return - const arg = args.shift() - const observedType = guessType(arg) - const matchingTypes = optional.cmd.filter(function (type) { - return type === observedType || type === '*' + parsed.optional.forEach(function (optional) { + if (args.length === 0) return + const arg = args.shift() + const observedType = guessType(arg) + const matchingTypes = optional.cmd.filter(function (type) { + return type === observedType || type === '*' + }) + if (matchingTypes.length === 0) argumentTypeError(observedType, optional.cmd, position, true) + position += 1 }) - if (matchingTypes.length === 0) argumentTypeError(observedType, optional.cmd, position, true) - position += 1 - }) + } catch (err) { + if (err instanceof YError) console.warn(err) + else throw err + } } function guessType (arg) { @@ -61,6 +68,6 @@ function guessType (arg) { } function argumentTypeError (observedType, allowedTypes, position, optional) { - throw Error('Invalid ' + (positionName[position] || 'manyith') + ' argument.' + + throw new YError('Invalid ' + (positionName[position] || 'manyith') + ' argument.' + ' Expected ' + allowedTypes.join(' or ') + ' but received ' + observedType + '.') } diff --git a/test/argsert.js b/test/argsert.js index 07262bc3b..6c37ef2b9 100644 --- a/test/argsert.js +++ b/test/argsert.js @@ -1,24 +1,32 @@ /* global describe, it */ const argsert = require('../lib/argsert') +const checkOutput = require('./helpers/utils').checkOutput const expect = require('chai').expect require('chai').should() describe('Argsert', function () { it('does not throw exception if optional argument is not provided', function () { - argsert('[object]', [].slice.call(arguments)) + var o = checkOutput(function () { + argsert('[object]', [].slice.call(arguments)) + }) + + o.warnings.length.should.equal(0) }) it('throws exception if wrong type is provided for optional argument', function () { - function foo (opts) { - argsert('[object|number]', [].slice.call(arguments)) - } - expect(function () { + var o = checkOutput(function () { + function foo (opts) { + argsert('[object|number]', [].slice.call(arguments)) + } + foo('hello') - }).to.throw(/Invalid first argument. Expected object or number but received string./) - }) + }) + o.warnings[0].should.match(/Invalid first argument. Expected object or number but received string./) + }) +/* it('does not throw exception if optional argument is valid', function () { function foo (opts) { argsert('[object]', [].slice.call(arguments)) @@ -94,5 +102,5 @@ describe('Argsert', function () { argsert('', [].slice.call(arguments)) } foo(null) - }) + })*/ }) diff --git a/test/helpers/utils.js b/test/helpers/utils.js index d896dae67..5a2ccc4f9 100644 --- a/test/helpers/utils.js +++ b/test/helpers/utils.js @@ -10,6 +10,7 @@ exports.checkOutput = function (f, argv, cb) { var _argv = process.argv var _error = console.error var _log = console.log + var _warn = console.warn process.exit = function () { exit = true } process.env = Hash.merge(process.env, { _: 'node' }) @@ -17,9 +18,11 @@ exports.checkOutput = function (f, argv, cb) { var errors = [] var logs = [] + var warnings = [] console.error = function (msg) { errors.push(msg) } console.log = function (msg) { logs.push(msg) } + console.warn = function (msg) { warnings.push(msg) } var result @@ -57,6 +60,7 @@ exports.checkOutput = function (f, argv, cb) { console.error = _error console.log = _log + console.warn = _warn } function done () { @@ -65,6 +69,7 @@ exports.checkOutput = function (f, argv, cb) { return { errors: errors, logs: logs, + warnings: warnings, exit: exit, result: result } From 4387f03d5df442380af4186d18fb42a97617a4b4 Mon Sep 17 00:00:00 2001 From: Daniel Zou Date: Sun, 26 Feb 2017 02:20:15 -0600 Subject: [PATCH 3/6] fix: converted argsert tests from throws to warnings --- test/argsert.js | 138 ++++++++++++++++++++++++++++++------------------ 1 file changed, 88 insertions(+), 50 deletions(-) diff --git a/test/argsert.js b/test/argsert.js index 6c37ef2b9..2c424ff10 100644 --- a/test/argsert.js +++ b/test/argsert.js @@ -7,7 +7,7 @@ const expect = require('chai').expect require('chai').should() describe('Argsert', function () { - it('does not throw exception if optional argument is not provided', function () { + it('does not warn if optional argument is not provided', function () { var o = checkOutput(function () { argsert('[object]', [].slice.call(arguments)) }) @@ -15,7 +15,7 @@ describe('Argsert', function () { o.warnings.length.should.equal(0) }) - it('throws exception if wrong type is provided for optional argument', function () { + it('warn if wrong type is provided for optional argument', function () { var o = checkOutput(function () { function foo (opts) { argsert('[object|number]', [].slice.call(arguments)) @@ -26,81 +26,119 @@ describe('Argsert', function () { o.warnings[0].should.match(/Invalid first argument. Expected object or number but received string./) }) -/* - it('does not throw exception if optional argument is valid', function () { - function foo (opts) { - argsert('[object]', [].slice.call(arguments)) - } - foo({foo: 'bar'}) + + it('does not warn if optional argument is valid', function () { + var o = checkOutput(function () { + function foo (opts) { + argsert('[object]', [].slice.call(arguments)) + } + + foo({foo: 'bar'}) + }) + + o.warnings.length.should.equal(0) }) - it('throws exception if required argument is not provided', function () { - expect(function () { + it('warns if required argument is not provided', function () { + var o = checkOutput(function () { argsert('', [].slice.call(arguments)) - }).to.throw(/Not enough arguments provided. Expected 1 but received 0./) + }) + + o.warnings[0].should.match(/Not enough arguments provided. Expected 1 but received 0./) }) - it('throws exception if required argument is of wrong type', function () { - function foo (opts) { - argsert('', [].slice.call(arguments)) - } - expect(function () { + it('warns if required argument is of wrong type', function () { + var o = checkOutput(function () { + function foo (opts) { + argsert('', [].slice.call(arguments)) + } + foo('bar') - }).to.throw(/Invalid first argument. Expected object but received string./) + }) + + o.warnings[0].should.match(/Invalid first argument. Expected object but received string./) }) it('supports a combination of required and optional arguments', function () { - function foo (opts) { - argsert(' [string|object]', [].slice.call(arguments)) - } - foo([], 'foo', {}) + var o = checkOutput(function () { + function foo (opts) { + argsert(' [string|object]', [].slice.call(arguments)) + } + + foo([], 'foo', {}) + }) + + o.warnings.length.should.equal(0) }) - it('throws an exception if too many arguments are provided', function () { - function foo (expected) { - argsert(' [batman]', [].slice.call(arguments)) - } - expect(function () { + it('warns if too many arguments are provided', function () { + var o = checkOutput(function () { + function foo (expected) { + argsert(' [batman]', [].slice.call(arguments)) + } + foo([], 33, 99) - }).to.throw(/Too many arguments provided. Expected max 2 but received 3./) + }) + + o.warnings[0].should.match(/Too many arguments provided. Expected max 2 but received 3./) }) it('configures function to accept 0 parameters, if only arguments object is provided', function () { - function foo (expected) { - argsert([].slice.call(arguments)) - } - expect(function () { + var o = checkOutput(function () { + function foo (expected) { + argsert([].slice.call(arguments)) + } + foo(99) - }).to.throw(/Too many arguments provided. Expected max 0 but received 1./) + }) + + o.warnings[0].should.match(/Too many arguments provided. Expected max 0 but received 1./) }) it('allows for any type if * is provided', function () { - function foo (opts) { - argsert('<*>', [].slice.call(arguments)) - } - foo('bar') + var o = checkOutput(function () { + function foo (opts) { + argsert('<*>', [].slice.call(arguments)) + } + + foo('bar') + }) + + o.warnings.length.should.equal(0) }) it('should ignore trailing undefined values', function () { - function foo (opts) { - argsert('<*>', [].slice.call(arguments)) - } - foo('bar', undefined, undefined) + var o = checkOutput(function () { + function foo (opts) { + argsert('<*>', [].slice.call(arguments)) + } + + foo('bar', undefined, undefined) + }) + + o.warnings.length.should.equal(0) }) it('should not ignore undefined values that are not trailing', function () { - function foo (opts) { - argsert('<*>', [].slice.call(arguments)) - } - expect(function () { + var o = checkOutput(function () { + function foo (opts) { + argsert('<*>', [].slice.call(arguments)) + } + foo('bar', undefined, undefined, 33) - }).to.throw(/Too many arguments provided. Expected max 1 but received 4./) + }) + + o.warnings[0].should.match(/Too many arguments provided. Expected max 1 but received 4./) }) it('supports null as special type', function () { - function foo (arg) { - argsert('', [].slice.call(arguments)) - } - foo(null) - })*/ + var o = checkOutput(function () { + function foo (arg) { + argsert('', [].slice.call(arguments)) + } + foo(null) + }) + + o.warnings.length.should.equal(0) + }) }) From 200f8e2beedc32bbe1c2ecbba0336b172096522e Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sun, 26 Feb 2017 00:31:37 -0800 Subject: [PATCH 4/6] fix: we were no longer using expect --- test/argsert.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/argsert.js b/test/argsert.js index 2c424ff10..9139d0d0e 100644 --- a/test/argsert.js +++ b/test/argsert.js @@ -2,7 +2,6 @@ const argsert = require('../lib/argsert') const checkOutput = require('./helpers/utils').checkOutput -const expect = require('chai').expect require('chai').should() From b54a1c28174a2d6ce487a73676a7293804878db5 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sun, 26 Feb 2017 00:56:08 -0800 Subject: [PATCH 5/6] chore: always console.warn() --- lib/argsert.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/argsert.js b/lib/argsert.js index 013405fe7..d3e72fce5 100644 --- a/lib/argsert.js +++ b/lib/argsert.js @@ -53,8 +53,7 @@ module.exports = function (expected, callerArguments, length) { position += 1 }) } catch (err) { - if (err instanceof YError) console.warn(err) - else throw err + console.warn(err.stack) } } From ee36008e9612af39d14ea2cf1cc7164547069161 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Sun, 26 Feb 2017 01:29:53 -0800 Subject: [PATCH 6/6] fix: merge-conflict, somehow ended up with two copies of default command tests --- test/command.js | 86 ------------------------------------------------- 1 file changed, 86 deletions(-) diff --git a/test/command.js b/test/command.js index b5fb37468..dfa5c7060 100644 --- a/test/command.js +++ b/test/command.js @@ -1286,90 +1286,4 @@ describe('Command', function () { }) }) }) - - describe('default commands', function () { - it('executes default command if no positional arguments given', function (done) { - yargs('--foo bar') - .command('*', 'default command', function () {}, function (argv) { - argv.foo.should.equal('bar') - return done() - }) - .argv - }) - - it('does not execute default command if another command is provided', function (done) { - yargs('run bcoe --foo bar') - .command('*', 'default command', function () {}, function (argv) {}) - .command('run ', 'run command', function () {}, function (argv) { - argv.name.should.equal('bcoe') - argv.foo.should.equal('bar') - return done() - }) - .argv - }) - - it('allows default command to be set as alias', function (done) { - yargs('bcoe --foo bar') - .command(['start ', '*'], 'start command', function () {}, function (argv) { - argv._.should.eql([]) - argv.name.should.equal('bcoe') - argv.foo.should.equal('bar') - return done() - }) - .argv - }) - - it('allows command to be run when alias is default command', function (done) { - yargs('start bcoe --foo bar') - .command(['start ', '*'], 'start command', function () {}, function (argv) { - argv._.should.eql(['start']) - argv.name.should.equal('bcoe') - argv.foo.should.equal('bar') - return done() - }) - .argv - }) - - it('the last default command set should take precedence', function (done) { - yargs('bcoe --foo bar') - .command(['first', '*'], 'override me', function () {}, function () {}) - .command(['second ', '*'], 'start command', function () {}, function (argv) { - argv._.should.eql([]) - argv.name.should.equal('bcoe') - argv.foo.should.equal('bar') - return done() - }) - .argv - }) - - describe('strict', function () { - it('executes default command when strict mode is enabled', function (done) { - yargs('--foo bar') - .command('*', 'default command', function () {}, function (argv) { - argv.foo.should.equal('bar') - return done() - }) - .option('foo', { - describe: 'a foo command' - }) - .strict() - .argv - }) - - it('allows default command aliases, when strict mode is enabled', function (done) { - yargs('bcoe --foo bar') - .command(['start ', '*'], 'start command', function () {}, function (argv) { - argv._.should.eql([]) - argv.name.should.equal('bcoe') - argv.foo.should.equal('bar') - return done() - }) - .strict() - .option('foo', { - describe: 'a foo command' - }) - .argv - }) - }) - }) })