Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

clearInterval should work with the Mock Clock #253

Closed
wants to merge 1 commit into from

5 participants

@strworkstation

Resolves Issue #137

Original fix: jiwire@63d5af2

@infews
Owner

We're interested in taking this. If you're still interested, can you please update your fix to what's at HEAD? Also, if you can move the clearInterval call to inside the it() I think your spec is more understandable.

Thanks for your help with Jasmine!

@submitteddenied

Sure thing, new commit on the way!

@submitteddenied

Yikes, that was messier than I expected. I'll re-open a new pull request and re-implement the fix. Standby.

@submitteddenied

Ok, git surgery done. @infews, let me know if there's anything more that needs to be changed

@twuttke

I just ran into this bug today, wasted an hour debugging my own code. I came up with almost the same solution.

After running with your changes on a large test suite, the js stack blows up. It has something to do with not clearing the handler function from the scheduledFunctions hash right away (too many closures?) I experimented with marking the scheduledFunctions with the sentinel value 'null' to indiciate the function has been queued. If the interval or timer gets cleared, it becomes jasmine.undefined.

This works for me, but I wouldn't be surprised if someone objects to the tri-state variable magic.

jasmine.FakeTimer.prototype.runFunctionsWithinRange = function(oldMillis, nowMillis) {
var scheduledFunc;
var funcsToRun = [];
for (var timeoutKey in this.scheduledFunctions) {
scheduledFunc = this.scheduledFunctions[timeoutKey];
if (scheduledFunc &&
scheduledFunc.runAtMillis >= oldMillis &&
scheduledFunc.runAtMillis <= nowMillis) {
funcsToRun.push(scheduledFunc);
this.scheduledFunctions[timeoutKey] = null; // null means the handler is queued but can still be cancelled
}
}

if (funcsToRun.length > 0) {
funcsToRun.sort(function(a, b) {
return a.runAtMillis - b.runAtMillis;
});
for (var i = 0; i < funcsToRun.length; ++i) {
try {
var funcToRun = funcsToRun[i];
this.nowMillis = funcToRun.runAtMillis;
if (this.scheduledFunctions[funcToRun.timeoutKey] !== null) {
// Timer was cancelled after it was queued
continue;
}
funcToRun.funcToCall();
if (funcToRun.recurring && this.scheduledFunctions[funcToRun.timeoutKey] === null) {
this.scheduleFunction(funcToRun.timeoutKey,
funcToRun.funcToCall,
funcToRun.millis,
true);
}
} catch(e) {
}
}
this.runFunctionsWithinRange(oldMillis, nowMillis);
}
};

@twuttke twuttke commented on the diff
lib/jasmine-core/jasmine.js
@@ -1636,11 +1637,13 @@ jasmine.FakeTimer.prototype.runFunctionsWithinRange = function(oldMillis, nowMil
var funcToRun = funcsToRun[i];
this.nowMillis = funcToRun.runAtMillis;
funcToRun.funcToCall();
@twuttke
twuttke added a note

You should probably put something like
if (this.scheduledFunctions[funcToRun.timeoutKey] == jasmine.undefined) {
continue; // Timer has been cancelled by another timer
}
before calling
funcToRun.funcToCall();

since a timer might get cancelled inside the callback of another timer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@infews
Owner

Has someone looked at master? We've changed the implementation of the Clock considerably for 2.0 and these sorts of cleanups should be covered.

@slackersoft
Owner

As @infews mentioned, this should be covered in 2.0. Closing this for now, feel free to re-open or create a new pull request on top of master if there is still an issue

@slackersoft slackersoft closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 31, 2012
  1. @submitteddenied
This page is out of date. Refresh to see the latest.
View
2  lib/jasmine-core/jasmine-html.js
@@ -154,7 +154,7 @@ jasmine.HtmlReporter = function(_doc) {
dom.symbolSummary = self.createDom('ul', {className: 'symbolSummary'}),
dom.alert = self.createDom('div', {className: 'alert'},
self.createDom('span', { className: 'exceptions' },
- self.createDom('label', { className: 'label', 'for': 'no_try_catch' }, 'No try/catch'),
+ self.createDom('label', { className: 'label', for: 'no_try_catch' }, 'No try/catch'),
self.createDom('input', { id: 'no_try_catch', type: 'checkbox' }))),
dom.results = self.createDom('div', {className: 'results'},
dom.summary = self.createDom('div', { className: 'summary' }),
View
46 lib/jasmine-core/jasmine.js
@@ -35,6 +35,11 @@ jasmine.VERBOSE = false;
jasmine.DEFAULT_UPDATE_INTERVAL = 250;
/**
+ * Maximum levels of nesting that will be included when an object is pretty-printed
+ */
+jasmine.MAX_PRETTY_PRINT_DEPTH = 40;
+
+/**
* Default timeout interval in milliseconds for waitsFor() blocks.
*/
jasmine.DEFAULT_TIMEOUT_INTERVAL = 5000;
@@ -470,7 +475,7 @@ jasmine.log = function() {
* @see jasmine.createSpy
* @param obj
* @param methodName
- * @returns a Jasmine spy that can be chained with all spy methods
+ * @return {jasmine.Spy} a Jasmine spy that can be chained with all spy methods
*/
var spyOn = function(obj, methodName) {
return jasmine.getEnv().currentSpec.spyOn(obj, methodName);
@@ -515,6 +520,7 @@ if (isCommonJS) exports.xit = xit;
* jasmine.Matchers functions.
*
* @param {Object} actual Actual value to test against and expected value
+ * @return {jasmine.Matchers}
*/
var expect = function(actual) {
return jasmine.getEnv().currentSpec.expect(actual);
@@ -1404,18 +1410,14 @@ jasmine.Matchers.prototype.toHaveBeenCalledWith = function() {
throw new Error('Expected a spy, but got ' + jasmine.pp(this.actual) + '.');
}
this.message = function() {
+ var invertedMessage = "Expected spy " + this.actual.identity + " not to have been called with " + jasmine.pp(expectedArgs) + " but it was.";
+ var positiveMessage = "";
if (this.actual.callCount === 0) {
- // todo: what should the failure message for .not.toHaveBeenCalledWith() be? is this right? test better. [xw]
- return [
- "Expected spy " + this.actual.identity + " to have been called with " + jasmine.pp(expectedArgs) + " but it was never called.",
- "Expected spy " + this.actual.identity + " not to have been called with " + jasmine.pp(expectedArgs) + " but it was."
- ];
+ positiveMessage = "Expected spy " + this.actual.identity + " to have been called with " + jasmine.pp(expectedArgs) + " but it was never called.";
} else {
- return [
- "Expected spy " + this.actual.identity + " to have been called with " + jasmine.pp(expectedArgs) + " but was called with " + jasmine.pp(this.actual.argsForCall),
- "Expected spy " + this.actual.identity + " not to have been called with " + jasmine.pp(expectedArgs) + " but was called with " + jasmine.pp(this.actual.argsForCall)
- ];
+ positiveMessage = "Expected spy " + this.actual.identity + " to have been called with " + jasmine.pp(expectedArgs) + " but actual calls were " + jasmine.pp(this.actual.argsForCall).replace(/^\[ | \]$/g, '')
}
+ return [positiveMessage, invertedMessage];
};
return this.env.contains_(this.actual.argsForCall, expectedArgs);
@@ -1485,7 +1487,7 @@ jasmine.Matchers.prototype.toBeCloseTo = function(expected, precision) {
/**
* Matcher that checks that the expected exception was thrown by the actual.
*
- * @param {String} expected
+ * @param {String} [expected]
*/
jasmine.Matchers.prototype.toThrow = function(expected) {
var result = false;
@@ -1623,7 +1625,6 @@ jasmine.FakeTimer.prototype.runFunctionsWithinRange = function(oldMillis, nowMil
scheduledFunc.runAtMillis >= oldMillis &&
scheduledFunc.runAtMillis <= nowMillis) {
funcsToRun.push(scheduledFunc);
- this.scheduledFunctions[timeoutKey] = jasmine.undefined;
}
}
@@ -1636,11 +1637,13 @@ jasmine.FakeTimer.prototype.runFunctionsWithinRange = function(oldMillis, nowMil
var funcToRun = funcsToRun[i];
this.nowMillis = funcToRun.runAtMillis;
funcToRun.funcToCall();
@twuttke
twuttke added a note

You should probably put something like
if (this.scheduledFunctions[funcToRun.timeoutKey] == jasmine.undefined) {
continue; // Timer has been cancelled by another timer
}
before calling
funcToRun.funcToCall();

since a timer might get cancelled inside the callback of another timer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- if (funcToRun.recurring) {
+ if (funcToRun.recurring && this.scheduledFunctions[funcToRun.timeoutKey] != jasmine.undefined) {
this.scheduleFunction(funcToRun.timeoutKey,
funcToRun.funcToCall,
funcToRun.millis,
true);
+ } else {
+ this.scheduledFunctions[funcToRun.timeoutKey] = jasmine.undefined;
}
} catch(e) {
}
@@ -1883,10 +1886,6 @@ jasmine.PrettyPrinter = function() {
* @param value
*/
jasmine.PrettyPrinter.prototype.format = function(value) {
- if (this.ppNestLevel_ > 40) {
- throw new Error('jasmine.PrettyPrinter: format() nested too deeply!');
- }
-
this.ppNestLevel_++;
try {
if (value === jasmine.undefined) {
@@ -1929,6 +1928,7 @@ jasmine.PrettyPrinter.prototype.format = function(value) {
jasmine.PrettyPrinter.prototype.iterateObject = function(obj, fn) {
for (var property in obj) {
+ if (!obj.hasOwnProperty(property)) continue;
if (property == '__Jasmine_been_here_before__') continue;
fn(property, obj.__lookupGetter__ ? (obj.__lookupGetter__(property) !== jasmine.undefined &&
obj.__lookupGetter__(property) !== null) : false);
@@ -1956,6 +1956,11 @@ jasmine.StringPrettyPrinter.prototype.emitString = function(value) {
};
jasmine.StringPrettyPrinter.prototype.emitArray = function(array) {
+ if (this.ppNestLevel_ > jasmine.MAX_PRETTY_PRINT_DEPTH) {
+ this.append("Array");
+ return;
+ }
+
this.append('[ ');
for (var i = 0; i < array.length; i++) {
if (i > 0) {
@@ -1967,6 +1972,11 @@ jasmine.StringPrettyPrinter.prototype.emitArray = function(array) {
};
jasmine.StringPrettyPrinter.prototype.emitObject = function(obj) {
+ if (this.ppNestLevel_ > jasmine.MAX_PRETTY_PRINT_DEPTH) {
+ this.append("Object");
+ return;
+ }
+
var self = this;
this.append('{ ');
var first = true;
@@ -2587,5 +2597,5 @@ jasmine.version_= {
"major": 1,
"minor": 2,
"build": 0,
- "revision": 1343710612
+ "revision": 1351706343
};
View
19 spec/core/MockClockSpec.js
@@ -1,7 +1,7 @@
describe("MockClock", function () {
beforeEach(function() {
- jasmine.Clock.useMock();
+ jasmine.Clock.useMock();
});
describe("setTimeout", function () {
@@ -32,6 +32,23 @@ describe("MockClock", function () {
});
});
+ describe("clearInterval", function () {
+ it("should prevent the interval function from being called", function() {
+ var callCount = 0, intervalId;
+ intervalId = setInterval(function() {
+ callCount++;
+ if (callCount > 1) { clearInterval(intervalId); }
+ }, 30000);
+ expect(callCount).toEqual(0);
+ jasmine.Clock.tick(30000);
+ expect(callCount).toEqual(1);
+ jasmine.Clock.tick(30000);
+ expect(callCount).toEqual(2);
+ jasmine.Clock.tick(30000);
+ expect(callCount).toEqual(2);
+ });
+ });
+
it("shouldn't complain if you call jasmine.Clock.useMock() more than once", function() {
jasmine.Clock.useMock();
});
View
5 src/core/mock-timeout.js
@@ -49,7 +49,6 @@ jasmine.FakeTimer.prototype.runFunctionsWithinRange = function(oldMillis, nowMil
scheduledFunc.runAtMillis >= oldMillis &&
scheduledFunc.runAtMillis <= nowMillis) {
funcsToRun.push(scheduledFunc);
- this.scheduledFunctions[timeoutKey] = jasmine.undefined;
}
}
@@ -62,11 +61,13 @@ jasmine.FakeTimer.prototype.runFunctionsWithinRange = function(oldMillis, nowMil
var funcToRun = funcsToRun[i];
this.nowMillis = funcToRun.runAtMillis;
funcToRun.funcToCall();
- if (funcToRun.recurring) {
+ if (funcToRun.recurring && this.scheduledFunctions[funcToRun.timeoutKey] != jasmine.undefined) {
this.scheduleFunction(funcToRun.timeoutKey,
funcToRun.funcToCall,
funcToRun.millis,
true);
+ } else {
+ this.scheduledFunctions[funcToRun.timeoutKey] = jasmine.undefined;
}
} catch(e) {
}
View
2  src/version.js
@@ -3,5 +3,5 @@ jasmine.version_= {
"major": 1,
"minor": 2,
"build": 0,
- "revision": 1343710612
+ "revision": 1351706632
};
Something went wrong with that request. Please try again.