Some exploration and refactoring re waitsFor() bug: waitsFor() hangs forever if latch function never returns true.

spec/suites/SpecRunningSpec.js

@@ -258,86 +258,89 @@ describe("jasmine spec running", function () {
     expect(another_spec.results().getItems()[0].passed()).toEqual(true);
   });
 
-  it("testWaitsFor", function() {
+  describe("waitsFor", function() {
-    var doneWaiting = false;
+    it("testWaitsFor", function() {
-    var runsBlockExecuted = false;
+      var doneWaiting = false;
+      var runsBlockExecuted = false;
 
-    var spec;
+      var spec;
-    env.describe('foo', function() {
+      env.describe('foo', function() {
-      spec = env.it('has a waits for', function() {
+        spec = env.it('has a waits for', function() {
-        this.runs(function() {
+          this.runs(function() {
-        });
+          });
 
-        this.waitsFor(500, function() {
+          this.waitsFor(500, function() {
-          return doneWaiting;
+            return doneWaiting;
-        });
+          });
 
-        this.runs(function() {
+          this.runs(function() {
-          runsBlockExecuted = true;
+            runsBlockExecuted = true;
+          });
         });
       });
+
+      spec.execute();
+      expect(runsBlockExecuted).toEqual(false); //, 'should not have executed runs block yet');
+      fakeTimer.tick(100);
+      doneWaiting = true;
+      fakeTimer.tick(100);
+      expect(runsBlockExecuted).toEqual(true); //, 'should have executed runs block');
     });
 
-    spec.execute();
+    it("fails with message", function() {
-    expect(runsBlockExecuted).toEqual(false); //, 'should not have executed runs block yet');
+      var spec;
-    fakeTimer.tick(100);
+      env.describe('foo', function() {
-    doneWaiting = true;
+        spec = env.it('has a waits for', function() {
-    fakeTimer.tick(100);
+          this.runs(function() {
-    expect(runsBlockExecuted).toEqual(true); //, 'should have executed runs block');
+          });
-  });
 
-  it("testWaitsForFailsWithMessage", function() {
+          this.waitsFor(500, function() {
-    var spec;
+            return false; // force a timeout
-    env.describe('foo', function() {
+          }, 'my awesome condition');
-      spec = env.it('has a waits for', function() {
-        this.runs(function() {
-        });
 
-        this.waitsFor(500, function() {
+          this.runs(function() {
-          return false; // force a timeout
+          });
-        }, 'my awesome condition');
-
-        this.runs(function() {
         });
       });
+
+      spec.execute();
+      fakeTimer.tick(1000);
+      expect(spec.results().getItems()[0].message).toEqual('timeout: timed out after 500 msec waiting for my awesome condition');
     });
 
-    spec.execute();
+    it("fails and skips the rest of the spec if timeout is reached and the latch function hasn't returned true", function() {
-    fakeTimer.tick(1000);
+      var runsBlockExecuted = false;
-    var actual = spec.results().getItems()[0].message;
+      var subsequentSpecRan = false;
-    var expected = 'timeout: timed out after 500 msec waiting for my awesome condition';
-    expect(actual).toEqual(expected);
-  });
 
-  it("waitsFor fails and skips the rest of the spec if timeout is reached and the latch function is still false", function() {
+      var timeoutSpec, subsequentSpec;
-    var runsBlockExecuted = false;
+      var suite = env.describe('foo', function() {
+        timeoutSpec = env.it('has a waits for', function() {
+          this.runs(function() {
+          });
 
-    var spec;
+          this.waitsFor(500, function() {
-    env.describe('foo', function() {
+            return false;
-      spec = env.it('has a waits for', function() {
+          });
-        this.runs(function() {
+
+          this.runs(function() {
+            runsBlockExecuted = true;
+          });
         });
 
-        this.waitsFor(500, function() {
+        subsequentSpec = env.it('then carries on to the next test', function() {
-          return false;
+          subsequentSpecRan = true;
-        });
-
-        this.runs(function() {
-          runsBlockExecuted = true;
         });
       });
-    });
 
-    spec.execute();
+      env.execute();
-    expect(runsBlockExecuted).toEqual(false);
+      expect(runsBlockExecuted).toEqual(false);
-    fakeTimer.tick(100);
+      fakeTimer.tick(100);
-    expect(runsBlockExecuted).toEqual(false);
+      expect(runsBlockExecuted).toEqual(false);
-    fakeTimer.tick(400);
+      fakeTimer.tick(400);
-    expect(runsBlockExecuted).toEqual(false);
+      expect(runsBlockExecuted).toEqual(false);
-    var actual = spec.results().getItems()[0].message;
+      expect(timeoutSpec.results().getItems()[0].message).toEqual('timeout: timed out after 500 msec waiting for something to happen');
-    var expected = 'timeout: timed out after 500 msec waiting for something to happen';
+// todo: expect(subsequentSpecRan).toEqual(true); [xw 20100819]
-    expect(actual).toEqual(expected,
+    });
-      'expected "' + expected + '" but found "' + actual + '"');
   });
 
   it("testSpecAfter", function() {
@@ -520,17 +523,15 @@ describe("jasmine spec running", function () {
   });
 
   describe('#waitsFor should allow consecutive calls', function () {
-
     var foo;
     beforeEach(function () {
-
       foo = 0;
     });
 
     it('exits immediately (does not stack) if the latchFunction times out', function () {
       var reachedFirstWaitsFor = false;
       var reachedSecondWaitsFor = false;
-      var waitsSuite = env.describe('suite that waits', function () {
+      env.describe('suite that waits', function () {
         env.it('should stack timeouts', function() {
           this.waitsFor(500, function () {
             reachedFirstWaitsFor = true;
@@ -546,7 +547,7 @@ describe("jasmine spec running", function () {
       });
 
       expect(reachedFirstWaitsFor).toEqual(false);
-      waitsSuite.execute();
+      env.execute();
 
       expect(reachedFirstWaitsFor).toEqual(true);
       expect(foo).toEqual(0);

spec/suites/WaitsForBlockSpec.js

@@ -15,7 +15,7 @@ describe('WaitsForBlock', function () {
       return true;
     };
     var block = new jasmine.WaitsForBlock(env, timeout, latchFunction, message, spec);
-    expect(onComplete).wasNotCalled();
+    expect(onComplete).not.toHaveBeenCalled();
     block.execute(onComplete);
     expect(onComplete).toHaveBeenCalled();
   });
@@ -51,22 +51,22 @@ describe('WaitsForBlock', function () {
       env.clearInterval = fakeTimer.clearInterval;
     });
 
-    it('latchFunction should be retried after 100 ms', function () {
+    it('latchFunction should be retried after 10 ms', function () {
       var block = new jasmine.WaitsForBlock(env, timeout, latchFunction, message, spec);
-      expect(latchFunction).wasNotCalled();
+      expect(latchFunction).not.toHaveBeenCalled();
       block.execute(onComplete);
       expect(latchFunction.callCount).toEqual(1);
-      fakeTimer.tick(50);
+      fakeTimer.tick(5);
       expect(latchFunction.callCount).toEqual(1);
-      fakeTimer.tick(50);
+      fakeTimer.tick(5);
       expect(latchFunction.callCount).toEqual(2);
     });
 
     it('onComplete should be called if latchFunction returns true before timeout', function () {
       var block = new jasmine.WaitsForBlock(env, timeout, latchFunction, message, spec);
-      expect(onComplete).wasNotCalled();
+      expect(onComplete).not.toHaveBeenCalled();
       block.execute(onComplete);
-      expect(onComplete).wasNotCalled();
+      expect(onComplete).not.toHaveBeenCalled();
       latchFunction.andReturn(true);
       fakeTimer.tick(100);
       expect(onComplete).toHaveBeenCalled();
@@ -76,12 +76,12 @@ describe('WaitsForBlock', function () {
       spyOn(spec, 'fail');
       var block = new jasmine.WaitsForBlock(env, timeout, latchFunction, message, spec);
       block.execute(onComplete);
-      expect(spec.fail).wasNotCalled();
+      expect(spec.fail).not.toHaveBeenCalled();
       fakeTimer.tick(timeout);
       expect(spec.fail).toHaveBeenCalled();
       var failMessage = spec.fail.mostRecentCall.args[0].message;
       expect(failMessage).toMatch(message);
-      expect(onComplete).wasNotCalled();
+      expect(onComplete).not.toHaveBeenCalled(); // todo: this is an issue... [xw 20100819]
     });
   });
 });

src/WaitsForBlock.js

@@ -1,3 +1,14 @@
+/**
+ * A block which waits for some condition to become true, with timeout.
+ *
+ * @constructor
+ * @extends jasmine.Block
+ * @param {jasmine.Env} env The Jasmine environment.
+ * @param {Number} timeout The maximum time in milliseconds to wait for the condition to become true.
+ * @param {Function} latchFunction A function which returns true when the desired condition has been met.
+ * @param {String} message The message to display if the desired condition hasn't been met within the given time period.
+ * @param {jasmine.Spec} spec The Jasmine spec.
+ */
 jasmine.WaitsForBlock = function(env, timeout, latchFunction, message, spec) {
   this.timeout = timeout;
   this.latchFunction = latchFunction;
@@ -5,33 +16,35 @@ jasmine.WaitsForBlock = function(env, timeout, latchFunction, message, spec) {
   this.totalTimeSpentWaitingForLatch = 0;
   jasmine.Block.call(this, env, null, spec);
 };
-
 jasmine.util.inherit(jasmine.WaitsForBlock, jasmine.Block);
 
-jasmine.WaitsForBlock.TIMEOUT_INCREMENT = 100;
+jasmine.WaitsForBlock.TIMEOUT_INCREMENT = 10;
 
-jasmine.WaitsForBlock.prototype.execute = function (onComplete) {
+jasmine.WaitsForBlock.prototype.execute = function(onComplete) {
-  var self = this;
+  this.env.reporter.log('>> Jasmine waiting for ' + (this.message || 'something to happen'));
-  self.env.reporter.log('>> Jasmine waiting for ' + (self.message || 'something to happen'));
   var latchFunctionResult;
   try {
-    latchFunctionResult = self.latchFunction.apply(self.spec);
+    latchFunctionResult = this.latchFunction.apply(this.spec);
   } catch (e) {
-    self.spec.fail(e);
+    this.spec.fail(e);
     onComplete();
     return;
   }
 
   if (latchFunctionResult) {
     onComplete();
-  } else if (self.totalTimeSpentWaitingForLatch >= self.timeout) {
+  } else if (this.totalTimeSpentWaitingForLatch >= this.timeout) {
-    var message = 'timed out after ' + self.timeout + ' msec waiting for ' + (self.message || 'something to happen');
+    var message = 'timed out after ' + this.timeout + ' msec waiting for ' + (this.message || 'something to happen');
-    self.spec.fail({
+    this.spec.fail({
       name: 'timeout',
       message: message
     });
+    // todo: need to prevent additional blocks in this spec from running... [xw 20100819]
   } else {
-    self.totalTimeSpentWaitingForLatch += jasmine.WaitsForBlock.TIMEOUT_INCREMENT;
+    this.totalTimeSpentWaitingForLatch += jasmine.WaitsForBlock.TIMEOUT_INCREMENT;
-    self.env.setTimeout(function () { self.execute(onComplete); }, jasmine.WaitsForBlock.TIMEOUT_INCREMENT);
+    var self = this;
+    this.env.setTimeout(function() {
+      self.execute(onComplete);
+    }, jasmine.WaitsForBlock.TIMEOUT_INCREMENT);
   }
 };