Fix hook pattern of this.skip() in it() tests (#3859)

mochajs · Aug 11, 2019 · 7297b0a · 7297b0a
1 parent 6d7c32a
commit 7297b0a
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 19 deletions.
diff --git a/lib/runner.js b/lib/runner.js
@@ -673,11 +673,6 @@ Runner.prototype.runTests = function(suite, fn) {
             self.fail(test, err);
           }
           self.emit(constants.EVENT_TEST_END, test);
-
-          if (err instanceof Pending) {
-            return next();
-          }
-
           return self.hookUp(HOOK_TYPE_AFTER_EACH, next);
         }
 

diff --git a/test/integration/fixtures/pending/skip-async-spec.fixture.js b/test/integration/fixtures/pending/skip-async-spec.fixture.js
@@ -1,12 +1,30 @@
 'use strict';
+var assert = require('assert');
 
 describe('skip in test', function () {
+  var runOrder = [];
+  beforeEach(function () {
+    runOrder.push('beforeEach');
+  });
+
   it('should skip async', function (done) {
     var self = this;
     setTimeout(function () {
       self.skip();   // done() is not required
     }, 0);
   });
+  it('should run other tests in suite', function () {});
 
-  it('should run other tests in the suite', function () {});
+  afterEach(function() {
+    runOrder.push('afterEach');
+  });
+  after(function() {
+    runOrder.push('after');
+    assert.deepStrictEqual(runOrder, [
+      'beforeEach', 'afterEach',
+      'beforeEach', 'afterEach',
+      'after'
+    ]);
+    throw new Error('should throw this error');
+  });
 });
diff --git a/test/integration/fixtures/pending/skip-sync-spec.fixture.js b/test/integration/fixtures/pending/skip-sync-spec.fixture.js
@@ -1,10 +1,28 @@
 'use strict';
+var assert = require('assert');
 
 describe('skip in test', function () {
+  var runOrder = [];
+  beforeEach(function () {
+    runOrder.push('beforeEach');
+  });
+
   it('should skip immediately', function () {
     this.skip();
     throw new Error('never run this test');
   });
+  it('should run other tests in suite', function () {});
 
-  it('should run other tests in the suite', function () {});
+  afterEach(function() {
+    runOrder.push('afterEach');
+  });
+  after(function() {
+    runOrder.push('after');
+    assert.deepStrictEqual(runOrder, [
+      'beforeEach', 'afterEach',
+      'beforeEach', 'afterEach',
+      'after'
+    ]);
+    throw new Error('should throw this error');
+  });
 });
diff --git a/test/integration/pending.spec.js b/test/integration/pending.spec.js
@@ -59,13 +59,14 @@ describe('pending', function() {
       it('should immediately skip the spec and run all others', function(done) {
         run('pending/skip-sync-spec.fixture.js', args, function(err, res) {
           if (err) {
-            done(err);
-            return;
+            return done(err);
           }
-          assert.strictEqual(res.stats.pending, 1);
-          assert.strictEqual(res.stats.passes, 1);
-          assert.strictEqual(res.stats.failures, 0);
-          assert.strictEqual(res.code, 0);
+          expect(res, 'to have failed with error', 'should throw this error')
+            .and('to have failed test count', 1)
+            .and('to have pending test count', 1)
+            .and('to have pending test order', 'should skip immediately')
+            .and('to have passed test count', 1)
+            .and('to have passed tests', 'should run other tests in suite');
           done();
         });
       });
@@ -192,13 +193,14 @@ describe('pending', function() {
       it('should immediately skip the spec and run all others', function(done) {
         run('pending/skip-async-spec.fixture.js', args, function(err, res) {
           if (err) {
-            done(err);
-            return;
+            return done(err);
           }
-          assert.strictEqual(res.stats.pending, 1);
-          assert.strictEqual(res.stats.passes, 1);
-          assert.strictEqual(res.stats.failures, 0);
-          assert.strictEqual(res.code, 0);
+          expect(res, 'to have failed with error', 'should throw this error')
+            .and('to have failed test count', 1)
+            .and('to have pending test count', 1)
+            .and('to have pending test order', 'should skip async')
+            .and('to have passed test count', 1)
+            .and('to have passed tests', 'should run other tests in suite');
           done();
         });
       });