Skip to content

Commit 118c9ae

Browse files
vkarpov15vkarpov15
authored
Refactor out usages of Suite#_onlyTests and Suite#_onlyTests (#3689) (#3707)
* Refactor out usages of Suite#_onlyTests and Suite#_onlyTests (#3689) * Mark new Suite methods as private * Make hasOnly() and filterOnly() consistently return boolean * Add test coverage for Suite#hasOnly() and Suite#filterOnly() * Refactor tests re: code review comments
1 parent 0dacd1f commit 118c9ae

5 files changed

Lines changed: 164 additions & 54 deletions

File tree

‎jsdoc.conf.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"default": {
2828
"includeDate": false,
2929
"outputSourceFiles": true
30-
},
30+
},
3131
"monospaceLinks": false
3232
}
3333
}

‎lib/interfaces/common.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ module.exports = function(suites, context, mocha) {
130130
throw new Error('`.only` forbidden');
131131
}
132132

133-
suite.parent._onlySuites = suite.parent._onlySuites.concat(suite);
133+
suite.parent.appendOnlySuite(suite);
134134
}
135135
if (suite.pending) {
136136
if (mocha.options.forbidPending && shouldBeTested(suite)) {
@@ -175,7 +175,7 @@ module.exports = function(suites, context, mocha) {
175175
* @returns {*}
176176
*/
177177
only: function(mocha, test) {
178-
test.parent._onlyTests = test.parent._onlyTests.concat(test);
178+
test.parent.appendOnlyTest(test);
179179
return test;
180180
},
181181

‎lib/runner.js‎

Lines changed: 5 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,9 @@ Runner.prototype.runTest = function(fn) {
505505
if (!test) {
506506
return;
507507
}
508-
if (this.forbidOnly && hasOnly(this.parents().reverse()[0] || this.suite)) {
508+
509+
var suite = this.parents().reverse()[0] || this.suite;
510+
if (this.forbidOnly && suite.hasOnly()) {
509511
fn(new Error('`.only` forbidden'));
510512
return;
511513
}
@@ -874,8 +876,8 @@ Runner.prototype.run = function(fn) {
874876

875877
function start() {
876878
// If there is an `only` filter
877-
if (hasOnly(rootSuite)) {
878-
filterOnly(rootSuite);
879+
if (rootSuite.hasOnly()) {
880+
rootSuite.filterOnly();
879881
}
880882
self.started = true;
881883
if (self._delay) {
@@ -932,54 +934,6 @@ Runner.prototype.abort = function() {
932934
return this;
933935
};
934936

935-
/**
936-
* Filter suites based on `isOnly` logic.
937-
*
938-
* @param {Array} suite
939-
* @returns {Boolean}
940-
* @private
941-
*/
942-
function filterOnly(suite) {
943-
if (suite._onlyTests.length) {
944-
// If the suite contains `only` tests, run those and ignore any nested suites.
945-
suite.tests = suite._onlyTests;
946-
suite.suites = [];
947-
} else {
948-
// Otherwise, do not run any of the tests in this suite.
949-
suite.tests = [];
950-
suite._onlySuites.forEach(function(onlySuite) {
951-
// If there are other `only` tests/suites nested in the current `only` suite, then filter that `only` suite.
952-
// Otherwise, all of the tests on this `only` suite should be run, so don't filter it.
953-
if (hasOnly(onlySuite)) {
954-
filterOnly(onlySuite);
955-
}
956-
});
957-
// Run the `only` suites, as well as any other suites that have `only` tests/suites as descendants.
958-
suite.suites = suite.suites.filter(function(childSuite) {
959-
return (
960-
suite._onlySuites.indexOf(childSuite) !== -1 || filterOnly(childSuite)
961-
);
962-
});
963-
}
964-
// Keep the suite only if there is something to run
965-
return suite.tests.length || suite.suites.length;
966-
}
967-
968-
/**
969-
* Determines whether a suite has an `only` test or suite as a descendant.
970-
*
971-
* @param {Array} suite
972-
* @returns {Boolean}
973-
* @private
974-
*/
975-
function hasOnly(suite) {
976-
return (
977-
suite._onlyTests.length ||
978-
suite._onlySuites.length ||
979-
suite.suites.some(hasOnly)
980-
);
981-
}
982-
983937
/**
984938
* Filter leaks with the given globals flagged as `ok`.
985939
*

‎lib/suite.js‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -434,6 +434,73 @@ Suite.prototype.run = function run() {
434434
}
435435
};
436436

437+
/**
438+
* Determines whether a suite has an `only` test or suite as a descendant.
439+
*
440+
* @private
441+
* @returns {Boolean}
442+
*/
443+
Suite.prototype.hasOnly = function hasOnly() {
444+
return (
445+
this._onlyTests.length > 0 ||
446+
this._onlySuites.length > 0 ||
447+
this.suites.some(function(suite) {
448+
return suite.hasOnly();
449+
})
450+
);
451+
};
452+
453+
/**
454+
* Filter suites based on `isOnly` logic.
455+
*
456+
* @private
457+
* @returns {Boolean}
458+
*/
459+
Suite.prototype.filterOnly = function filterOnly() {
460+
if (this._onlyTests.length) {
461+
// If the suite contains `only` tests, run those and ignore any nested suites.
462+
this.tests = this._onlyTests;
463+
this.suites = [];
464+
} else {
465+
// Otherwise, do not run any of the tests in this suite.
466+
this.tests = [];
467+
this._onlySuites.forEach(function(onlySuite) {
468+
// If there are other `only` tests/suites nested in the current `only` suite, then filter that `only` suite.
469+
// Otherwise, all of the tests on this `only` suite should be run, so don't filter it.
470+
if (onlySuite.hasOnly()) {
471+
onlySuite.filterOnly();
472+
}
473+
});
474+
// Run the `only` suites, as well as any other suites that have `only` tests/suites as descendants.
475+
var onlySuites = this._onlySuites;
476+
this.suites = this.suites.filter(function(childSuite) {
477+
return onlySuites.indexOf(childSuite) !== -1 || childSuite.filterOnly();
478+
});
479+
}
480+
// Keep the suite only if there is something to run
481+
return this.tests.length > 0 || this.suites.length > 0;
482+
};
483+
484+
/**
485+
* Adds a suite to the list of subsuites marked `only`.
486+
*
487+
* @private
488+
* @param {Suite} suite
489+
*/
490+
Suite.prototype.appendOnlySuite = function(suite) {
491+
this._onlySuites.push(suite);
492+
};
493+
494+
/**
495+
* Adds a test to the list of tests marked `only`.
496+
*
497+
* @private
498+
* @param {Test} test
499+
*/
500+
Suite.prototype.appendOnlyTest = function(test) {
501+
this._onlyTests.push(test);
502+
};
503+
437504
/**
438505
* Returns the array of hooks by hook name; see `HOOK_TYPE_*` constants.
439506
* @private

‎test/unit/suite.spec.js‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,95 @@ describe('Suite', function() {
545545
expect(suite.timeout(), 'to be', 100);
546546
});
547547
});
548+
549+
describe('hasOnly()', function() {
550+
it('should return true if a test has `only`', function() {
551+
var suite = new Suite('foo');
552+
var test = new Test('bar');
553+
554+
suite.appendOnlyTest(test);
555+
556+
expect(suite.hasOnly(), 'to be', true);
557+
});
558+
559+
it('should return true if a suite has `only`', function() {
560+
var suite = new Suite('foo');
561+
var nested = new Suite('bar');
562+
563+
suite.appendOnlySuite(nested);
564+
565+
expect(suite.hasOnly(), 'to be', true);
566+
});
567+
568+
it('should return true if nested suite has `only`', function() {
569+
var suite = new Suite('foo');
570+
var nested = new Suite('bar');
571+
var test = new Test('baz');
572+
573+
nested.appendOnlyTest(test);
574+
// `nested` has a `only` test, but `suite` doesn't know about it
575+
suite.suites.push(nested);
576+
577+
expect(suite.hasOnly(), 'to be', true);
578+
});
579+
580+
it('should return false if no suite or test is marked `only`', function() {
581+
var suite = new Suite('foo');
582+
var nested = new Suite('bar');
583+
var test = new Test('baz');
584+
585+
suite.suites.push(nested);
586+
nested.tests.push(test);
587+
588+
expect(suite.hasOnly(), 'to be', false);
589+
});
590+
});
591+
592+
describe('.filterOnly()', function() {
593+
it('should filter out all other tests and suites if a test has `only`', function() {
594+
var suite = new Suite('a');
595+
var nested = new Suite('b');
596+
var test = new Test('c');
597+
var test2 = new Test('d');
598+
599+
suite.suites.push(nested);
600+
suite.appendOnlyTest(test);
601+
suite.tests.push(test2);
602+
603+
suite.filterOnly();
604+
605+
expect(suite, 'to satisfy', {
606+
suites: expect.it('to be empty'),
607+
tests: expect
608+
.it('to have length', 1)
609+
.and('to have an item satisfying', {title: 'c'})
610+
});
611+
});
612+
613+
it('should filter out all other tests and suites if a suite has `only`', function() {
614+
var suite = new Suite('a');
615+
var nested1 = new Suite('b');
616+
var nested2 = new Suite('c');
617+
var test = new Test('d');
618+
var nestedTest = new Test('e');
619+
620+
nested1.appendOnlyTest(nestedTest);
621+
622+
suite.tests.push(test);
623+
suite.suites.push(nested1);
624+
suite.appendOnlySuite(nested1);
625+
suite.suites.push(nested2);
626+
627+
suite.filterOnly();
628+
629+
expect(suite, 'to satisfy', {
630+
suites: expect
631+
.it('to have length', 1)
632+
.and('to have an item satisfying', {title: 'b'}),
633+
tests: expect.it('to be empty')
634+
});
635+
});
636+
});
548637
});
549638

550639
describe('Test', function() {

0 commit comments

Comments
 (0)