Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix timeout/slow string values and duplicate arguments #3831

Merged
merged 2 commits into from
May 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions lib/cli/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)}),
{}
)
);

/**
Expand Down
14 changes: 12 additions & 2 deletions lib/cli/run-option-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,18 @@ exports.types = {
'sort',
'watch'
],
number: ['retries', 'slow', 'timeout'],
string: ['fgrep', 'grep', 'package', 'reporter', 'ui']
number: ['retries'],
string: [
'config',
'fgrep',
'grep',
'opts',
'package',
'reporter',
'ui',
'slow',
'timeout'
]
};

/**
Expand Down
36 changes: 36 additions & 0 deletions test/integration/duplicate-arguments.spec.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
});
11 changes: 11 additions & 0 deletions test/integration/fixtures/options/slow-test.fixture.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
7 changes: 7 additions & 0 deletions test/integration/fixtures/passing-async.fixture.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

describe('a suite', function() {
it('should succeed in 50ms', function(done) {
setTimeout(done, 50);
});
});
6 changes: 6 additions & 0 deletions test/integration/fixtures/passing-sync.fixture.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use strict';

describe('a suite', function() {
it('should succeed', function() {
});
});
28 changes: 15 additions & 13 deletions test/integration/invalid-arguments.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Said this before and provided code for it.
This should be run for all arguments expecting a value, not just --ui.

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'}
);
});
});
});
52 changes: 52 additions & 0 deletions test/integration/options/timeout.spec.js
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
12 changes: 6 additions & 6 deletions test/node-unit/cli/options.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe('options', function() {
config: false,
opts: false,
package: false,
retries: 3
retries: '3'
})
);
});
Expand Down Expand Up @@ -202,7 +202,7 @@ describe('options', function() {
config: false,
opts: false,
package: false,
retries: 3
retries: '3'
}
)
);
Expand Down Expand Up @@ -427,7 +427,7 @@ describe('options', function() {
config: false,
opts: false,
package: false,
retries: 3
retries: '3'
})
);
});
Expand Down Expand Up @@ -476,7 +476,7 @@ describe('options', function() {
config: false,
opts: false,
package: false,
retries: 3
retries: '3'
})
);
});
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -692,7 +692,7 @@ describe('options', function() {
loadOptions('--timeout 500'),
'to have property',
'timeout',
500
'500'
);
});
});
Expand Down