From 7a6a6e07e9417808281a3597057c9e8540662939 Mon Sep 17 00:00:00 2001
From: Christopher Hiller <boneskull@boneskull.com>
Date: Thu, 21 Jul 2016 00:40:27 -0400
Subject: [PATCH] let child suites run if parent is exclusive; closes #2378

- includes some more refactoring of interface code into `lib/interfaces/common.js` for DRY (still more work to be done here)
- unfortunately said refactoring contains hellish logic which addresses this issue (someone should make a flowchart of this for lolz)
- original PR did not address `exports` interface; this doesn't either
- may need more coverage against `qunit` interface
---
 lib/interfaces/bdd.js                         | 30 ++++----
 lib/interfaces/common.js                      | 69 ++++++++++++++++---
 lib/interfaces/qunit.js                       | 23 ++++---
 lib/interfaces/tdd.js                         | 30 ++++----
 lib/runner.js                                 |  6 +-
 test/integration/fixtures/options/only/bdd.js | 52 +++++++++++++-
 test/integration/fixtures/options/only/tdd.js | 14 +++-
 test/integration/only.js                      |  4 +-
 8 files changed, 173 insertions(+), 55 deletions(-)

diff --git a/lib/interfaces/bdd.js b/lib/interfaces/bdd.js
index 186986b863..830a5fe67a 100644
--- a/lib/interfaces/bdd.js
+++ b/lib/interfaces/bdd.js
@@ -2,7 +2,6 @@
  * Module dependencies.
  */
 
-var Suite = require('../suite');
 var Test = require('../test');
 
 /**
@@ -26,7 +25,7 @@ module.exports = function(suite) {
   var suites = [suite];
 
   suite.on('pre-require', function(context, file, mocha) {
-    var common = require('./common')(suites, context);
+    var common = require('./common')(suites, context, mocha);
 
     context.before = common.before;
     context.after = common.after;
@@ -40,12 +39,11 @@ module.exports = function(suite) {
      */
 
     context.describe = context.context = function(title, fn) {
-      var suite = Suite.create(suites[0], title);
-      suite.file = file;
-      suites.unshift(suite);
-      fn.call(suite);
-      suites.shift();
-      return suite;
+      return common.suite.create({
+        title: title,
+        file: file,
+        fn: fn
+      });
     };
 
     /**
@@ -53,11 +51,11 @@ module.exports = function(suite) {
      */
 
     context.xdescribe = context.xcontext = context.describe.skip = function(title, fn) {
-      var suite = Suite.create(suites[0], title);
-      suite.pending = true;
-      suites.unshift(suite);
-      fn.call(suite);
-      suites.shift();
+      return common.suite.skip({
+        title: title,
+        file: file,
+        fn: fn
+      });
     };
 
     /**
@@ -65,7 +63,11 @@ module.exports = function(suite) {
      */
 
     context.describe.only = function(title, fn) {
-      return common.suite.only(mocha, context.describe(title, fn));
+      return common.suite.only({
+        title: title,
+        file: file,
+        fn: fn
+      });
     };
 
     /**
diff --git a/lib/interfaces/common.js b/lib/interfaces/common.js
index f5ffd9609c..22432cb479 100644
--- a/lib/interfaces/common.js
+++ b/lib/interfaces/common.js
@@ -1,13 +1,16 @@
 'use strict';
 
+var Suite = require('../suite');
+
 /**
  * Functions common to more than one interface.
  *
  * @param {Suite[]} suites
  * @param {Context} context
+ * @param {Mocha} mocha
  * @return {Object} An object containing common functions.
  */
-module.exports = function(suites, context) {
+module.exports = function(suites, context, mocha) {
   return {
     /**
      * This is only present if flag --delay is passed into Mocha. It triggers
@@ -64,15 +67,60 @@ module.exports = function(suites, context) {
 
     suite: {
       /**
-       * Exclusive suite.
+       * Create an exclusive Suite; convenience function
+       * See docstring for create() below.
        *
-       * @param {Object} mocha
-       * @param {Function} suite
+       * @param {Object} opts
+       * @returns {Suite}
        */
-
-      only: function(mocha, suite) {
-        suite.isOnly = true;
+      only: function only(opts) {
         mocha.options.hasOnly = true;
+        opts.isOnly = true;
+        return this.create(opts);
+      },
+
+      /**
+       * Create a Suite, but skip it; convenience function
+       * See docstring for create() below.
+       *
+       * @param {Object} opts
+       * @returns {Suite}
+       */
+      skip: function skip(opts) {
+        opts.pending = true;
+        return this.create(opts);
+      },
+
+      /**
+       * Creates a suite.
+       * @param {Object} opts Options
+       * @param {string} opts.title Title of Suite
+       * @param {Function} [opts.fn] Suite Function (not always applicable)
+       * @param {boolean} [opts.pending] Is Suite pending?
+       * @param {string} [opts.file] Filepath where this Suite resides
+       * @param {boolean} [opts.isOnly] Is Suite exclusive?
+       * @returns {Suite}
+       */
+      create: function create(opts) {
+        var suite = Suite.create(suites[0], opts.title);
+        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 : [];
+        }
+        if (typeof opts.fn === 'function') {
+          opts.fn.call(suite);
+          suites.shift();
+        }
+
         return suite;
       }
     },
@@ -88,8 +136,11 @@ module.exports = function(suites, context) {
        */
       only: function(mocha, test) {
         var suite = test.parent;
-        suite.isOnly = true;
-        suite.onlyTests = (suite.onlyTests || []).concat(test);
+        if (suite.onlyTests === suite.tests) {
+          suite.onlyTests = [test];
+        } else {
+          suite.onlyTests = (suite.onlyTests || []).concat(test);
+        }
         mocha.options.hasOnly = true;
         return test;
       },
diff --git a/lib/interfaces/qunit.js b/lib/interfaces/qunit.js
index 6ce20bb828..a2d67ef90f 100644
--- a/lib/interfaces/qunit.js
+++ b/lib/interfaces/qunit.js
@@ -2,7 +2,6 @@
  * Module dependencies.
  */
 
-var Suite = require('../suite');
 var Test = require('../test');
 
 /**
@@ -34,7 +33,7 @@ module.exports = function(suite) {
   var suites = [suite];
 
   suite.on('pre-require', function(context, file, mocha) {
-    var common = require('./common')(suites, context);
+    var common = require('./common')(suites, context, mocha);
 
     context.before = common.before;
     context.after = common.after;
@@ -49,18 +48,24 @@ module.exports = function(suite) {
       if (suites.length > 1) {
         suites.shift();
       }
-      var suite = Suite.create(suites[0], title);
-      suite.file = file;
-      suites.unshift(suite);
-      return suite;
+      return common.suite.create({
+        title: title,
+        file: file
+      });
     };
 
     /**
-     * Exclusive test-case.
+     * Exclusive Suite.
      */
 
-    context.suite.only = function(title, fn) {
-      return common.suite.only(mocha, context.suite(title, fn));
+    context.suite.only = function(title) {
+      if (suites.length > 1) {
+        suites.shift();
+      }
+      return common.suite.only({
+        title: title,
+        file: file
+      });
     };
 
     /**
diff --git a/lib/interfaces/tdd.js b/lib/interfaces/tdd.js
index 90dcc787ad..445e992213 100644
--- a/lib/interfaces/tdd.js
+++ b/lib/interfaces/tdd.js
@@ -2,7 +2,6 @@
  * Module dependencies.
  */
 
-var Suite = require('../suite');
 var Test = require('../test');
 
 /**
@@ -34,7 +33,7 @@ module.exports = function(suite) {
   var suites = [suite];
 
   suite.on('pre-require', function(context, file, mocha) {
-    var common = require('./common')(suites, context);
+    var common = require('./common')(suites, context, mocha);
 
     context.setup = common.beforeEach;
     context.teardown = common.afterEach;
@@ -47,30 +46,33 @@ module.exports = function(suite) {
      * nested suites and/or tests.
      */
     context.suite = function(title, fn) {
-      var suite = Suite.create(suites[0], title);
-      suite.file = file;
-      suites.unshift(suite);
-      fn.call(suite);
-      suites.shift();
-      return suite;
+      return common.suite.create({
+        title: title,
+        file: file,
+        fn: fn
+      });
     };
 
     /**
      * Pending suite.
      */
     context.suite.skip = function(title, fn) {
-      var suite = Suite.create(suites[0], title);
-      suite.pending = true;
-      suites.unshift(suite);
-      fn.call(suite);
-      suites.shift();
+      return common.suite.skip({
+        title: title,
+        file: file,
+        fn: fn
+      });
     };
 
     /**
      * Exclusive test-case.
      */
     context.suite.only = function(title, fn) {
-      return common.suite.only(mocha, context.suite(title, fn));
+      return common.suite.only({
+        title: title,
+        file: file,
+        fn: fn
+      });
     };
 
     /**
diff --git a/lib/runner.js b/lib/runner.js
index 1e085a6e45..5f91c0beb3 100644
--- a/lib/runner.js
+++ b/lib/runner.js
@@ -847,13 +847,9 @@ Runner.prototype.abort = function() {
  */
 function filterOnly(suite) {
   // If it has `only` tests, run only those
-  if (suite.onlyTests) {
-    suite.tests = suite.onlyTests;
-  }
+  suite.tests = suite.onlyTests ? suite.onlyTests : [];
   // Filter the nested suites
   suite.suites = filter(suite.suites, filterOnly);
-  // Don't run tests from suites that are not marked as `only`
-  suite.tests = suite.isOnly ? suite.tests : [];
   // Keep the suite only if there is something to run
   return suite.suites.length || suite.tests.length;
 }
diff --git a/test/integration/fixtures/options/only/bdd.js b/test/integration/fixtures/options/only/bdd.js
index f2fc4fd40b..783ce19b0b 100644
--- a/test/integration/fixtures/options/only/bdd.js
+++ b/test/integration/fixtures/options/only/bdd.js
@@ -18,4 +18,54 @@ describe('should not run this suite', function() {
   it('should not run this test', function() {
     (true).should.equal(false);
   });
-});
\ No newline at end of file
+});
+
+describe.only('should run this suite too', function() {
+  describe('should run this nested suite', function () {
+    it('should run this test', function() {});
+
+    it('should run this test', function() {});
+
+    it('should run this test', function() {});
+  });
+});
+
+describe.only('should run this suite, even', function() {
+  describe('should run this nested suite, even', function () {
+    describe('should run this doubly-nested suite!', function () {
+      it('should run this test', function() {});
+
+      it('should run this test', function() {});
+
+      it('should run this test', function() {});
+    });
+  });
+});
+
+
+describe('should run this suite with an exclusive test', function() {
+  it.only('should run this test', function () {});
+
+  describe('should not run this nested suite', function () {
+    describe.only('should not run this doubly-nested suite', function () {
+      it('should not run this test', function() {});
+
+      it('should not run this test', function() {});
+
+      it('should not run this test', function() {});
+    });
+  });
+});
+
+describe('should run this suite with an exclusive test (reverse order)', function() {
+  describe('should not run this nested suite', function () {
+    describe.only('should not run this doubly-nested suite', function () {
+      it('should not run this test', function() {});
+
+      it('should not run this test', function() {});
+
+      it('should not run this test', function() {});
+    });
+  });
+  it.only('should run this test', function () {});
+});
diff --git a/test/integration/fixtures/options/only/tdd.js b/test/integration/fixtures/options/only/tdd.js
index 6e1e37becf..129a9e5d74 100644
--- a/test/integration/fixtures/options/only/tdd.js
+++ b/test/integration/fixtures/options/only/tdd.js
@@ -20,4 +20,16 @@ suite('should not run this suite', function() {
   test('should not run this test', function() {
     (true).should.equal(false);
   });
-});
\ No newline at end of file
+});
+
+suite.only('should run this suite too', function() {
+  suite('should run this nested suite', function () {
+    test('should run this test', function() {});
+
+    test('should run this test', function() {});
+
+    test('should run this test', function() {});
+
+    test('should run this test', function() {});
+  });
+});
diff --git a/test/integration/only.js b/test/integration/only.js
index 3d0e4dc682..dc7644e5a4 100644
--- a/test/integration/only.js
+++ b/test/integration/only.js
@@ -6,7 +6,7 @@ describe('.only()', function() {
     run('options/only/bdd.js', ['--ui', 'bdd'], function(err, res) {
       assert(!err);
       assert.equal(res.stats.pending, 0);
-      assert.equal(res.stats.passes, 3);
+      assert.equal(res.stats.passes, 11);
       assert.equal(res.stats.failures, 0);
       assert.equal(res.code, 0);
       done();
@@ -17,7 +17,7 @@ describe('.only()', function() {
     run('options/only/tdd.js', ['--ui', 'tdd'], function(err, res) {
       assert(!err);
       assert.equal(res.stats.pending, 0);
-      assert.equal(res.stats.passes, 4);
+      assert.equal(res.stats.passes, 8);
       assert.equal(res.stats.failures, 0);
       assert.equal(res.code, 0);
       done();