Bug 3840: --forbid-only doesn't recognize it.only when before crashes

lib/interfaces/common.js

@@ -3,6 +3,7 @@
 var Suite = require('../suite');
 var errors = require('../errors');
 var createMissingArgumentError = errors.createMissingArgumentError;
+var createUnsupportedError = errors.createUnsupportedError;
 
 /**
  * Functions common to more than one interface.
@@ -92,6 +93,9 @@ module.exports = function(suites, context, mocha) {
        * @returns {Suite}
        */
       only: function only(opts) {
+        if (mocha.options.forbidOnly) {
+          throw createUnsupportedError('`.only` forbidden');
+        }
         opts.isOnly = true;
         return this.create(opts);
       },
@@ -125,15 +129,11 @@ module.exports = function(suites, context, mocha) {
         suite.file = opts.file;
         suites.unshift(suite);
         if (opts.isOnly) {
-          if (mocha.options.forbidOnly && shouldBeTested(suite)) {
-            throw new Error('`.only` forbidden');
-          }
-
-          suite.parent.appendOnlySuite(suite);
+          suite.markOnly();
         }
         if (suite.pending) {
           if (mocha.options.forbidPending && shouldBeTested(suite)) {
-            throw new Error('Pending test forbidden');
+            throw createUnsupportedError('Pending test forbidden');
           }
         }
         if (typeof opts.fn === 'function') {
@@ -165,6 +165,8 @@ module.exports = function(suites, context, mocha) {
        * @returns {*}
        */
       only: function(mocha, test) {
+        if (mocha.options.forbidOnly)
+          throw createUnsupportedError('`.only` forbidden');
         test.markOnly();
         return test;
       },

lib/runner.js

@@ -522,11 +522,6 @@ Runner.prototype.runTest = function(fn) {
     return;
   }
 
-  var suite = this.parents().reverse()[0] || this.suite;
-  if (this.forbidOnly && suite.hasOnly()) {
-    fn(new Error('`.only` forbidden'));
-    return;
-  }
   if (this.asyncOnly) {
     test.asyncOnly = true;
   }

lib/suite.js

@@ -493,6 +493,15 @@ Suite.prototype.appendOnlySuite = function(suite) {
   this._onlySuites.push(suite);
 };
 
+/**
+ * Marks a suite to be `only`.
+ *
+ * @private
+ */
+Suite.prototype.markOnly = function() {
+  this.parent.appendOnlySuite(this);
+};
+
 /**
  * Adds a test to the list of tests marked `only`.
  *

test/integration/fixtures/options/forbid-only/only-before-each.fixture.js

@@ -0,0 +1,8 @@
+'use strict';
+
+describe('test marked with only and beforeEach has skip', function() {
+  beforeEach(function() {
+    this.skip();
+  });
+  it.only('only test', function() {});
+});

test/integration/fixtures/options/forbid-only/only-before.fixture.js

@@ -0,0 +1,8 @@
+'use strict';
+
+describe('test marked with only and before has skip', function() {
+  before(function() {
+    this.skip();
+  });
+  it.only('only test', function() {});
+});

test/integration/options/forbidOnly.spec.js

@@ -26,13 +26,19 @@ describe('--forbid-only', function() {
 
   it('should fail if there are tests marked only', function(done) {
     var fixture = path.join('options', 'forbid-only', 'only');
-    runMochaJSON(fixture, args, function(err, res) {
-      if (err) {
-        return done(err);
-      }
-      expect(res, 'to have failed with error', onlyErrorMessage);
-      done();
-    });
+    var spawnOpts = {stdio: 'pipe'};
+    runMocha(
+      fixture,
+      args,
+      function(err, res) {
+        if (err) {
+          return done(err);
+        }
+        expect(res, 'to have failed with output', new RegExp(onlyErrorMessage));
+        done();
+      },
+      spawnOpts
+    );
   });
 
   it('should fail if there are tests in suites marked only', function(done) {
@@ -45,11 +51,7 @@ describe('--forbid-only', function() {
         if (err) {
           return done(err);
         }
-
-        expect(res, 'to satisfy', {
-          code: 1,
-          output: new RegExp(onlyErrorMessage)
-        });
+        expect(res, 'to have failed with output', new RegExp(onlyErrorMessage));
         done();
       },
       spawnOpts
@@ -66,10 +68,7 @@ describe('--forbid-only', function() {
         if (err) {
           return done(err);
         }
-        expect(res, 'to satisfy', {
-          code: 1,
-          output: new RegExp(onlyErrorMessage)
-        });
+        expect(res, 'to have failed with output', new RegExp(onlyErrorMessage));
         done();
       },
       spawnOpts
@@ -86,42 +85,78 @@ describe('--forbid-only', function() {
         if (err) {
           return done(err);
         }
-        expect(res, 'to satisfy', {
-          code: 1,
-          output: new RegExp(onlyErrorMessage)
-        });
+        expect(res, 'to have failed with output', new RegExp(onlyErrorMessage));
         done();
       },
       spawnOpts
     );
   });
 
-  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
+    );
+  });
+
+  it('should fail even if before has "skip"', function(done) {
+    var fixture = path.join('options', 'forbid-only', 'only-before');
+    var spawnOpts = {stdio: 'pipe'};
+    runMocha(
+      fixture,
+      args,
+      function(err, res) {
+        if (err) {
+          return done(err);
+        }
+        expect(res, 'to have failed with output', new RegExp(onlyErrorMessage));
+        done();
+      },
+      spawnOpts
+    );
+  });
+
+  it('should fail even if beforeEach has "skip"', function(done) {
+    var fixture = path.join('options', 'forbid-only', 'only-before-each');
+    var spawnOpts = {stdio: 'pipe'};
+    runMocha(
+      fixture,
+      args,
+      function(err, res) {
+        if (err) {
+          return done(err);
+        }
+        expect(res, 'to have failed with output', new RegExp(onlyErrorMessage));
+        done();
+      },
+      spawnOpts
     );
   });
 });

test/unit/suite.spec.js

@@ -638,6 +638,31 @@ describe('Suite', function() {
       });
     });
   });
+
+  describe('.markOnly()', function() {
+    var sandbox;
+
+    beforeEach(function() {
+      sandbox = sinon.createSandbox();
+    });
+
+    afterEach(function() {
+      sandbox.restore();
+    });
+
+    it('should call appendOnlySuite on parent', function() {
+      var suite = new Suite('a');
+      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() {