From ca1cfc4caa6bcb94aa2d99c7dbab5ced06fb91dd Mon Sep 17 00:00:00 2001
From: Arvid Ottenberg <arvid.ottenberg@gmx.de>
Date: Fri, 5 Jun 2020 23:24:19 +0200
Subject: [PATCH] Consistent handling of --forbid-only for suites and tests
 (#4282)

* implement markOnly method for suite class

* add test cases to cover suites markOnly method

* make forbidOnly option throw exception even if suite is excluded by grep

* adapt test cases to changed forbidOnly logic

* reuse existing sandbox variable

* remove unnecessary check for forbid only option

* remove duplicate beforeEach afterEach defined for markOnly test suite
---
 lib/interfaces/common.js                    | 18 ++++++-----
 lib/suite.js                                |  9 ++++++
 test/integration/options/forbidOnly.spec.js | 35 ++++++++++++---------
 test/unit/suite.spec.js                     | 15 +++++++++
 4 files changed, 54 insertions(+), 23 deletions(-)

diff --git a/lib/interfaces/common.js b/lib/interfaces/common.js
index 5fa87e4537..3c171ef8d7 100644
--- a/lib/interfaces/common.js
+++ b/lib/interfaces/common.js
@@ -94,6 +94,9 @@ module.exports = function(suites, context, mocha) {
        * @returns {Suite}
        */
       only: function only(opts) {
+        if (mocha.options.forbidOnly) {
+          throw createForbiddenExclusivityError(mocha);
+        }
         opts.isOnly = true;
         return this.create(opts);
       },
@@ -127,15 +130,14 @@ module.exports = function(suites, context, mocha) {
         suite.file = opts.file;
         suites.unshift(suite);
         if (opts.isOnly) {
-          if (mocha.options.forbidOnly && shouldBeTested(suite)) {
-            throw createForbiddenExclusivityError(mocha);
-          }
-          suite.parent.appendOnlySuite(suite);
+          suite.markOnly();
         }
-        if (suite.pending) {
-          if (mocha.options.forbidPending && shouldBeTested(suite)) {
-            throw createUnsupportedError('Pending test forbidden');
-          }
+        if (
+          suite.pending &&
+          mocha.options.forbidPending &&
+          shouldBeTested(suite)
+        ) {
+          throw createUnsupportedError('Pending test forbidden');
         }
         if (typeof opts.fn === 'function') {
           opts.fn.call(suite);
diff --git a/lib/suite.js b/lib/suite.js
index f3c8b104af..24e6dd344e 100644
--- a/lib/suite.js
+++ b/lib/suite.js
@@ -491,6 +491,15 @@ Suite.prototype.appendOnlySuite = function(suite) {
   this._onlySuites.push(suite);
 };
 
+/**
+ * Marks a suite to be `only`.
+ *
+ * @private
+ */
+Suite.prototype.markOnly = function() {
+  this.parent && this.parent.appendOnlySuite(this);
+};
+
 /**
  * Adds a test to the list of tests marked `only`.
  *
diff --git a/test/integration/options/forbidOnly.spec.js b/test/integration/options/forbidOnly.spec.js
index 1886becb7d..252c5b99b9 100644
--- a/test/integration/options/forbidOnly.spec.js
+++ b/test/integration/options/forbidOnly.spec.js
@@ -92,32 +92,37 @@ describe('--forbid-only', function() {
     );
   });
 
-  it('should succeed if suite marked only does not match grep', function(done) {
+  it('should fail if suite marked only does not match grep', function(done) {
     var fixture = path.join('options', 'forbid-only', 'only-suite');
-    runMochaJSON(fixture, args.concat('--fgrep', 'bumble bees'), function(
-      err,
-      res
-    ) {
-      if (err) {
-        return done(err);
-      }
-      expect(res, 'to have passed');
-      done();
-    });
+    var spawnOpts = {stdio: 'pipe'};
+    runMocha(
+      fixture,
+      args.concat('--fgrep', 'bumble bees'),
+      function(err, res) {
+        if (err) {
+          return done(err);
+        }
+        expect(res, 'to have failed with output', new RegExp(onlyErrorMessage));
+        done();
+      },
+      spawnOpts
+    );
   });
 
-  it('should succeed if suite marked only does not match inverted grep', function(done) {
+  it('should fail if suite marked only does not match inverted grep', function(done) {
     var fixture = path.join('options', 'forbid-only', 'only-suite');
-    runMochaJSON(
+    var spawnOpts = {stdio: 'pipe'};
+    runMocha(
       fixture,
       args.concat('--fgrep', 'suite marked with only', '--invert'),
       function(err, res) {
         if (err) {
           return done(err);
         }
-        expect(res, 'to have passed');
+        expect(res, 'to have failed with output', new RegExp(onlyErrorMessage));
         done();
-      }
+      },
+      spawnOpts
     );
   });
 
diff --git a/test/unit/suite.spec.js b/test/unit/suite.spec.js
index a5063b7f91..6166496ee5 100644
--- a/test/unit/suite.spec.js
+++ b/test/unit/suite.spec.js
@@ -680,6 +680,21 @@ describe('Suite', function() {
       });
     });
   });
+
+  describe('.markOnly()', function() {
+    it('should call appendOnlySuite on parent', function() {
+      var suite = new Suite('foo');
+      var spy = sandbox.spy();
+      suite.parent = {
+        appendOnlySuite: spy
+      };
+      suite.markOnly();
+
+      expect(spy, 'to have a call exhaustively satisfying', [suite]).and(
+        'was called once'
+      );
+    });
+  });
 });
 
 describe('Test', function() {