Detect an Error passed to done and add an expectation failure

- See #567

lib/jasmine-core/jasmine.js

@@ -218,6 +218,17 @@ getJasmineRequireObj().base = function(j$, jasmineGlobal) {
     return j$.getType_(value) === '[object ' + typeName + ']';
   };
 
+  j$.isError_ = function(value) {
+    if (value instanceof Error) {
+      return true;
+    }
+    if (value && value.constructor && value.constructor.constructor &&
+      (value instanceof (value.constructor.constructor('return this')()).Error)) {
+      return true;
+    }
+    return false;
+  };
+
   j$.getType_ = function(value) {
     return Object.prototype.toString.apply(value);
   };
@@ -3846,7 +3857,7 @@ getJasmineRequireObj().toThrowError = function(j$) {
           thrown = e;
         }
 
-        if (!isErrorObject(thrown)) {
+        if (!j$.isError_(thrown)) {
           return fail(function() { return 'Expected function to throw an Error, but it threw ' + j$.pp(thrown) + '.'; });
         }
 
@@ -3958,18 +3969,7 @@ getJasmineRequireObj().toThrowError = function(j$) {
 
       var Surrogate = function() {};
       Surrogate.prototype = type.prototype;
-      return isErrorObject(new Surrogate());
+      return j$.isError_(new Surrogate());
-    }
-
-    function isErrorObject(thrown) {
-      if (thrown instanceof Error) {
-        return true;
-      }
-      if (thrown && thrown.constructor && thrown.constructor.constructor &&
-          (thrown instanceof (thrown.constructor.constructor('return this')()).Error)) {
-        return true;
-      }
-      return false;
     }
   }
 
@@ -4453,7 +4453,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     return function() {
       if (!called) {
         called = true;
-        fn();
+        fn.apply(null, arguments);
       }
       return null;
     };
@@ -4507,15 +4507,20 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     var self = this, completedSynchronously = true,
       handleError = function handleError(error) {
         onException(error);
-        next();
+        next(error);
       },
       cleanup = once(function cleanup() {
         self.clearTimeout(timeoutId);
         self.globalErrors.popListener(handleError);
       }),
-      next = once(function next() {
+      next = once(function next(err) {
         cleanup();
 
+        if (j$.isError_(err)) {
+          self.fail(err);
+          errored = true;
+        }
+
         function runNext() {
           if (self.completeOnFirstError && errored) {
             self.skipToCleanup(iterativeIndex);

spec/core/QueueRunnerSpec.js

@@ -135,6 +135,29 @@ describe("QueueRunner", function() {
       expect(queueableFn2.fn).toHaveBeenCalled();
     });
 
+    it("explicitly fails an async function when next is called with an Error and moves to the next function", function() {
+      var err = new Error('foo'),
+        queueableFn1 = { fn: function(done) {
+          setTimeout(function() { done(err); }, 100);
+        } },
+        queueableFn2 = { fn: jasmine.createSpy('fn2') },
+        failFn = jasmine.createSpy('fail'),
+        queueRunner = new jasmineUnderTest.QueueRunner({
+          queueableFns: [queueableFn1, queueableFn2],
+          fail: failFn
+        });
+
+      queueRunner.execute();
+
+      expect(failFn).not.toHaveBeenCalled();
+      expect(queueableFn2.fn).not.toHaveBeenCalled();
+
+      jasmine.clock().tick(100);
+
+      expect(failFn).toHaveBeenCalledWith(err);
+      expect(queueableFn2.fn).toHaveBeenCalled();
+    });
+
     it("sets a timeout if requested for asynchronous functions so they don't go on forever", function() {
       var timeout = 3,
         beforeFn = { fn: function(done) { }, type: 'before', timeout: function() { return timeout; } },
@@ -522,6 +545,24 @@ describe("QueueRunner", function() {
         expect(nextQueueableFn.fn).not.toHaveBeenCalled();
         expect(cleanupFn.fn).toHaveBeenCalled();
       });
+
+      it("skips to cleanup functions when next is called with an Error", function() {
+        var queueableFn = { fn: function(done) {
+            done(new Error('nope'));
+          } },
+          nextQueueableFn = { fn: jasmine.createSpy('nextFunction') },
+          cleanupFn = { fn: jasmine.createSpy('cleanup') },
+          queueRunner = new jasmineUnderTest.QueueRunner({
+            queueableFns: [queueableFn, nextQueueableFn],
+            cleanupFns: [cleanupFn],
+            completeOnFirstError: true,
+         });
+
+        queueRunner.execute();
+        jasmine.clock().tick();
+        expect(nextQueueableFn.fn).not.toHaveBeenCalled();
+        expect(cleanupFn.fn).toHaveBeenCalled();
+      });
     });
   });
 

spec/core/integration/EnvSpec.js

@@ -1205,30 +1205,21 @@ describe("Env integration", function() {
       env.addReporter({
         specDone: specDone,
         jasmineDone: function() {
-          expect(specDone).toHaveBeenCalledWith(jasmine.objectContaining({
+          expect(specDone).toHaveFailedExpectationsForRunnable('failing has a default message',
-            description: 'has a default message',
+            ['Failed']
-            failedExpectations: [jasmine.objectContaining({
+          );
-              message: 'Failed'
+          expect(specDone).toHaveFailedExpectationsForRunnable('failing specifies a message',
-            })]
+            ['Failed: messy message']
-          }));
+          );
-          expect(specDone).toHaveBeenCalledWith(jasmine.objectContaining({
+          expect(specDone).toHaveFailedExpectationsForRunnable('failing fails via the done callback',
-            description: 'specifies a message',
+            ['Failed: done failed']
-            failedExpectations: [jasmine.objectContaining({
+          );
-              message: 'Failed: messy message'
+          expect(specDone).toHaveFailedExpectationsForRunnable('failing has a message from an Error',
-            })]
+            ['Failed: error message']
-          }));
+          );
-          expect(specDone).toHaveBeenCalledWith(jasmine.objectContaining({
+          expect(specDone).toHaveFailedExpectationsForRunnable('failing has a message from an Error to done',
-            description: 'fails via the done callback',
+            ['Failed: done error']
-            failedExpectations: [jasmine.objectContaining({
+          );
-              message: 'Failed: done failed'
-            })]
-          }));
-          expect(specDone).toHaveBeenCalledWith(jasmine.objectContaining({
-            description: 'has a message from an Error',
-            failedExpectations: [jasmine.objectContaining({
-              message: 'Failed: error message'
-            })]
-          }));
 
           jasmine.clock().tick(1);
           realSetTimeout(done);
@@ -1274,6 +1265,15 @@ describe("Env integration", function() {
           jasmine.clock().tick(1);
           jasmine.clock().tick(1);
         });
+
+        env.it('has a message from an Error to done', function(innerDone) {
+          setTimeout(function() {
+            innerDone(new Error('done error'));
+          }, 1);
+          jasmine.clock().tick(1);
+          jasmine.clock().tick(1);
+          jasmine.clock().tick(1);
+        });
       });
 
       env.execute();

src/core/QueueRunner.js

@@ -5,7 +5,7 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     return function() {
       if (!called) {
         called = true;
-        fn();
+        fn.apply(null, arguments);
       }
       return null;
     };
@@ -59,15 +59,20 @@ getJasmineRequireObj().QueueRunner = function(j$) {
     var self = this, completedSynchronously = true,
       handleError = function handleError(error) {
         onException(error);
-        next();
+        next(error);
       },
       cleanup = once(function cleanup() {
         self.clearTimeout(timeoutId);
         self.globalErrors.popListener(handleError);
       }),
-      next = once(function next() {
+      next = once(function next(err) {
         cleanup();
 
+        if (j$.isError_(err)) {
+          self.fail(err);
+          errored = true;
+        }
+
         function runNext() {
           if (self.completeOnFirstError && errored) {
             self.skipToCleanup(iterativeIndex);

src/core/base.js

@@ -85,6 +85,17 @@ getJasmineRequireObj().base = function(j$, jasmineGlobal) {
     return j$.getType_(value) === '[object ' + typeName + ']';
   };
 
+  j$.isError_ = function(value) {
+    if (value instanceof Error) {
+      return true;
+    }
+    if (value && value.constructor && value.constructor.constructor &&
+      (value instanceof (value.constructor.constructor('return this')()).Error)) {
+      return true;
+    }
+    return false;
+  };
+
   j$.getType_ = function(value) {
     return Object.prototype.toString.apply(value);
   };

src/core/matchers/toThrowError.js

@@ -32,7 +32,7 @@ getJasmineRequireObj().toThrowError = function(j$) {
           thrown = e;
         }
 
-        if (!isErrorObject(thrown)) {
+        if (!j$.isError_(thrown)) {
           return fail(function() { return 'Expected function to throw an Error, but it threw ' + j$.pp(thrown) + '.'; });
         }
 
@@ -144,18 +144,7 @@ getJasmineRequireObj().toThrowError = function(j$) {
 
       var Surrogate = function() {};
       Surrogate.prototype = type.prototype;
-      return isErrorObject(new Surrogate());
+      return j$.isError_(new Surrogate());
-    }
-
-    function isErrorObject(thrown) {
-      if (thrown instanceof Error) {
-        return true;
-      }
-      if (thrown && thrown.constructor && thrown.constructor.constructor &&
-          (thrown instanceof (thrown.constructor.constructor('return this')()).Error)) {
-        return true;
-      }
-      return false;
     }
   }