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
mochajs · Jun 5, 2020 · ca1cfc4 · ca1cfc4
1 parent 0a66259
commit ca1cfc4
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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() {