From 80fc8905de0a6a0a4622483c69b6fb1eaa860f8d Mon Sep 17 00:00:00 2001
From: jjoos <jan@deelstra.org>
Date: Fri, 23 Oct 2015 10:59:46 +0200
Subject: [PATCH] (re-)enable after each hooks to fail tests.

In version 1.2.1, support was added to fail test in the after each hook:
>* Added `this.test.error(err)` support to after each hooks. Closes #287

While migrating from teaspoon-mocha to karma-mocha, we discovered that
in the current mocha version this isn't supported anymore. That
behaviour has been changed in: ebc6aee50dd1fdca9ab940b43ab79db72f0a4f93
without any documentation change.

If this was a conscious decision adding some documentation about
dropping this functionality would then be nice. The old behaviour is
documented in:
https://github.com/mochajs/mocha/wiki/Conditionally-failing-tests-in-afterEach()-hooks
.
---
 lib/runner.js                                 | 31 ++++++++++---------
 .../fixtures/hooks/afterEach.hook.failure.js  | 16 ++++++++++
 test/integration/hooks.js                     | 17 ++++++++++
 test/integration/multiple.done.js             |  2 +-
 4 files changed, 51 insertions(+), 15 deletions(-)
 create mode 100644 test/integration/fixtures/hooks/afterEach.hook.failure.js

diff --git a/lib/runner.js b/lib/runner.js
index ba4d7df97c..6b672b3159 100644
--- a/lib/runner.js
+++ b/lib/runner.js
@@ -532,7 +532,10 @@ Runner.prototype.runTests = function(suite, fn) {
           if (err instanceof Pending) {
             test.pending = true;
             self.emit('pending', test);
-          } else if (retry < test.retries()) {
+            return next();
+          }
+
+          if (retry < test.retries()) {
             var clonedTest = test.clone();
             clonedTest.currentRetry(retry + 1);
             tests.unshift(clonedTest);
@@ -540,22 +543,22 @@ Runner.prototype.runTests = function(suite, fn) {
             // Early return + hook trigger so that it doesn't
             // increment the count wrong
             return self.hookUp('afterEach', next);
-          } else {
-            self.fail(test, err);
           }
-          self.emit('test end', test);
 
-          if (err instanceof Pending) {
-            return next();
-          }
-
-          return self.hookUp('afterEach', next);
+          self.fail(test, err);
+        } else {
+          test.state = 'passed';
         }
-
-        test.state = 'passed';
-        self.emit('pass', test);
-        self.emit('test end', test);
-        self.hookUp('afterEach', next);
+        // Run the actual emit that the test passed after the forEach hooks,
+        // so they can still fail a test. And it's not already reported as
+        // passed.
+        self.hookUp('afterEach', function(err, errSuite) {
+          if (test.state === 'passed') {
+            self.emit('pass', test);
+          }
+          self.emit('test end', test);
+          next(err, errSuite);
+        });
       });
     });
   }
diff --git a/test/integration/fixtures/hooks/afterEach.hook.failure.js b/test/integration/fixtures/hooks/afterEach.hook.failure.js
new file mode 100644
index 0000000000..408e1ad9e1
--- /dev/null
+++ b/test/integration/fixtures/hooks/afterEach.hook.failure.js
@@ -0,0 +1,16 @@
+describe('spec 1', function () {
+  afterEach(function () {
+    this.test.error(new Error('something went wrong'));
+  });
+  it('should be set to failure in the afterEach', function () {
+    console.log('test 1');
+  });
+  it('should be set to failure in the afterEach', function () {
+    console.log('test 2');
+  });
+});
+describe('spec 2', function () {
+  it('should succeed', function () {
+    console.log('test 3');
+  });
+});
diff --git a/test/integration/hooks.js b/test/integration/hooks.js
index 4236c7f82d..376e536bcf 100644
--- a/test/integration/hooks.js
+++ b/test/integration/hooks.js
@@ -41,4 +41,21 @@ describe('hooks', function() {
       done();
     });
   });
+
+  describe('afterEach sets tests to fail', function() {
+    var res;
+    before(function(done) {
+      run('hooks/afterEach.hook.failure.js', args, function(err, result) {
+        res = result;
+        done(err);
+      });
+    });
+
+    it('afterEach can set a test to fail', function() {
+      assert.equal(res.pending, 0);
+      assert.equal(res.passing, 1);
+      assert.equal(res.failing, 2);
+      assert.equal(res.code, 2);
+    });
+  });
 });
diff --git a/test/integration/multiple.done.js b/test/integration/multiple.done.js
index 8bcedf6332..ae86ea2478 100644
--- a/test/integration/multiple.done.js
+++ b/test/integration/multiple.done.js
@@ -16,7 +16,7 @@ describe('multiple calls to done()', function() {
 
     it('results in failures', function() {
       assert.equal(res.stats.pending, 0);
-      assert.equal(res.stats.passes, 1);
+      assert.equal(res.stats.passes, 0);
       assert.equal(res.stats.failures, 1);
       assert.equal(res.code, 1);
     });