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

Support for --tags to include/exclude tests based on some optional tagging #1445

Closed
wants to merge 1 commit into from
Closed
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
10 changes: 10 additions & 0 deletions bin/_mocha
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ program
.option('--trace', 'trace function calls')
.option('--trace-deprecation', 'show stack traces on deprecations')
.option('--watch-extensions <ext>,...', 'additional extensions to monitor with --watch', list, [])
.option('--tags <tags>', 'only include tests with the given tags', list, [])

Choose a reason for hiding this comment

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

This should probably mention the a+b and a,b syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This version doesn't actually have the + syntax, that was just a suggestion in the gh-pages branch. But 👍 to the ,, I'll amend that.

.option('--skip-tags <tags>', 'exclude all tests with the given tags', list, [])

program.name = 'mocha';

Expand Down Expand Up @@ -255,6 +257,14 @@ if (program.grep) mocha.grep(new RegExp(program.grep));

if (program.invert) mocha.invert();

// --tags

if (program.tags) mocha.tags(program.tags);

// --skip-tags

if (program.skipTags) mocha.skipTags(program.skipTags);

// --check-leaks

if (program.checkLeaks) mocha.checkLeaks();
Expand Down
4 changes: 3 additions & 1 deletion lib/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ module.exports = Context;
* @api private
*/

function Context(){}
function Context(){
this._tags = [];
}

/**
* Set or get the context `Runnable` to `runnable`.
Expand Down
36 changes: 36 additions & 0 deletions lib/filter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
var utils = require('./utils');

module.exports = Filter;

function Filter() {
this.grep = /.*/;
this.invertGrep = false;
this.tags = [];
this.skipTags = [];
}

Filter.prototype.count = function(suite) {
var self = this;
var total = 0;
suite.eachTest(function(test){
if (self.shouldRun(test)) total++;
});
return total;
};

Filter.prototype.shouldRun = function(test) {
// --grep
var grepMatch = this.grep.test(test.fullTitle());
if (this.invertGrep) grepMatch = !grepMatch;
// --tags --skip-tags
var include = (!this.tags.length) || matchTags(test.ctx._tags, this.tags);
var exclude = this.skipTags.length && matchTags(test.ctx._tags, this.skipTags);
// final decision
return grepMatch && include && !exclude;
Copy link
Contributor

Choose a reason for hiding this comment

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

We could early return once we calculate a condition that fails so that we skip unnecessary calculus.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's true. I thought grepMatch && include && !exclude was simple and easy to understand, and chose that over performance. Do you think performance matters a lot here?

Copy link
Contributor

@dasilvacontin dasilvacontin Oct 23, 2016

Choose a reason for hiding this comment

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

From my testing, the performance gain is negligible, and this reads easier. LGTM, good call.

(like 12ms per 100 000 tests)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sharing so that someone points out my test is bad or wrong:

const filterTags = ['ci']
const testTags = ['slow', 'vendor', 'thing', 'ci']

function matchTags (actualTags, against) {
  return against.some((tag) => {
    return actualTags.indexOf(tag) !== -1
  })
}

console.time('shouldRun')

let tests = 100 * 1000
while (--tests) matchTags(testTags, filterTags)

console.timeEnd('shouldRun')

};

function matchTags(actualTags, against) {
return utils.some(against, function(tag) {
return utils.indexOf(actualTags, tag) !== -1;
});
}
24 changes: 24 additions & 0 deletions lib/mocha.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ function image(name) {
* - `slow` milliseconds to wait before considering a test slow
* - `ignoreLeaks` ignore global leaks
* - `grep` string or regexp to filter tests with
* - `tags` array of tags to be included when filtering
* - `skipTags` array of tags to be excluded when filtering
*
* @param {Object} options
* @api public
Expand Down Expand Up @@ -238,6 +240,26 @@ Mocha.prototype.invert = function(){
return this;
};

/**
* Set a list of tags to include
* @returns {Mocha}
* @api public
*/
Mocha.prototype.tags = function(tags) {
this.options.tags = tags;
return this;
};

/**
* Set a list of tags to exclude
* @returns {Mocha}
* @api public
*/
Mocha.prototype.skipTags = function(tags) {
this.options.skipTags = tags;
return this;
};

/**
* Ignore global leaks.
*
Expand Down Expand Up @@ -399,6 +421,8 @@ Mocha.prototype.run = function(fn){
runner.ignoreLeaks = false !== options.ignoreLeaks;
runner.asyncOnly = options.asyncOnly;
if (options.grep) runner.grep(options.grep, options.invert);
if (options.tags) runner.tags(options.tags);
if (options.skipTags) runner.skipTags(options.skipTags);
if (options.globals) runner.globals(options.globals);
if (options.growl) this._growl(runner, reporter);
exports.reporters.Base.useColors = options.useColors;
Expand Down
3 changes: 1 addition & 2 deletions lib/reporters/tap.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ function TAP(runner) {
, failures = 0;

runner.on('start', function(){
var total = runner.grepTotal(runner.suite);
console.log('%d..%d', 1, total);
console.log('%d..%d', 1, runner.total);
});

runner.on('test end', function(){
Expand Down
55 changes: 31 additions & 24 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
var EventEmitter = require('events').EventEmitter
, debug = require('debug')('mocha:runner')
, Test = require('./test')
, Filter = require('./filter')
, utils = require('./utils')
, filter = utils.filter
, keys = utils.keys;
Expand Down Expand Up @@ -54,12 +55,12 @@ function Runner(suite) {
var self = this;
this._globals = [];
this._abort = false;
this._filter = new Filter();
this.suite = suite;
this.total = suite.total();
this.failures = 0;
this.on('test end', function(test){ self.checkGlobals(test); });
this.on('hook end', function(hook){ self.checkGlobals(hook); });
this.grep(/.*/);
this.globals(this.globalProps().concat(extraGlobals()));
}

Expand Down Expand Up @@ -88,34 +89,42 @@ Runner.prototype.__proto__ = EventEmitter.prototype;
* @api public
*/

Runner.prototype.grep = function(re, invert){
debug('grep %s', re);
this._grep = re;
this._invert = invert;
this.total = this.grepTotal(this.suite);
Runner.prototype.grep = function(re, invert) {
debug('grep %s (%s)', re, invert);
this._filter.grep = re;
this._filter.invertGrep = invert;
this.total = this._filter.count(this.suite);
return this;
};

/**
* Returns the number of tests matching the grep search for the
* given suite.
* Only run tests matching the given tags
*
* @param {Suite} suite
* @return {Number}
* @param {Array} tags
* @return {Runner} for chaining
* @api public
*/

Runner.prototype.grepTotal = function(suite) {
var self = this;
var total = 0;
Runner.prototype.tags = function(tags){
debug('include tags: %s', tags);
this._filter.tags = tags;
this.total = this._filter.count(this.suite);
return this;
};

suite.eachTest(function(test){
var match = self._grep.test(test.fullTitle());
if (self._invert) match = !match;
if (match) total++;
});
/**
* Exclude tests matching the given tags
*
* @param {Array} tags
* @return {Runner} for chaining
* @api public
*/

return total;
Runner.prototype.skipTags = function(tags){
debug('skip tags: %s', tags);
this._filter.skipTags = tags;
this.total = this._filter.count(this.suite);
Copy link
Contributor

@dasilvacontin dasilvacontin Sep 28, 2016

Choose a reason for hiding this comment

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

Seeing this line quite a few times – might be worth refactoring into Runner#updateTotal.

total is not very descriptive imo, but we can leave renaming that to an additional issue/PR and move on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point thanks, I'll change extract & for a more descriptive name.

return this;
};

/**
Expand Down Expand Up @@ -430,10 +439,8 @@ Runner.prototype.runTests = function(suite, fn){
// all done
if (!test) return fn();

// grep
var match = self._grep.test(test.fullTitle());
if (self._invert) match = !match;
if (!match) return next();
// filter
if (!self._filter.shouldRun(test)) return next();

// pending
if (test.pending) {
Expand Down Expand Up @@ -480,7 +487,7 @@ Runner.prototype.runTests = function(suite, fn){
*/

Runner.prototype.runSuite = function(suite, fn){
var total = this.grepTotal(suite)
var total = this._filter.count(suite)
, self = this
, i = 0;

Expand Down
18 changes: 18 additions & 0 deletions lib/suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,24 @@ Suite.prototype.bail = function(bail){
return this;
};

/*
* Sets tags on the suite's context,
* inheriting any parent tags
*
* @param {String...} tags
* @return {Suite} for chaining
* @api private
*/

Suite.prototype.tag = function(tags) {
if (this.parent) {
this.ctx._tags = Array.prototype.concat.apply(this.parent.ctx._tags, arguments);
} else {
this.ctx._tags = Array.prototype.slice.apply(arguments);
}
return this;
};

/**
* Run `fn(test[, done])` before running tests.
*
Expand Down
15 changes: 15 additions & 0 deletions lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,21 @@ exports.filter = function(arr, fn){
return ret;
};

/**
* Array#some
*
* @param {Array} array
* @param {Function} fn
* @api private
*/

exports.some = function(arr, fn) {
for (var i = 0, l = arr.length; i < l; ++i) {
if (fn(arr[i], i, arr)) return true;
}
return false;
};

/**
* Object.keys (<=IE8)
*
Expand Down
Loading