Wrap global.setTimeout to work with mocks

[dax]

When a mock testing library (like sinonjs) override global.setTimeout, bluebird
was keeping the original version and was not using the mocked one.
By wrapping the call to global.setTimeout, bluebird will always use the
correct setTimeout implementation.

[spion]

Rather than every single library that saves setTimeout fixing this to become compatible with sinonjs, wouldn't it be much better if the fix was in sinonjs? It would be done by replacing setTimeout permanently and making sure that sinon.js is loaded first. It would choose whether to use the original or not internally depending on a flag)

[dax]

I am not fan of forcing module load order. Everytime I had to deal with that kind of assertion, it led to some time consuming bugs.

In the case of this PR, why not always use global.setTimeout, and create a wrapped global object (in the global.js module) with the expected setTimeout implementation only when it does not implement the signature bluebird expects ?

[spion]

Thats fine, I think that @petkaantonov will accept the patch, I was just wondering if it isn't extremely tedious to hunt down all libraries that save global functions. I think that it might be a failing of the testing library and should be examined.

Mutating shared globals automatically implies some kind of ordering - its an inevitable consequence. Setting up the fake environment before loading the modules that are going to be using it makes a lot of sense to me.

So rather than swapping functions around, sinon.js could just do this once:

function setTimeoutReplacement() {
  if (fakeMode) setTimeoutFake.call(void 0, arguments);
  else setTimeoutReal.call(void 0, arguments);
}

and if loaded first will always have complete control over what gets called.

[benjamingr]

I'm with @gorgi here, it is changing global state after all.

On 28 במרץ 2014, at 13:50, Gorgi Kosev [email protected] wrote:

Thats fine, I think that @petkaantonov will accept the patch, I was just wondering if it isn't extremely tedious to hunt down all libraries that save global functions. I think that its a failing of the testing library which should be examined.

Mutating shared globals automatically implies some kind of ordering - its an inevitable consequence. Setting up the fake environment before loading the modules that are going to be using it makes a lot of sense to me.

So rather than swapping functions around, sinon.js could just do this once:

function setTimeoutReplacement() {
if (fakeMode) setTimeoutFake.call(void 0, arguments);
else setTimeoutReal.call(void 0, arguments);
}
and if loaded first will always have complete control over what gets called.

—
Reply to this email directly or view it on GitHub.