Mocha.js and useFakeTimers giving global leak errors

[camwest]

I get this error when running Mocha 1.1.0 and sinon 1.3.4

Error: global leaks detected: setTimeout, setInterval, clearTimeout, clearInterval

beforeEach(function() { this.clock = sinon.useFakeTimers(); });
afterEach(function() { this.clock.restore(); });

[cjohansen]

I guess overwriting, then restoring these functions causes these properties to have their enumerable property set to true. Don't think there's much I can here, unless delete this.setTimeout works consistently cross-browser.

[Raynos]

@visionmedia we may want to change the global leak detection to allow overwriting of existing global tokens.

[tj]

overwriting existing ones shouldn't trigger it, we just check the names not the values

[cjohansen]

By the time the test completes (i.e. after restore) they're no longer overwritten. But I'm guessing setTimeout and friends are not enumerable to begin with, but are after having been tampered with.

[tj]

ah I see, that might be the case. gotta love them prop descriptors

[camwest]

So is there something I can do to help?

[cjohansen]

My suggestion is that mocha not care about "leaking" known globals. If you want to help I guess you could research whether it's possible to overwrite window.setTimeout and then restoring it again without making it enumerable in all browses. I suspect you can't, but I'm not sure

[camwest]

@cjohansen how can I check if something is enumerable?

[tj]

@cjohansen yeah that seems to be the case:

Object.keys(window)
["window", "top", "location", "external", "chrome", "v8Locale", "document", "html5", "Modernizr", "$", "jQuery", "jQuery1720022294572554528713", "moment", "GitHub", "BaconPlayer", "DateInput", "clippyCopiedCallback", "debug", "_gaq", "_gauges", "$stats", "_gat", "gaGlobal"]

I'll open an issue for mocha

[domenic]

@cjohansen @visionmedia There's a couple solutions that wouldn't require a change to Mocha (which IMO shouldn't be necessary).

1. Put setTimeout et al. on the prototype of window. Note that window.hasOwnProperty("setTimeout") === false, whereas Object.getPrototypeOf(window).hasOwnProperty("setTimeout") === true.
2. Redefine them with enumerable set to false: replace window.setTimeout = ... with Object.defineProperty(window, "setTimeout", { value: ..., configurable: true, writable: true }).

Note that setTimeout is by default enumerable and on Object.getPrototypeOf(window), so solution 1 is pretty much the best.

@camwest You can check with obj.propertyIsEnumerable("propName").

[cjohansen]

@domenic If I can define these methods on window's prototype I will. Does it work in IE6 + 7?

[domenic]

@cjohansen Support for Object.getPrototypeOf is apparently not great, but at least in the latest Firefox, Object.getPrototypeOf(window) === window.constructor.prototype.

But ugh, tested in IE7 and IE8 modes of IE10, and that doesn't work either (window has no constructor property). Further thinking required.

[neonstalwart]

shouldn't delete window.setTimeout work? maybe it's happening too late?

[cjohansen]

@neonstalwart to be honest, I can't remember if I tried (haven't looked into this for ~2 years). If someone wants to do the research (remember, Sinon still supports IE6/7) I'll be happy to change this for the better.

[telaviv]

@cjohansen @neonstalwart i did some research on ie 6 and 8. The way sinon define things like setTimeout on those platforms is like so:

function setTimeout() {}
setTimeout = sinon.timers.setTimeout

this is done so that window.setTimeout becomes writeable. Unfortunately this also has a side effect of making window.setTimeout not configurable. The only way to make a property configurable again is with Object.defineProperty, which unfortunately doesn't work pre ie9. When a property is not configurable, the delete operator doesn't work on it any longer.

[cjohansen]

Is there a way to make setTimeout both writable and configurable in IE?

[telaviv]

I experimented with simply setting window.blah = blah, but to no avail. At this point i'm happy that we can mock setTimeout at all in IE.

[Radagaisus]

I think a fix for modern browsers first, then for IE 6 + 7 will fix this issue for most users.

[Radagaisus]

Also, you can just disable the global leaks test for certain functions:

mocha.setup({globals: ['setTimeout', 'setInterval', 'clearTimeout', 'clearInterval']});

Will fix this problem. It's also available in the command line as --globals.

[telaviv]

I recently patched mocha to do just that by default.

[gtramontina]

I could not find a way to specify the globals when using Karma, given the mocha adapter is setting globals: ['__cov*']. Am I missing something?

[mantoni]

I'll mark this for 2.0. I would suggest to drop old IE support in the next major release and go with @domenic's proposal.

[cjohansen]

I don't think the trade-off in worse browser support for this is worth it at the moment. @domenic's proposal can be implemented at a later point, but I'm closing this issue for now.

[mightyiam]

Is it possible to mitigate this by only setting window.setTimeout and friends if a feature that requires them is used?

I'm assuming that most of Sinon's features do not require this.