fix before pending hook pattern

mochajs · Apr 8, 2020 · cdda8c7 · cdda8c7
1 parent 54475eb
commit cdda8c7
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 31 deletions.
diff --git a/lib/runnable.js b/lib/runnable.js
@@ -295,6 +295,8 @@ Runnable.prototype.run = function(fn) {
   var finished;
   var emitted;
 
+  if (this.isPending()) return fn();
+
   // Sometimes the ctx exists, but it is not runnable
   if (ctx && ctx.runnable) {
     ctx.runnable(this);
@@ -376,11 +378,7 @@ Runnable.prototype.run = function(fn) {
 
   // sync or promise-returning
   try {
-    if (this.isPending()) {
-      done();
-    } else {
-      callFn(this.fn);
-    }
+    callFn(this.fn);
   } catch (err) {
     emitted = true;
     if (err instanceof Pending) {
@@ -481,7 +479,11 @@ var constants = utils.defineConstants(
     /**
      * Value of `state` prop when a `Runnable` has passed
      */
-    STATE_PASSED: 'passed'
+    STATE_PASSED: 'passed',
+    /**
+     * Value of `state` prop when a `Runnable` has been skipped by user
+     */
+    STATE_PENDING: 'pending'
   }
 );
 

diff --git a/lib/runner.js b/lib/runner.js
@@ -18,6 +18,7 @@ var HOOK_TYPE_BEFORE_ALL = Suite.constants.HOOK_TYPE_BEFORE_ALL;
 var EVENT_ROOT_SUITE_RUN = Suite.constants.EVENT_ROOT_SUITE_RUN;
 var STATE_FAILED = Runnable.constants.STATE_FAILED;
 var STATE_PASSED = Runnable.constants.STATE_PASSED;
+var STATE_PENDING = Runnable.constants.STATE_PENDING;
 var dQuote = utils.dQuote;
 var ngettext = utils.ngettext;
 var sQuote = utils.sQuote;
@@ -122,8 +123,7 @@ module.exports = Runner;
  * @public
  * @class
  * @param {Suite} suite Root suite
- * @param {boolean} [delay] Whether or not to delay execution of root suite
- * until ready.
+ * @param {boolean} [delay] Whether to delay execution of root suite until ready.
  */
 function Runner(suite, delay) {
   var self = this;
@@ -285,11 +285,13 @@ Runner.prototype.checkGlobals = function(test) {
  * Fail the given `test`.
  *
  * @private
- * @param {Test} test
+ * @param {Runnable} test
  * @param {Error} err
+ * @param {boolean} [force=false] - Whether to fail a pending test.
  */
-Runner.prototype.fail = function(test, err) {
-  if (test.isPending()) {
+Runner.prototype.fail = function(test, err, force) {
+  force = force === true;
+  if (test.isPending() && !force) {
     return;
   }
 
@@ -386,7 +388,7 @@ Runner.prototype.hook = function(name, fn) {
       });
     }
 
-    hook.run(function(err) {
+    hook.run(function cbHookRun(err) {
       var testError = hook.error();
       if (testError) {
         self.fail(self.test, testError);
@@ -412,6 +414,7 @@ Runner.prototype.hook = function(name, fn) {
           suite.suites.forEach(function(suite) {
             suite.pending = true;
           });
+          hooks = [];
         } else {
           hook.pending = false;
           var errForbid = createUnsupportedError('`this.skip` forbidden');
@@ -533,9 +536,6 @@ Runner.prototype.runTest = function(fn) {
     test.asyncOnly = true;
   }
   test.on('error', function(err) {
-    if (err instanceof Pending) {
-      return;
-    }
     self.fail(test, err);
   });
   if (this.allowUncaught) {
@@ -634,10 +634,9 @@ Runner.prototype.runTests = function(suite, fn) {
     // static skip, no hooks are executed
     if (test.isPending()) {
       if (self.forbidPending) {
-        test.isPending = alwaysFalse;
-        self.fail(test, new Error('Pending test forbidden'));
-        delete test.isPending;
+        self.fail(test, new Error('Pending test forbidden'), true);
       } else {
+        test.state = STATE_PENDING;
         self.emit(constants.EVENT_TEST_PENDING, test);
       }
       self.emit(constants.EVENT_TEST_END, test);
@@ -650,10 +649,9 @@ Runner.prototype.runTests = function(suite, fn) {
       // conditional skip within beforeEach
       if (test.isPending()) {
         if (self.forbidPending) {
-          test.isPending = alwaysFalse;
-          self.fail(test, new Error('Pending test forbidden'));
-          delete test.isPending;
+          self.fail(test, new Error('Pending test forbidden'), true);
         } else {
+          test.state = STATE_PENDING;
           self.emit(constants.EVENT_TEST_PENDING, test);
         }
         self.emit(constants.EVENT_TEST_END, test);
@@ -674,10 +672,9 @@ Runner.prototype.runTests = function(suite, fn) {
         // conditional skip within it
         if (test.pending) {
           if (self.forbidPending) {
-            test.isPending = alwaysFalse;
-            self.fail(test, new Error('Pending test forbidden'));
-            delete test.isPending;
+            self.fail(test, new Error('Pending test forbidden'), true);
           } else {
+            test.state = STATE_PENDING;
             self.emit(constants.EVENT_TEST_PENDING, test);
           }
           self.emit(constants.EVENT_TEST_END, test);
@@ -714,10 +711,6 @@ Runner.prototype.runTests = function(suite, fn) {
   next();
 };
 
-function alwaysFalse() {
-  return false;
-}
-
 /**
  * Run the given `suite` and invoke the callback `fn()` when complete.
  *
@@ -850,9 +843,7 @@ Runner.prototype.uncaught = function(err) {
     return;
   } else if (runnable.isPending()) {
     // report 'pending test' retrospectively as failed
-    runnable.isPending = alwaysFalse;
-    this.fail(runnable, err);
-    delete runnable.isPending;
+    this.fail(runnable, err, true);
     return;
   }