Refactor Spec and QueueRunner [#62585700]

- QueueRunner now responsible for timing out async specs instead of
   Spec
 - Make sure only spec functions are timeoutable and not suites (due to
   the refactor)

lib/jasmine-core/jasmine.js

@@ -46,7 +46,7 @@ getJasmineRequireObj().core = function(jRequire) {
   j$.matchersUtil = jRequire.matchersUtil(j$);
   j$.ObjectContaining = jRequire.ObjectContaining(j$);
   j$.pp = jRequire.pp(j$);
-  j$.QueueRunner = jRequire.QueueRunner();
+  j$.QueueRunner = jRequire.QueueRunner(j$);
   j$.ReportDispatcher = jRequire.ReportDispatcher();
   j$.Spec = jRequire.Spec(j$);
   j$.SpyStrategy = jRequire.SpyStrategy();
@@ -242,8 +242,6 @@ getJasmineRequireObj().Spec = function(j$) {
     this.queueRunnerFactory = attrs.queueRunnerFactory || function() {};
     this.catchingExceptions = attrs.catchingExceptions || function() { return true; };
 
-    this.timer = attrs.timer || {setTimeout: setTimeout, clearTimeout: clearTimeout};
-
     if (!this.fn) {
       this.pend();
     }
@@ -268,8 +266,7 @@ getJasmineRequireObj().Spec = function(j$) {
   };
 
   Spec.prototype.execute = function(onComplete) {
-    var self = this,
+    var self = this;
-        timeout;
 
     this.onStart(this);
 
@@ -278,42 +275,16 @@ getJasmineRequireObj().Spec = function(j$) {
       return;
     }
 
-    function timeoutable(fn) {
+    var allFns = this.beforeFns().concat(this.fn).concat(this.afterFns());
-      return function(done) {
-        timeout = Function.prototype.apply.apply(self.timer.setTimeout, [j$.getGlobal(), [function() {
-          onException(new Error('Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.'));
-          done();
-        }, j$.DEFAULT_TIMEOUT_INTERVAL]]);
-
-        var callDone = function() {
-          clearTimeoutable();
-          done();
-        };
-
-        fn.call(this, callDone); //TODO: do we care about more than 1 arg?
-      };
-    }
-
-    function clearTimeoutable() {
-      Function.prototype.apply.apply(self.timer.clearTimeout, [j$.getGlobal(), [timeout]]);
-      timeout = void 0;
-    }
-
-    var allFns = this.beforeFns().concat(this.fn).concat(this.afterFns()),
-      allTimeoutableFns = [];
-    for (var i = 0; i < allFns.length; i++) {
-      var fn = allFns[i];
-      allTimeoutableFns.push(fn.length > 0 ? timeoutable(fn) : fn);
-    }
 
     this.queueRunnerFactory({
-      fns: allTimeoutableFns,
+      fns: allFns,
       onException: onException,
-      onComplete: complete
+      onComplete: complete,
+      enforceTimeout: function() { return true; }
     });
 
     function onException(e) {
-      clearTimeoutable();
       if (Spec.isPendingSpecException(e)) {
         self.pend();
         return;
@@ -517,6 +488,7 @@ getJasmineRequireObj().Env = function(j$) {
     var queueRunnerFactory = function(options) {
       options.catchException = catchException;
       options.clearStack = options.clearStack || clearStack;
+      options.timer = {setTimeout: realSetTimeout, clearTimeout: realClearTimeout};
 
       new j$.QueueRunner(options).execute();
     };
@@ -652,8 +624,7 @@ getJasmineRequireObj().Env = function(j$) {
         description: description,
         expectationResultFactory: expectationResultFactory,
         queueRunnerFactory: queueRunnerFactory,
-        fn: fn,
+        fn: fn
-        timer: {setTimeout: realSetTimeout, clearTimeout: realClearTimeout}
       });
 
       runnableLookupTable[spec.id] = spec;
@@ -1483,7 +1454,7 @@ getJasmineRequireObj().pp = function(j$) {
   };
 };
 
-getJasmineRequireObj().QueueRunner = function() {
+getJasmineRequireObj().QueueRunner = function(j$) {
 
   function QueueRunner(attrs) {
     this.fns = attrs.fns || [];
@@ -1491,7 +1462,9 @@ getJasmineRequireObj().QueueRunner = function() {
     this.clearStack = attrs.clearStack || function(fn) {fn();};
     this.onException = attrs.onException || function() {};
     this.catchException = attrs.catchException || function() { return true; };
+    this.enforceTimeout = attrs.enforceTimeout || function() { return false; };
     this.userContext = {};
+    this.timer = attrs.timeout || {setTimeout: setTimeout, clearTimeout: clearTimeout};
   }
 
   QueueRunner.prototype.execute = function() {
@@ -1527,7 +1500,21 @@ getJasmineRequireObj().QueueRunner = function() {
     }
 
     function attemptAsync(fn) {
-      var next = function () { self.run(fns, iterativeIndex + 1); };
+      var clearTimeout = function () {
+          Function.prototype.apply.apply(self.timer.clearTimeout, [j$.getGlobal(), [timeoutId]]);
+        },
+        next = function () {
+          clearTimeout(timeoutId);
+          self.run(fns, iterativeIndex + 1);
+        },
+        timeoutId;
+
+      if (self.enforceTimeout()) {
+        timeoutId = Function.prototype.apply.apply(self.timer.setTimeout, [j$.getGlobal(), [function() {
+          self.onException(new Error('Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.'));
+          next();
+        }, j$.DEFAULT_TIMEOUT_INTERVAL]]);
+      }
 
       try {
         fn.call(self.userContext, next);

spec/core/QueueRunnerSpec.js

@@ -55,21 +55,15 @@ describe("QueueRunner", function() {
         afterCallback = jasmine.createSpy('afterCallback'),
         fn1 = function(done) {
           beforeCallback();
-          setTimeout(function() {
+          setTimeout(done, 100);
-            done()
-          }, 100);
         },
         fn2 = function(done) {
           fnCallback();
-          setTimeout(function() {
+          setTimeout(done, 100);
-            done()
-          }, 100);
         },
         fn3 = function(done) {
           afterCallback();
-          setTimeout(function() {
+          setTimeout(done, 100);
-            done()
-          }, 100);
         },
         queueRunner = new j$.QueueRunner({
           fns: [fn1, fn2, fn3],
@@ -98,6 +92,86 @@ describe("QueueRunner", function() {
 
       expect(onComplete).toHaveBeenCalled();
     });
+
+    it("sets a timeout if requested for asynchronous functions so they don't go on forever", function() {
+      var beforeFn = function(done) { },
+        fn = jasmine.createSpy('fn'),
+        onComplete = jasmine.createSpy('onComplete'),
+        onException = jasmine.createSpy('onException'),
+        queueRunner = new j$.QueueRunner({
+          fns: [beforeFn, fn],
+          onComplete: onComplete,
+          onException: onException,
+          enforceTimeout: function() { return true; }
+        });
+
+      queueRunner.execute();
+      expect(fn).not.toHaveBeenCalled();
+
+      jasmine.clock().tick(j$.DEFAULT_TIMEOUT_INTERVAL);
+
+      expect(onException).toHaveBeenCalledWith(jasmine.any(Error));
+      expect(fn).toHaveBeenCalled();
+      expect(onComplete).toHaveBeenCalled();
+    });
+
+    it("by default does not set a timeout for asynchronous functions", function() {
+      var beforeFn = function(done) { },
+        fn = jasmine.createSpy('fn'),
+        onComplete = jasmine.createSpy('onComplete'),
+        onException = jasmine.createSpy('onException'),
+        queueRunner = new j$.QueueRunner({
+          fns: [beforeFn, fn],
+          onComplete: onComplete,
+          onException: onException,
+        });
+
+      queueRunner.execute();
+      expect(fn).not.toHaveBeenCalled();
+
+      jasmine.clock().tick(j$.DEFAULT_TIMEOUT_INTERVAL);
+
+      expect(onException).not.toHaveBeenCalled();
+      expect(fn).not.toHaveBeenCalled();
+      expect(onComplete).not.toHaveBeenCalled();
+    });
+
+    it("clears the timeout when an async function throws an exception, to prevent additional onException calls", function() {
+       var fn = function(done) { throw new Error("error!"); },
+        onComplete = jasmine.createSpy('onComplete'),
+        onException = jasmine.createSpy('onException'),
+        queueRunner = new j$.QueueRunner({
+          fns: [fn],
+          onComplete: onComplete,
+          onException: onException
+        });
+
+      queueRunner.execute();
+
+      expect(onComplete).toHaveBeenCalled();
+      expect(onException).toHaveBeenCalled();
+
+      jasmine.clock().tick(j$.DEFAULT_TIMEOUT_INTERVAL);
+      expect(onException.calls.count()).toEqual(1);
+    });
+
+    it("clears the timeout when the done callback is called", function() {
+      var fn = function(done) { done(); },
+        onComplete = jasmine.createSpy('onComplete'),
+        onException = jasmine.createSpy('onException'),
+        queueRunner = new j$.QueueRunner({
+          fns: [fn],
+          onComplete: onComplete,
+          onException: onException
+        });
+
+      queueRunner.execute();
+
+      expect(onComplete).toHaveBeenCalled();
+
+      jasmine.clock().tick(j$.DEFAULT_TIMEOUT_INTERVAL);
+      expect(onException).not.toHaveBeenCalled();
+    });
   });
 
   it("calls an exception handler when an exception is thrown in a fn", function() {
@@ -124,7 +198,7 @@ describe("QueueRunner", function() {
         catchException: function(e) { return false; }
       });
 
-    expect(function() { queueRunner.execute(); }).toThrow();
+    expect(queueRunner.execute).toThrow();
   });
 
   it("continues running the functions even after an exception is thrown in an async spec", function() {

spec/core/SpecSpec.js

@@ -222,90 +222,7 @@ describe("Spec", function() {
     expect(specNameSpy.calls.mostRecent().args[0].id).toEqual(spec.id);
   });
 
-  it("sets a timeout for async functions to keep them from running forever", function() {
+   describe("when a spec is marked pending during execution", function() {
-    var queueRunnerSpy = jasmine.createSpy('queue runner'),
-      setTimeoutSpy = jasmine.createSpy('setTimeout'),
-      spec = new j$.Spec({
-        beforeFns: function() { return [function(done) { }]; },
-        fn: function(done) { },
-        afterFns: function() { return [function(done) { }]; },
-        timer: {
-          setTimeout: setTimeoutSpy,
-          clearTimeout: function() {}
-        },
-        queueRunnerFactory: queueRunnerSpy
-      });
-
-    spec.execute();
-    var fns = queueRunnerSpy.calls.mostRecent().args[0].fns;
-
-    for (var i = 0; i < fns.length; i++) {
-      fns[i]();
-    }
-
-    expect(setTimeoutSpy.calls.count()).toEqual(3);
-    expect(setTimeoutSpy).toHaveBeenCalledWith(jasmine.any(Function), j$.DEFAULT_TIMEOUT_INTERVAL);
-  });
-
-  it("resets the timeout timer when an async before throws an exception", function() {
-    var queueRunnerSpy = jasmine.createSpy('queueRunner'),
-      clearTimeoutSpy = jasmine.createSpy('clear timeout'),
-      spec = new j$.Spec({
-        beforeFns: function() { return [function(done) {}]; },
-        fn: function() { },
-        timer: {
-          setTimeout: function () { return 920; },
-          clearTimeout: clearTimeoutSpy
-        },
-        queueRunnerFactory: queueRunnerSpy
-      });
-
-    spec.execute();
-    queueRunnerSpy.calls.mostRecent().args[0].fns[0]();
-    queueRunnerSpy.calls.mostRecent().args[0].onException(new Error());
-
-    expect(clearTimeoutSpy).toHaveBeenCalledWith(920);
-  });
-
-  it("resets the timeout timer when an async spec throws an exception", function() {
-    var queueRunnerSpy = jasmine.createSpy('queueRunner'),
-      clearTimeoutSpy = jasmine.createSpy('clear timeout'),
-      spec = new j$.Spec({
-        fn: function(done) { },
-        timer: {
-          setTimeout: function () { return 920; },
-          clearTimeout: clearTimeoutSpy
-        },
-        queueRunnerFactory: queueRunnerSpy
-      });
-
-    spec.execute();
-    queueRunnerSpy.calls.mostRecent().args[0].fns[0]();
-    queueRunnerSpy.calls.mostRecent().args[0].onException(new Error());
-
-    expect(clearTimeoutSpy).toHaveBeenCalledWith(920);
-  });
-
-  it("resets the timeout timer when an async after spec throws an exception", function() {
-    var queueRunnerSpy = jasmine.createSpy('queueRunner'),
-      clearTimeoutSpy = jasmine.createSpy('clear timeout'),
-      spec = new j$.Spec({
-        fn: function() { },
-        afterFns: function() { return [function(done) {}]; },
-        timer: {
-          setTimeout: function () { return 920; },
-          clearTimeout: clearTimeoutSpy
-        },
-        queueRunnerFactory: queueRunnerSpy
-      });
-
-    spec.execute();
-    queueRunnerSpy.calls.mostRecent().args[0].fns[1]();
-    queueRunnerSpy.calls.mostRecent().args[0].onException(new Error());
-
-    expect(clearTimeoutSpy).toHaveBeenCalledWith(920);
-  });
-  describe("when a spec is marked pending during execution", function() {
     it("should mark the spec as pending", function() {
       var fakeQueueRunner = function(opts) {
           opts.onException(new Error(j$.Spec.pendingSpecExceptionMessage));

spec/core/integration/EnvSpec.js

@@ -299,10 +299,14 @@ describe("Env integration", function() {
 
     it("should wait a specified interval before failing specs haven't called done yet", function(done) {
       var env = new j$.Env(),
-          reporter = jasmine.createSpyObj('fakeReporter', [ "specDone" ]);
+          reporter = jasmine.createSpyObj('fakeReporter', [ "specDone", "jasmineDone" ]);
 
       reporter.specDone.and.callFake(function() {
         expect(reporter.specDone).toHaveBeenCalledWith(jasmine.objectContaining({status: 'failed'}));
+      });
+
+      reporter.jasmineDone.and.callFake(function() {
+        expect(reporter.jasmineDone.calls.count()).toEqual(1);
         done();
       });
 
@@ -311,7 +315,7 @@ describe("Env integration", function() {
 
       env.it("async spec that doesn't call done", function(underTestCallback) {
         env.expect(true).toBeTruthy();
-        jasmine.getEnv().clock.tick(8415);
+        jasmine.getEnv().clock.tick(8416);
       });
 
       env.execute();

src/core/Env.js

@@ -136,6 +136,7 @@ getJasmineRequireObj().Env = function(j$) {
     var queueRunnerFactory = function(options) {
       options.catchException = catchException;
       options.clearStack = options.clearStack || clearStack;
+      options.timer = {setTimeout: realSetTimeout, clearTimeout: realClearTimeout};
 
       new j$.QueueRunner(options).execute();
     };
@@ -271,8 +272,7 @@ getJasmineRequireObj().Env = function(j$) {
         description: description,
         expectationResultFactory: expectationResultFactory,
         queueRunnerFactory: queueRunnerFactory,
-        fn: fn,
+        fn: fn
-        timer: {setTimeout: realSetTimeout, clearTimeout: realClearTimeout}
       });
 
       runnableLookupTable[spec.id] = spec;

src/core/QueueRunner.js

@@ -1,4 +1,4 @@
-getJasmineRequireObj().QueueRunner = function() {
+getJasmineRequireObj().QueueRunner = function(j$) {
 
   function QueueRunner(attrs) {
     this.fns = attrs.fns || [];
@@ -6,7 +6,9 @@ getJasmineRequireObj().QueueRunner = function() {
     this.clearStack = attrs.clearStack || function(fn) {fn();};
     this.onException = attrs.onException || function() {};
     this.catchException = attrs.catchException || function() { return true; };
+    this.enforceTimeout = attrs.enforceTimeout || function() { return false; };
     this.userContext = {};
+    this.timer = attrs.timeout || {setTimeout: setTimeout, clearTimeout: clearTimeout};
   }
 
   QueueRunner.prototype.execute = function() {
@@ -42,7 +44,21 @@ getJasmineRequireObj().QueueRunner = function() {
     }
 
     function attemptAsync(fn) {
-      var next = function () { self.run(fns, iterativeIndex + 1); };
+      var clearTimeout = function () {
+          Function.prototype.apply.apply(self.timer.clearTimeout, [j$.getGlobal(), [timeoutId]]);
+        },
+        next = function () {
+          clearTimeout(timeoutId);
+          self.run(fns, iterativeIndex + 1);
+        },
+        timeoutId;
+
+      if (self.enforceTimeout()) {
+        timeoutId = Function.prototype.apply.apply(self.timer.setTimeout, [j$.getGlobal(), [function() {
+          self.onException(new Error('Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.'));
+          next();
+        }, j$.DEFAULT_TIMEOUT_INTERVAL]]);
+      }
 
       try {
         fn.call(self.userContext, next);

src/core/Spec.js

@@ -14,8 +14,6 @@ getJasmineRequireObj().Spec = function(j$) {
     this.queueRunnerFactory = attrs.queueRunnerFactory || function() {};
     this.catchingExceptions = attrs.catchingExceptions || function() { return true; };
 
-    this.timer = attrs.timer || {setTimeout: setTimeout, clearTimeout: clearTimeout};
-
     if (!this.fn) {
       this.pend();
     }
@@ -40,8 +38,7 @@ getJasmineRequireObj().Spec = function(j$) {
   };
 
   Spec.prototype.execute = function(onComplete) {
-    var self = this,
+    var self = this;
-        timeout;
 
     this.onStart(this);
 
@@ -50,42 +47,16 @@ getJasmineRequireObj().Spec = function(j$) {
       return;
     }
 
-    function timeoutable(fn) {
+    var allFns = this.beforeFns().concat(this.fn).concat(this.afterFns());
-      return function(done) {
-        timeout = Function.prototype.apply.apply(self.timer.setTimeout, [j$.getGlobal(), [function() {
-          onException(new Error('Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.'));
-          done();
-        }, j$.DEFAULT_TIMEOUT_INTERVAL]]);
-
-        var callDone = function() {
-          clearTimeoutable();
-          done();
-        };
-
-        fn.call(this, callDone); //TODO: do we care about more than 1 arg?
-      };
-    }
-
-    function clearTimeoutable() {
-      Function.prototype.apply.apply(self.timer.clearTimeout, [j$.getGlobal(), [timeout]]);
-      timeout = void 0;
-    }
-
-    var allFns = this.beforeFns().concat(this.fn).concat(this.afterFns()),
-      allTimeoutableFns = [];
-    for (var i = 0; i < allFns.length; i++) {
-      var fn = allFns[i];
-      allTimeoutableFns.push(fn.length > 0 ? timeoutable(fn) : fn);
-    }
 
     this.queueRunnerFactory({
-      fns: allTimeoutableFns,
+      fns: allFns,
       onException: onException,
-      onComplete: complete
+      onComplete: complete,
+      enforceTimeout: function() { return true; }
     });
 
     function onException(e) {
-      clearTimeoutable();
       if (Spec.isPendingSpecException(e)) {
         self.pend();
         return;

src/core/requireCore.js

@@ -24,7 +24,7 @@ getJasmineRequireObj().core = function(jRequire) {
   j$.matchersUtil = jRequire.matchersUtil(j$);
   j$.ObjectContaining = jRequire.ObjectContaining(j$);
   j$.pp = jRequire.pp(j$);
-  j$.QueueRunner = jRequire.QueueRunner();
+  j$.QueueRunner = jRequire.QueueRunner(j$);
   j$.ReportDispatcher = jRequire.ReportDispatcher();
   j$.Spec = jRequire.Spec(j$);
   j$.SpyStrategy = jRequire.SpyStrategy();