From 1f5ec0b5271836c8f95c140324f5af55e28ee45e Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Tue, 12 Mar 2019 11:57:53 -0700 Subject: [PATCH 1/2] fix timeout/slow string values and duplicate arguments --- lib/cli/run-option-metadata.js | 4 +- lib/cli/run.js | 7 +++ test/integration/duplicate-arguments.spec.js | 36 +++++++++++++ .../fixtures/options/slow-test.fixture.js | 11 ++++ .../fixtures/passing-async.fixture.js | 7 +++ .../fixtures/passing-sync.fixture.js | 6 +++ test/integration/invalid-arguments.spec.js | 28 +++++----- test/integration/options/timeout.spec.js | 52 +++++++++++++++++++ test/node-unit/cli/options.spec.js | 4 +- 9 files changed, 138 insertions(+), 17 deletions(-) create mode 100644 test/integration/duplicate-arguments.spec.js create mode 100644 test/integration/fixtures/options/slow-test.fixture.js create mode 100644 test/integration/fixtures/passing-async.fixture.js create mode 100644 test/integration/fixtures/passing-sync.fixture.js create mode 100644 test/integration/options/timeout.spec.js diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js index e10e008fa6..f19bd57788 100644 --- a/lib/cli/run-option-metadata.js +++ b/lib/cli/run-option-metadata.js @@ -44,8 +44,8 @@ exports.types = { 'sort', 'watch' ], - number: ['retries', 'slow', 'timeout'], - string: ['fgrep', 'grep', 'package', 'reporter', 'ui'] + number: ['retries'], + string: ['fgrep', 'grep', 'package', 'reporter', 'ui', 'slow', 'timeout'] }; /** diff --git a/lib/cli/run.js b/lib/cli/run.js index da5ffd7b6a..0541ec6a83 100644 --- a/lib/cli/run.js +++ b/lib/cli/run.js @@ -258,6 +258,13 @@ exports.builder = yargs => } }); + types.boolean + .concat(types.string, types.number) + .filter(opt => Array.isArray(argv[opt])) + .forEach(opt => { + argv[opt] = argv[opt].pop(); + }); + // yargs.implies() isn't flexible enough to handle this if (argv.invert && !('fgrep' in argv || 'grep' in argv)) { throw createMissingArgumentError( diff --git a/test/integration/duplicate-arguments.spec.js b/test/integration/duplicate-arguments.spec.js new file mode 100644 index 0000000000..28fd225d9b --- /dev/null +++ b/test/integration/duplicate-arguments.spec.js @@ -0,0 +1,36 @@ +'use strict'; + +var runMocha = require('./helpers').runMocha; + +describe('when non-array argument is provided multiple times', function() { + describe('when the same argument name is used', function() { + it('should prefer the last value', function(done) { + runMocha( + 'passing-sync', + ['--no-async-only', '--async-only', '--no-async-only'], + function(err, result) { + if (err) { + return done(err); + } + expect(result, 'to have passed'); + done(); + } + ); + }); + }); + + describe('when a different argument name is used', function() { + it('should prefer the last value', function(done) { + runMocha('passing-async', ['--timeout', '100', '-t', '10'], function( + err, + result + ) { + if (err) { + return done(err); + } + expect(result, 'to have failed'); + done(); + }); + }); + }); +}); diff --git a/test/integration/fixtures/options/slow-test.fixture.js b/test/integration/fixtures/options/slow-test.fixture.js new file mode 100644 index 0000000000..f15cb6d9dd --- /dev/null +++ b/test/integration/fixtures/options/slow-test.fixture.js @@ -0,0 +1,11 @@ +'use strict'; + +describe('a suite', function() { + it('should succeed in 500ms', function(done) { + setTimeout(done, 500); + }); + + it('should succeed in 1.5s', function(done) { + setTimeout(done, 1500); + }); +}); diff --git a/test/integration/fixtures/passing-async.fixture.js b/test/integration/fixtures/passing-async.fixture.js new file mode 100644 index 0000000000..24db0b98d0 --- /dev/null +++ b/test/integration/fixtures/passing-async.fixture.js @@ -0,0 +1,7 @@ +'use strict'; + +describe('a suite', function() { + it('should succeed in 50ms', function(done) { + setTimeout(done, 50); + }); +}); diff --git a/test/integration/fixtures/passing-sync.fixture.js b/test/integration/fixtures/passing-sync.fixture.js new file mode 100644 index 0000000000..6087ebcbd4 --- /dev/null +++ b/test/integration/fixtures/passing-sync.fixture.js @@ -0,0 +1,6 @@ +'use strict'; + +describe('a suite', function() { + it('should succeed', function() { + }); +}); diff --git a/test/integration/invalid-arguments.spec.js b/test/integration/invalid-arguments.spec.js index 6c147bdb48..e1eabf20ae 100644 --- a/test/integration/invalid-arguments.spec.js +++ b/test/integration/invalid-arguments.spec.js @@ -3,18 +3,20 @@ var invokeMocha = require('./helpers').invokeMocha; describe('invalid arguments', function() { - it('should exit with failure if arguments are invalid', function(done) { - invokeMocha( - ['--ui'], - function(err, result) { - if (err) { - return done(err); - } - expect(result, 'to have failed'); - expect(result.output, 'to match', /not enough arguments/i); - done(); - }, - {stdio: 'pipe'} - ); + describe('when argument is missing required value', function() { + it('should exit with failure', function(done) { + invokeMocha( + ['--ui'], + function(err, result) { + if (err) { + return done(err); + } + expect(result, 'to have failed'); + expect(result.output, 'to match', /not enough arguments/i); + done(); + }, + {stdio: 'pipe'} + ); + }); }); }); diff --git a/test/integration/options/timeout.spec.js b/test/integration/options/timeout.spec.js new file mode 100644 index 0000000000..ac786da54d --- /dev/null +++ b/test/integration/options/timeout.spec.js @@ -0,0 +1,52 @@ +'use strict'; + +var helpers = require('../helpers'); +var runMochaJSON = helpers.runMochaJSON; + +describe('--timeout', function() { + it('should allow human-readable string value', function(done) { + runMochaJSON('options/slow-test', ['--timeout', '1s'], function(err, res) { + if (err) { + done(err); + return; + } + expect(res, 'to have failed') + .and('to have passed test count', 1) + .and('to have failed test count', 1); + done(); + }); + }); + + it('should allow numeric value', function(done) { + runMochaJSON('options/slow-test', ['--timeout', '1000'], function( + err, + res + ) { + if (err) { + done(err); + return; + } + expect(res, 'to have failed') + .and('to have passed test count', 1) + .and('to have failed test count', 1); + done(); + }); + }); + + it('should allow multiple values', function(done) { + var fixture = 'options/slow-test'; + runMochaJSON(fixture, ['--timeout', '2s', '--timeout', '1000'], function( + err, + res + ) { + if (err) { + done(err); + return; + } + expect(res, 'to have failed') + .and('to have passed test count', 1) + .and('to have failed test count', 1); + done(); + }); + }); +}); diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index 62a63f9fd6..2e1e4aa09d 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -629,7 +629,7 @@ describe('options', function() { findupSync }); - expect(loadOptions(), 'to satisfy', {timeout: 800, require: ['foo']}); + expect(loadOptions(), 'to satisfy', {timeout: '800', require: ['foo']}); }); it('should prioritize package.json over mocha.opts', function() { @@ -692,7 +692,7 @@ describe('options', function() { loadOptions('--timeout 500'), 'to have property', 'timeout', - 500 + '500' ); }); }); From 6c8b7e108c7c203bc8b0f201151536d85d4ad6e2 Mon Sep 17 00:00:00 2001 From: juergba Date: Thu, 2 May 2019 19:54:21 +0200 Subject: [PATCH 2/2] coerce function for boolean/string/number types --- lib/cli/options.js | 21 ++++++++++++++++----- lib/cli/run-option-metadata.js | 12 +++++++++++- lib/cli/run.js | 7 ------- test/node-unit/cli/options.spec.js | 8 ++++---- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/lib/cli/options.js b/lib/cli/options.js index 340fb01e86..fcc619a9b3 100644 --- a/lib/cli/options.js +++ b/lib/cli/options.js @@ -54,15 +54,26 @@ const configuration = Object.assign({}, YARGS_PARSER_CONFIG, { }); /** - * This is a really fancy way to ensure unique values for `array`-type - * options. + * This is a really fancy way to: + * - ensure unique values for `array`-type options + * - use its array's last element for `boolean`/`number`/`string`- options given multiple times * This is passed as the `coerce` option to `yargs-parser` * @private * @ignore */ -const coerceOpts = types.array.reduce( - (acc, arg) => Object.assign(acc, {[arg]: v => Array.from(new Set(list(v)))}), - {} +const coerceOpts = Object.assign( + types.array.reduce( + (acc, arg) => + Object.assign(acc, {[arg]: v => Array.from(new Set(list(v)))}), + {} + ), + types.boolean + .concat(types.string, types.number) + .reduce( + (acc, arg) => + Object.assign(acc, {[arg]: v => (Array.isArray(v) ? v.pop() : v)}), + {} + ) ); /** diff --git a/lib/cli/run-option-metadata.js b/lib/cli/run-option-metadata.js index f19bd57788..fbc4ea9072 100644 --- a/lib/cli/run-option-metadata.js +++ b/lib/cli/run-option-metadata.js @@ -45,7 +45,17 @@ exports.types = { 'watch' ], number: ['retries'], - string: ['fgrep', 'grep', 'package', 'reporter', 'ui', 'slow', 'timeout'] + string: [ + 'config', + 'fgrep', + 'grep', + 'opts', + 'package', + 'reporter', + 'ui', + 'slow', + 'timeout' + ] }; /** diff --git a/lib/cli/run.js b/lib/cli/run.js index 0541ec6a83..da5ffd7b6a 100644 --- a/lib/cli/run.js +++ b/lib/cli/run.js @@ -258,13 +258,6 @@ exports.builder = yargs => } }); - types.boolean - .concat(types.string, types.number) - .filter(opt => Array.isArray(argv[opt])) - .forEach(opt => { - argv[opt] = argv[opt].pop(); - }); - // yargs.implies() isn't flexible enough to handle this if (argv.invert && !('fgrep' in argv || 'grep' in argv)) { throw createMissingArgumentError( diff --git a/test/node-unit/cli/options.spec.js b/test/node-unit/cli/options.spec.js index 2e1e4aa09d..3c39cc7f09 100644 --- a/test/node-unit/cli/options.spec.js +++ b/test/node-unit/cli/options.spec.js @@ -86,7 +86,7 @@ describe('options', function() { config: false, opts: false, package: false, - retries: 3 + retries: '3' }) ); }); @@ -202,7 +202,7 @@ describe('options', function() { config: false, opts: false, package: false, - retries: 3 + retries: '3' } ) ); @@ -427,7 +427,7 @@ describe('options', function() { config: false, opts: false, package: false, - retries: 3 + retries: '3' }) ); }); @@ -476,7 +476,7 @@ describe('options', function() { config: false, opts: false, package: false, - retries: 3 + retries: '3' }) ); });