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

Refactor out usages of Suite#_onlyTests and Suite#_onlyTests (#3689) #3707

Merged
merged 6 commits into from
Feb 15, 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
2 changes: 1 addition & 1 deletion jsdoc.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"default": {
"includeDate": false,
"outputSourceFiles": true
},
},
Copy link
Contributor

Choose a reason for hiding this comment

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

How'd this get past prettier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea. Do you know whether my change is correct or the original was correct?

"monospaceLinks": false
}
}
4 changes: 2 additions & 2 deletions lib/interfaces/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ module.exports = function(suites, context, mocha) {
throw new Error('`.only` forbidden');
}

suite.parent._onlySuites = suite.parent._onlySuites.concat(suite);
suite.parent.appendOnlySuite(suite);
}
if (suite.pending) {
if (mocha.options.forbidPending && shouldBeTested(suite)) {
Expand Down Expand Up @@ -175,7 +175,7 @@ module.exports = function(suites, context, mocha) {
* @returns {*}
*/
only: function(mocha, test) {
test.parent._onlyTests = test.parent._onlyTests.concat(test);
test.parent.appendOnlyTest(test);
return test;
},

Expand Down
56 changes: 5 additions & 51 deletions lib/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,9 @@ Runner.prototype.runTest = function(fn) {
if (!test) {
return;
}
if (this.forbidOnly && hasOnly(this.parents().reverse()[0] || this.suite)) {

var suite = this.parents().reverse()[0] || this.suite;
if (this.forbidOnly && suite.hasOnly()) {
fn(new Error('`.only` forbidden'));
return;
}
Expand Down Expand Up @@ -869,8 +871,8 @@ Runner.prototype.run = function(fn) {

function start() {
// If there is an `only` filter
if (hasOnly(rootSuite)) {
filterOnly(rootSuite);
if (rootSuite.hasOnly()) {
rootSuite.filterOnly();
}
self.started = true;
if (self._delay) {
Expand Down Expand Up @@ -931,54 +933,6 @@ Runner.prototype.abort = function() {
return this;
};

/**
* Filter suites based on `isOnly` logic.
*
* @param {Array} suite
* @returns {Boolean}
* @private
*/
function filterOnly(suite) {
if (suite._onlyTests.length) {
// If the suite contains `only` tests, run those and ignore any nested suites.
suite.tests = suite._onlyTests;
suite.suites = [];
} else {
// Otherwise, do not run any of the tests in this suite.
suite.tests = [];
suite._onlySuites.forEach(function(onlySuite) {
// If there are other `only` tests/suites nested in the current `only` suite, then filter that `only` suite.
// Otherwise, all of the tests on this `only` suite should be run, so don't filter it.
if (hasOnly(onlySuite)) {
filterOnly(onlySuite);
}
});
// Run the `only` suites, as well as any other suites that have `only` tests/suites as descendants.
suite.suites = suite.suites.filter(function(childSuite) {
return (
suite._onlySuites.indexOf(childSuite) !== -1 || filterOnly(childSuite)
);
});
}
// Keep the suite only if there is something to run
return suite.tests.length || suite.suites.length;
}

/**
* Determines whether a suite has an `only` test or suite as a descendant.
*
* @param {Array} suite
* @returns {Boolean}
* @private
*/
function hasOnly(suite) {
return (
suite._onlyTests.length ||
suite._onlySuites.length ||
suite.suites.some(hasOnly)
);
}

/**
* Filter leaks with the given globals flagged as `ok`.
*
Expand Down
67 changes: 67 additions & 0 deletions lib/suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,73 @@ Suite.prototype.run = function run() {
}
};

/**
* Determines whether a suite has an `only` test or suite as a descendant.
*
* @private
* @returns {Boolean}
*/
Suite.prototype.hasOnly = function hasOnly() {
boneskull marked this conversation as resolved.
Show resolved Hide resolved
return (
this._onlyTests.length > 0 ||
this._onlySuites.length > 0 ||
this.suites.some(function(suite) {
return suite.hasOnly();
})
);
};

/**
* Filter suites based on `isOnly` logic.
*
* @private
* @returns {Boolean}
*/
Suite.prototype.filterOnly = function filterOnly() {
boneskull marked this conversation as resolved.
Show resolved Hide resolved
if (this._onlyTests.length) {
// If the suite contains `only` tests, run those and ignore any nested suites.
this.tests = this._onlyTests;
this.suites = [];
} else {
// Otherwise, do not run any of the tests in this suite.
this.tests = [];
this._onlySuites.forEach(function(onlySuite) {
// If there are other `only` tests/suites nested in the current `only` suite, then filter that `only` suite.
// Otherwise, all of the tests on this `only` suite should be run, so don't filter it.
if (onlySuite.hasOnly()) {
onlySuite.filterOnly();
}
});
// Run the `only` suites, as well as any other suites that have `only` tests/suites as descendants.
var onlySuites = this._onlySuites;
this.suites = this.suites.filter(function(childSuite) {
return onlySuites.indexOf(childSuite) !== -1 || childSuite.filterOnly();
});
}
// Keep the suite only if there is something to run
return this.tests.length > 0 || this.suites.length > 0;
};

/**
* Adds a suite to the list of subsuites marked `only`.
*
* @private
* @param {Suite} suite
*/
Suite.prototype.appendOnlySuite = function(suite) {
boneskull marked this conversation as resolved.
Show resolved Hide resolved
this._onlySuites.push(suite);
};

/**
* Adds a test to the list of tests marked `only`.
*
* @private
* @param {Test} test
*/
Suite.prototype.appendOnlyTest = function(test) {
boneskull marked this conversation as resolved.
Show resolved Hide resolved
this._onlyTests.push(test);
};

/**
* Returns the array of hooks by hook name; see `HOOK_TYPE_*` constants.
* @private
Expand Down
89 changes: 89 additions & 0 deletions test/unit/suite.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,95 @@ describe('Suite', function() {
expect(suite.timeout(), 'to be', 100);
});
});

describe('hasOnly()', function() {
it('should return true if a test has `only`', function() {
var suite = new Suite('foo');
var test = new Test('bar');

suite.appendOnlyTest(test);

expect(suite.hasOnly(), 'to be', true);
});

it('should return true if a suite has `only`', function() {
var suite = new Suite('foo');
var nested = new Suite('bar');

suite.appendOnlySuite(nested);

expect(suite.hasOnly(), 'to be', true);
});

it('should return true if nested suite has `only`', function() {
var suite = new Suite('foo');
var nested = new Suite('bar');
var test = new Test('baz');

nested.appendOnlyTest(test);
// `nested` has a `only` test, but `suite` doesn't know about it
suite.suites.push(nested);

expect(suite.hasOnly(), 'to be', true);
});

it('should return false if no suite or test is marked `only`', function() {
var suite = new Suite('foo');
var nested = new Suite('bar');
var test = new Test('baz');

suite.suites.push(nested);
nested.tests.push(test);

expect(suite.hasOnly(), 'to be', false);
});
});

describe('.filterOnly()', function() {
it('should filter out all other tests and suites if a test has `only`', function() {
var suite = new Suite('a');
var nested = new Suite('b');
var test = new Test('c');
var test2 = new Test('d');

suite.suites.push(nested);
suite.appendOnlyTest(test);
suite.tests.push(test2);

suite.filterOnly();

expect(suite, 'to satisfy', {
suites: expect.it('to be empty'),
tests: expect
.it('to have length', 1)
.and('to have an item satisfying', {title: 'c'})
});
});

it('should filter out all other tests and suites if a suite has `only`', function() {
var suite = new Suite('a');
var nested1 = new Suite('b');
var nested2 = new Suite('c');
var test = new Test('d');
var nestedTest = new Test('e');

nested1.appendOnlyTest(nestedTest);

suite.tests.push(test);
suite.suites.push(nested1);
suite.appendOnlySuite(nested1);
suite.suites.push(nested2);

suite.filterOnly();

expect(suite, 'to satisfy', {
suites: expect
.it('to have length', 1)
.and('to have an item satisfying', {title: 'b'}),
tests: expect.it('to be empty')
});
});
});
});

describe('Test', function() {
Expand Down