--forbid-only doesn't recognize it.only when before crashes (#4256)…

…; closes #3840

* add fixtures that result in it.only combined with --forbid-only bug

* adapt forbidonly test cases to cover it.only bug

* check if forbid only option is set prior to marking a test only and throw error

* change name of only test back to previous

* use custom assertion for expecting error in forbidOnly tests

* remove empty line

* use createUnsupportedError instead of throw new Error

* use createUnsupportedError instead of throw new Error

* remove check if suite hasOnly and forbidOnly option is set as this is done before runtime

* implement markOnly instance method in suite class

* add unit test for suites markOnly method

* throw exception if --forbid-only option is set even if suite is not selected by grep

* adapt forbidOnly integration tests to check for failure if only suite is not selected by grep

* fix jsdocs of suite markonly

* Revert "fix jsdocs of suite markonly"

This reverts commit cffc71a.

* Revert "adapt forbidOnly integration tests to check for failure if only suite is not selected by grep"

This reverts commit 336425a.

* Revert "throw exception if --forbid-only option is set even if suite is not selected by grep"

This reverts commit f871782.

* Revert "add unit test for suites markOnly method"

This reverts commit c2c8dc8.

* Revert "implement markOnly instance method in suite class"

This reverts commit 4b37e3c.

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.
@@ -126,14 +127,14 @@ module.exports = function(suites, context, mocha) {
         suites.unshift(suite);
         if (opts.isOnly) {
           if (mocha.options.forbidOnly && shouldBeTested(suite)) {
-            throw new Error('`.only` forbidden');
+            throw createUnsupportedError('`.only` forbidden');
           }
 
           suite.parent.appendOnlySuite(suite);
         }
         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 +166,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

@@ -577,11 +577,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;
   }

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,10 +85,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
@@ -124,4 +120,38 @@ describe('--forbid-only', function() {
       }
     );
   });
+
+  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
+    );
+  });
 });