fix weird "pending" behavior; closes #2286

Author: boneskull

lib/runner.js

@@ -313,15 +313,20 @@ Runner.prototype.hook = function (name, fn) {
       }
       if (err) {
         if (err instanceof Pending) {
-          if (name === 'beforeEach' || name === 'afterEach') {
-            self.test.pending = true;
-          } else {
-            utils.forEach(suite.tests, function (test) {
-              test.pending = true;
-            });
-            // a pending hook won't be executed twice.
-            hook.pending = true;
+          var runnables = suite.tests;
+          switch (name) {
+            case 'beforeAll':
+              runnables = runnables.concat(suite._beforeAll);
+              // falls through
+            case 'afterAll':
+              runnables = runnables.concat(suite._afterAll);
+              // falls through
+            default:
+              runnables = runnables.concat(suite._beforeEach);
           }
+          utils.forEach(runnables, function (runnable) {
+            runnable.pending = true;
+          });
         } else {
           self.failHook(hook, err);
 
@@ -567,10 +572,6 @@ Runner.prototype.runTests = function (suite, fn) {
           }
           self.emit('test end', test);
 
-          if (err instanceof Pending) {
-            return next();
-          }
-
           return self.hookUp('afterEach', next);
         }
 

test/integration/regression.spec.js

@@ -5,6 +5,7 @@ var fs = require('fs');
 var path = require('path');
 var run = require('./helpers').runMocha;
 var runJSON = require('./helpers').runMochaJSON;
+var map = require('../../lib/utils').map;
 
 describe('regressions', function () {
   it('issue-1327: should run all 3 specs exactly once', function (done) {
@@ -54,17 +55,118 @@ describe('regressions', function () {
     });
   });
 
-  describe('issue-2286: after doesn\'t execute if test was skipped in beforeEach', function () {
-    var afterWasRun = false;
-    describe('suite with skipped test for meta test', function () {
-      beforeEach(function () { this.skip(); });
-      after(function () { afterWasRun = true; });
-      it('should be pending', function () {});
-    });
-    after('meta test', function () {
-      afterWasRun.should.be.ok();
+  describe(
+    "issue-2286: after doesn't execute if test was skipped in beforeEach",
+    function () {
+      /**
+       * Generates tests for behavior of `this.skip()`
+       * @param {string} name - Name of Runnable abort via `this.skip()`
+       * @param {Array.<boolean>} expected - For each of the Runnable types,
+       *   whether or not the Runnable should have been run.  The order is
+       *   "beforeAll" x 2, "beforeEach" x 2, "test" x 2, "afterEach" x 2, then
+       *   "afterAll" x 2.  There should be 10 items in this array.
+       */
+      function testPendingRunnables (name, expected) {
+        var spies = [];
+
+        function spy (skip) {
+          function wrapped () {
+            wrapped.wasRun = true;
+            if (skip) {
+              this.skip();
+            }
+          }
+
+          wrapped.wasRun = false;
+          spies.push(wrapped);
+          return wrapped;
+        }
+
+        describe(name, function () {
+          describe('meta', function () {
+            before(spy(name === 'beforeAll'));
+            before(spy(name === 'beforeAll'));
+            beforeEach(spy(name === 'beforeEach'));
+            beforeEach(spy(name === 'beforeEach'));
+            it('should be pending', spy(name === 'test'));
+            it('should be pending', spy(name === 'test'));
+            afterEach(spy(name === 'afterEach'));
+            afterEach(spy(name === 'afterEach'));
+            after(spy(name === 'afterAll'));
+            after(spy(name === 'afterAll'));
+          });
+
+          after(name, function () {
+            map(spies, function (spy) {
+              return spy.wasRun;
+            })
+              .should
+              .deepEqual(expected);
+          });
+        });
+      }
+
+      testPendingRunnables('beforeAll', [
+        true, // beforeAll
+        false,
+        false, // beforeEach
+        false,
+        false, // test
+        false,
+        false, // afterEach
+        false,
+        false, // afterAll
+        false
+      ]);
+      testPendingRunnables('beforeEach', [
+        true, // beforeAll
+        true,
+        true, // beforeEach
+        false,
+        false, // test
+        false,
+        false, // afterEach
+        false,
+        true, // afterAll
+        true
+      ]);
+      testPendingRunnables('test', [
+        true, // beforeAll
+        true,
+        true, // beforeEach
+        true,
+        true, // test
+        true,
+        true, // afterEach
+        true,
+        true, // afterAll
+        true
+      ]);
+      testPendingRunnables('afterEach', [
+        true, // beforeAll
+        true,
+        true, // beforeEach
+        true,
+        true, // test
+        false,
+        true, // afterEach
+        true,
+        true, // afterAll
+        true
+      ]);
+      testPendingRunnables('afterAll', [
+        true, // beforeAll
+        true,
+        true, // beforeEach
+        true,
+        true, // test
+        true,
+        true, // afterEach
+        true,
+        true, // afterAll
+        false
+      ]);
     });
-  });
 
   it('issue-2315: cannot read property currentRetry of undefined', function (done) {
     runJSON('regression/issue-2315.js', [], function (err, res) {