Fix up promise handling.

Per comments in #329 we can factor out the calling into its own function. I also reused `this.resetTimeout()` instead of manually setting it, and added the timeout for promise case as well.
mochajs · Mar 14, 2014 · 17cfdee · 17cfdee
1 parent a2968bf
commit 17cfdee
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 23 deletions.
diff --git a/lib/runnable.js b/lib/runnable.js
@@ -168,16 +168,6 @@ Runnable.prototype.run = function(fn){
 
   if (ctx) ctx.runnable(this);
 
-  // timeout
-  if (this.async) {
-    this.timer = setTimeout(function(){
-      if (!finished) {
-        done(new Error('timeout of ' + ms + 'ms exceeded'));
-        self.timedOut = true;
-      }
-    }, ms);
-  }
-
   // called multiple times
   function multiple(err) {
     if (emitted) return;
@@ -198,8 +188,10 @@ Runnable.prototype.run = function(fn){
   // for .resetTimeout()
   this.callback = done;
 
-  // async
+  // explicit async with `done` argument
   if (this.async) {
+    this.resetTimeout();
+
     try {
       this.fn.call(ctx, function(err){
         if (err instanceof Error || toString.call(err) === "[object Error]") return done(err);
@@ -216,24 +208,29 @@ Runnable.prototype.run = function(fn){
     return done(new Error('--async-only option in use without declaring `done()`'));
   }
 
-  // sync
+  // sync or promise-returning
   try {
     if (!this.pending) {
-      var result = this.fn.call(ctx);
-      if (result && typeof result.then === 'function') {
-        result.then(
-          function(){
-            done();
-          },
-          done
-        );
-      } else {
-        done();
-      }
+      callFn(this.fn);
     } else {
       done();
     }
   } catch (err) {
     done(err);
   }
+
+  function callFn(fn) {
+    var result = fn.call(ctx);
+    if (result && typeof result.then === 'function') {
+      self.resetTimeout();
+      result.then(
+        function(){
+          done();
+        },
+        done
+      );
+    } else {
+      done();
+    }
+  }
 };
diff --git a/test/runnable.js b/test/runnable.js
@@ -298,6 +298,24 @@ describe('Runnable(title, fn)', function(){
           });
         })
       })
+
+      describe('when the promise takes too long to settle', function(){
+        var foreverPendingPromise = {
+          then: function () { }
+        };
+
+        it('should give the timeout error', function(done){
+          var test = new Runnable('foo', function(){
+            return foreverPendingPromise;
+          });
+
+          test.timeout(10);
+          test.run(function(err){
+            err.should.be.ok;
+            done();
+          });
+        })
+      })
     })
 
     describe('when fn returns a non-promise', function(){