From 0e6bf82c1ec57373793c754c213765d13448a687 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Thu, 31 Jan 2019 10:54:40 -0500 Subject: [PATCH 1/5] Refactor out usages of Suite#_onlyTests and Suite#_onlyTests (#3689) --- lib/interfaces/common.js | 4 +-- lib/runner.js | 56 ++++------------------------------- lib/suite.js | 63 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 53 deletions(-) diff --git a/lib/interfaces/common.js b/lib/interfaces/common.js index 6384c7d477..976b4d7a65 100644 --- a/lib/interfaces/common.js +++ b/lib/interfaces/common.js @@ -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)) { @@ -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; }, diff --git a/lib/runner.js b/lib/runner.js index 7c4435219e..c082ce9315 100644 --- a/lib/runner.js +++ b/lib/runner.js @@ -444,7 +444,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; } @@ -853,8 +855,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; Runner.immediately(function() { @@ -910,54 +912,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`. * diff --git a/lib/suite.js b/lib/suite.js index 2cb3a39de9..a768b9c95a 100644 --- a/lib/suite.js +++ b/lib/suite.js @@ -427,3 +427,66 @@ Suite.prototype.run = function run() { this.emit('run'); } }; + +/** + * Determines whether a suite has an `only` test or suite as a descendant. + * + * @returns {Boolean} + */ +Suite.prototype.hasOnly = function hasOnly() { + return ( + this._onlyTests.length || + this._onlySuites.length || + this.suites.some(function(suite) { + return suite.hasOnly(); + }) + ); +}; + +/** + * Filter suites based on `isOnly` logic. + * + * @returns {Boolean} + */ +Suite.prototype.filterOnly = function filterOnly() { + 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 || this.suites.length; +}; + +/** + * Adds a suite to the list of subsuites marked `only`. + * + * @param {Suite} suite + */ +Suite.prototype.appendOnlySuite = function(suite) { + this._onlySuites.push(suite); +}; + +/** + * Adds a test to the list of tests marked `only`. + * + * @param {Test} test + */ +Suite.prototype.appendOnlyTest = function(test) { + this._onlyTests.push(test); +}; From 4708d62c53b9c6694f2c15ef2462d874fdc7e147 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 6 Feb 2019 10:12:36 -0500 Subject: [PATCH 2/5] Mark new Suite methods as private --- lib/suite.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/suite.js b/lib/suite.js index 69a07289a8..8e235c22ce 100644 --- a/lib/suite.js +++ b/lib/suite.js @@ -437,6 +437,7 @@ 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() { @@ -452,6 +453,7 @@ Suite.prototype.hasOnly = function hasOnly() { /** * Filter suites based on `isOnly` logic. * + * @private * @returns {Boolean} */ Suite.prototype.filterOnly = function filterOnly() { @@ -482,6 +484,7 @@ Suite.prototype.filterOnly = function filterOnly() { /** * Adds a suite to the list of subsuites marked `only`. * + * @private * @param {Suite} suite */ Suite.prototype.appendOnlySuite = function(suite) { @@ -491,6 +494,7 @@ Suite.prototype.appendOnlySuite = function(suite) { /** * Adds a test to the list of tests marked `only`. * + * @private * @param {Test} test */ Suite.prototype.appendOnlyTest = function(test) { From a083a77fe940f6a50ca3b9c36153f0c6bab1577e Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 6 Feb 2019 10:41:30 -0500 Subject: [PATCH 3/5] Make hasOnly() and filterOnly() consistently return boolean --- lib/suite.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/suite.js b/lib/suite.js index 8e235c22ce..13c6ec792a 100644 --- a/lib/suite.js +++ b/lib/suite.js @@ -442,8 +442,8 @@ Suite.prototype.run = function run() { */ Suite.prototype.hasOnly = function hasOnly() { return ( - this._onlyTests.length || - this._onlySuites.length || + this._onlyTests.length > 0 || + this._onlySuites.length > 0 || this.suites.some(function(suite) { return suite.hasOnly(); }) @@ -478,7 +478,7 @@ Suite.prototype.filterOnly = function filterOnly() { }); } // Keep the suite only if there is something to run - return this.tests.length || this.suites.length; + return this.tests.length > 0 || this.suites.length > 0; }; /** From 62390b1863487fe560b5907140057d2db11722c5 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 6 Feb 2019 10:42:36 -0500 Subject: [PATCH 4/5] Add test coverage for Suite#hasOnly() and Suite#filterOnly() --- test/unit/suite.spec.js | 83 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/test/unit/suite.spec.js b/test/unit/suite.spec.js index 971208b9b9..65d3fd4f05 100644 --- a/test/unit/suite.spec.js +++ b/test/unit/suite.spec.js @@ -545,6 +545,89 @@ 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.suites.length, 'to be', 0); + expect(suite.tests.length, 'to be', 1); + expect(suite.tests[0].title, 'to be', '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.suites.length, 'to be', 1); + expect(suite.tests.length, 'to be', 0); + expect(suite.suites[0].title, 'to be', 'b'); + }); + }); }); describe('Test', function() { From 734d9da0a21d3aff78cbecf66e68fdcbfaec3614 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 11 Feb 2019 21:43:00 -0500 Subject: [PATCH 5/5] Refactor tests re: code review comments --- test/unit/suite.spec.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/test/unit/suite.spec.js b/test/unit/suite.spec.js index 65d3fd4f05..2a1d639c53 100644 --- a/test/unit/suite.spec.js +++ b/test/unit/suite.spec.js @@ -602,9 +602,12 @@ describe('Suite', function() { suite.filterOnly(); - expect(suite.suites.length, 'to be', 0); - expect(suite.tests.length, 'to be', 1); - expect(suite.tests[0].title, 'to be', 'c'); + 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() { @@ -623,9 +626,12 @@ describe('Suite', function() { suite.filterOnly(); - expect(suite.suites.length, 'to be', 1); - expect(suite.tests.length, 'to be', 0); - expect(suite.suites[0].title, 'to be', 'b'); + 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') + }); }); }); });