Merge pull request mochajs#2411 from not-an-aardvark/2406-nested-describe-only

fix nested describe.only suites; closes mochajs#2406

Author: boneskull

lib/interfaces/common.js

@@ -106,15 +106,9 @@ module.exports = function(suites, context, mocha) {
         suite.pending = Boolean(opts.pending);
         suite.file = opts.file;
         suites.unshift(suite);
-        // I should be pilloried for the following.
         if (opts.isOnly) {
-          if (suite.parent && suite.parent.onlyTests) {
-            suite.onlyTests = suite.parent.onlyTests === suite.parent.tests ? suite.tests : [];
-          } else {
-            suite.onlyTests = suite.tests;
-          }
-        } else {
-          suite.onlyTests = suite.parent && suite.parent.onlyTests === suite.parent.tests ? suite.tests : [];
+          suite.parent._onlySuites = suite.parent._onlySuites.concat(suite);
+          mocha.options.hasOnly = true;
         }
         if (typeof opts.fn === 'function') {
           opts.fn.call(suite);
@@ -135,12 +129,7 @@ module.exports = function(suites, context, mocha) {
        * @returns {*}
        */
       only: function(mocha, test) {
-        var suite = test.parent;
-        if (suite.onlyTests === suite.tests) {
-          suite.onlyTests = [test];
-        } else {
-          suite.onlyTests = (suite.onlyTests || []).concat(test);
-        }
+        test.parent._onlyTests = test.parent._onlyTests.concat(test);
         mocha.options.hasOnly = true;
         return test;
       },

lib/runner.js

@@ -10,6 +10,7 @@ var debug = require('debug')('mocha:runner');
 var Runnable = require('./runnable');
 var filter = utils.filter;
 var indexOf = utils.indexOf;
+var some = utils.some;
 var keys = utils.keys;
 var stackFilter = utils.stackTraceFilter();
 var stringify = utils.stringify;
@@ -846,12 +847,38 @@ Runner.prototype.abort = function() {
  * @api private
  */
 function filterOnly(suite) {
-  // If it has `only` tests, run only those
-  suite.tests = suite.onlyTests ? suite.onlyTests : [];
-  // Filter the nested suites
-  suite.suites = filter(suite.suites, filterOnly);
+  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 the current suite.
+      // Otherwise, all of the tests on this `only` suite should be run, so don't filter it.
+      if (hasOnly(onlySuite)) {
+        filterOnly(suite);
+      }
+    });
+    // Run the `only` suites, as well as any other suites that have `only` tests/suites as descendants.
+    suite.suites = filter(suite.suites, function(childSuite) {
+      return indexOf(suite._onlySuites, childSuite) !== -1 || filterOnly(childSuite);
+    });
+  }
   // Keep the suite only if there is something to run
-  return suite.suites.length || suite.tests.length;
+  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}
+ * @api private
+ */
+function hasOnly(suite) {
+  return suite._onlyTests.length || suite._onlySuites.length || some(suite.suites, hasOnly);
 }
 
 /**

lib/suite.js

@@ -61,6 +61,8 @@ function Suite(title, parentContext) {
   this._slow = 75;
   this._bail = false;
   this._retries = -1;
+  this._onlyTests = [];
+  this._onlySuites = [];
   this.delayed = false;
 }
 

lib/utils.js

@@ -138,6 +138,23 @@ exports.filter = function(arr, fn) {
   return ret;
 };
 
+/**
+ * Array#some (<=IE8)
+ *
+ * @api private
+ * @param {Array} arr
+ * @param {Function} fn
+ * @return {Array}
+ */
+exports.some = function(arr, fn) {
+  for (var i = 0, l = arr.length; i < l; i++) {
+    if (fn(arr[i])) {
+      return true;
+    }
+  }
+  return false;
+};
+
 /**
  * Object.keys (<=IE8)
  *

test/integration/fixtures/regression/issue-2406.js

@@ -0,0 +1,15 @@
+describe('outer describe', function() {
+  it('should not run this test', function() {});
+  describe('this suite should not run', function() {
+    it('should not run this test', function() {});
+  });
+  describe.only('this .only suite should run', function() {
+    describe('this suite should run', function() {
+      it('should run this test in a nested suite', function() {});
+    });
+    it('should run this test', function() {});
+  });
+  describe('this suite should not run', function() {
+    it('should not run this test', function() {});
+  });
+});

test/integration/regression.js

@@ -1,7 +1,8 @@
-var assert = require('assert');
-var fs     = require('fs');
-var path   = require('path');
-var run    = require('./helpers').runMocha;
+var assert  = require('assert');
+var fs      = require('fs');
+var path    = require('path');
+var run     = require('./helpers').runMocha;
+var runJSON = require('./helpers').runMochaJSON;
 
 describe('regressions', function() {
   it('issue-1327: should run all 3 specs exactly once', function(done) {
@@ -60,4 +61,16 @@ describe('regressions', function() {
       afterWasRun.should.be.ok();
     });
   });
+
+  it('issue-2406: should run nested describe.only suites', function(done) {
+    this.timeout(2000);
+    runJSON('regression/issue-2406.js', [], function(err, res) {
+      assert(!err);
+      assert.equal(res.stats.pending, 0);
+      assert.equal(res.stats.passes, 2);
+      assert.equal(res.stats.failures, 0);
+      assert.equal(res.code, 0);
+      done();
+    });
+  });
 });