Deprecate non-Error arguments passed to done()

[Finishes #178267600]

lib/jasmine-core/jasmine.js

@@ -861,10 +861,11 @@ getJasmineRequireObj().Spec = function(j$) {
         self.onException.apply(self, arguments);
       },
       onComplete: function() {
-        onComplete(
+        if (self.result.status === 'failed') {
-          self.result.status === 'failed' &&
+          onComplete(new j$.StopExecutionError('spec failed'));
-            new j$.StopExecutionError('spec failed')
+        } else {
-        );
+          onComplete();
+        }
       },
       userContext: this.userContext()
     };
@@ -3863,7 +3864,6 @@ getJasmineRequireObj().deprecatingSuiteProxy = function(j$) {
   };
 
   DeprecatingSuiteProxyHandler.prototype.set = function(target, prop, value) {
-    debugger;
     this._maybeDeprecate(target, prop);
     return (target[prop] = value);
   };
@@ -8176,6 +8176,12 @@ getJasmineRequireObj().QueueRunner = function(j$) {
             self.fail(err);
           }
           self.errored = errored = true;
+        } else if (typeof err !== 'undefined' && !self.errored) {
+          self.deprecated(
+            'Any argument passed to a done callback will be treated as an ' +
+              'error in a future release. Call the done callback without ' +
+              "arguments if you don't want to trigger a spec failure."
+          );
         }
 
         function runNext() {
@@ -8281,7 +8287,12 @@ getJasmineRequireObj().QueueRunner = function(j$) {
 
     this.clearStack(function() {
       self.globalErrors.popListener(self.handleFinalError);
-      self.onComplete(self.errored && new StopExecutionError());
+
+      if (self.errored) {
+        self.onComplete(new StopExecutionError());
+      } else {
+        self.onComplete();
+      }
     });
   };
 

spec/core/QueueRunnerSpec.js

@@ -149,31 +149,111 @@ 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() {
+    describe('When next is called with an argument', function() {
-      var err = new Error('foo'),
+      describe('that is an Error', function() {
-        queueableFn1 = {
+        it('explicitly fails and moves to the next function', function() {
-          fn: function(done) {
+          var err = new Error('foo'),
-            setTimeout(function() {
+            queueableFn1 = {
-              done(err);
+              fn: function(done) {
-            }, 100);
+                setTimeout(function() {
-          }
+                  done(err);
-        },
+                }, 100);
-        queueableFn2 = { fn: jasmine.createSpy('fn2') },
+              }
-        failFn = jasmine.createSpy('fail'),
+            },
-        queueRunner = new jasmineUnderTest.QueueRunner({
+            queueableFn2 = { fn: jasmine.createSpy('fn2') },
-          queueableFns: [queueableFn1, queueableFn2],
+            failFn = jasmine.createSpy('fail'),
-          fail: failFn
+            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();
         });
 
-      queueRunner.execute();
+        it('does not log a deprecation', function() {
+          var err = new Error('foo'),
+            queueableFn1 = {
+              fn: function(done) {
+                setTimeout(function() {
+                  done(err);
+                }, 100);
+              }
+            },
+            deprecated = jasmine.createSpy('deprecated'),
+            queueRunner = new jasmineUnderTest.QueueRunner({
+              queueableFns: [queueableFn1],
+              deprecated: deprecated
+            });
 
-      expect(failFn).not.toHaveBeenCalled();
+          queueRunner.execute();
-      expect(queueableFn2.fn).not.toHaveBeenCalled();
 
-      jasmine.clock().tick(100);
+          jasmine.clock().tick(100);
 
-      expect(failFn).toHaveBeenCalledWith(err);
+          expect(deprecated).not.toHaveBeenCalled();
-      expect(queueableFn2.fn).toHaveBeenCalled();
+        });
+      });
+
+      describe('that is not an Error', function() {
+        it('logs a deprecation', function() {
+          var queueableFn1 = {
+              fn: function(done) {
+                setTimeout(function() {
+                  done('not an Error');
+                }, 100);
+              }
+            },
+            deprecated = jasmine.createSpy('deprecated'),
+            queueRunner = new jasmineUnderTest.QueueRunner({
+              queueableFns: [queueableFn1],
+              deprecated: deprecated
+            });
+
+          queueRunner.execute();
+
+          jasmine.clock().tick(100);
+
+          expect(deprecated).toHaveBeenCalledWith(
+            'Any argument passed to a done callback will be treated as an ' +
+              'error in a future release. Call the done callback without ' +
+              "arguments if you don't want to trigger a spec failure."
+          );
+        });
+
+        it('moves to the next function without failing', function() {
+          var queueableFn1 = {
+              fn: function(done) {
+                setTimeout(function() {
+                  done('not an Error');
+                }, 100);
+              }
+            },
+            queueableFn2 = { fn: jasmine.createSpy('fn2') },
+            failFn = jasmine.createSpy('fail'),
+            queueRunner = new jasmineUnderTest.QueueRunner({
+              queueableFns: [queueableFn1, queueableFn2],
+              fail: failFn,
+              deprecated: function() {}
+            });
+
+          queueRunner.execute();
+
+          expect(failFn).not.toHaveBeenCalled();
+          expect(queueableFn2.fn).not.toHaveBeenCalled();
+
+          jasmine.clock().tick(100);
+
+          expect(failFn).not.toHaveBeenCalled();
+          expect(queueableFn2.fn).toHaveBeenCalled();
+        });
+      });
     });
 
     it('does not cause an explicit fail if execution is being stopped', function() {
@@ -523,8 +603,7 @@ describe('QueueRunner', function() {
         queueRunner = new jasmineUnderTest.QueueRunner({
           queueableFns: [queueableFn],
           deprecated: deprecated
-        }),
+        });
-        env = jasmineUnderTest.getEnv();
 
       queueRunner.execute();
 

spec/core/integration/CustomAsyncMatchersSpec.js

@@ -161,7 +161,12 @@ describe('Custom Async Matchers (Integration)', function() {
       };
 
       spyOn(env, 'deprecated');
-      env.addReporter({ specDone: specExpectations, jasmineDone: done });
+      env.addReporter({
+        specDone: specExpectations,
+        jasmineDone: function() {
+          done();
+        }
+      });
       env.execute();
     });
   });

spec/core/integration/CustomMatchersSpec.js

@@ -313,7 +313,12 @@ describe('Custom Matchers (Integration)', function() {
         );
       };
 
-      env.addReporter({ specDone: specExpectations, jasmineDone: done });
+      env.addReporter({
+        specDone: specExpectations,
+        jasmineDone: function() {
+          done();
+        }
+      });
       env.execute();
     });
   });

src/core/QueueRunner.js

@@ -105,6 +105,12 @@ getJasmineRequireObj().QueueRunner = function(j$) {
             self.fail(err);
           }
           self.errored = errored = true;
+        } else if (typeof err !== 'undefined' && !self.errored) {
+          self.deprecated(
+            'Any argument passed to a done callback will be treated as an ' +
+              'error in a future release. Call the done callback without ' +
+              "arguments if you don't want to trigger a spec failure."
+          );
         }
 
         function runNext() {
@@ -210,7 +216,12 @@ getJasmineRequireObj().QueueRunner = function(j$) {
 
     this.clearStack(function() {
       self.globalErrors.popListener(self.handleFinalError);
-      self.onComplete(self.errored && new StopExecutionError());
+
+      if (self.errored) {
+        self.onComplete(new StopExecutionError());
+      } else {
+        self.onComplete();
+      }
     });
   };
 

src/core/Spec.js

@@ -129,10 +129,11 @@ getJasmineRequireObj().Spec = function(j$) {
         self.onException.apply(self, arguments);
       },
       onComplete: function() {
-        onComplete(
+        if (self.result.status === 'failed') {
-          self.result.status === 'failed' &&
+          onComplete(new j$.StopExecutionError('spec failed'));
-            new j$.StopExecutionError('spec failed')
+        } else {
-        );
+          onComplete();
+        }
       },
       userContext: this.userContext()
     };

src/core/deprecatingSuiteProxy.js

@@ -65,7 +65,6 @@ getJasmineRequireObj().deprecatingSuiteProxy = function(j$) {
   };
 
   DeprecatingSuiteProxyHandler.prototype.set = function(target, prop, value) {
-    debugger;
     this._maybeDeprecate(target, prop);
     return (target[prop] = value);
   };