stefanpenner / es6-promise

A polyfill for ES6-style Promises
MIT License
7.29k stars 594 forks source link

Increase compatiblity with faking time in test tools #222

Closed fpirsch closed 8 years ago

fpirsch commented 8 years ago

Caching a reference to the global setTimeout is necessary to be compatible with test libraries that fake time, like sinon.js See https://github.com/taylorhakes/promise-polyfill/pull/15

stefanpenner commented 8 years ago

@fpirsch i don't think this is a good idea, because depend on the platform we will choose a different form of async. This means your stubbing wont work on all platforms.

stefanpenner commented 8 years ago

if you want to replace async entirely for mocking: https://github.com/stefanpenner/es6-promise/blob/master/lib/es6-promise/promise.js#L153 using Promise._setAsap might be appropriate, as you will get all scenarios (with the polyfil, but not native promises)

fpirsch commented 8 years ago

@stefanpenner Thanks for your quick answer.

Maybe I need to better explain the issue. We want to be able to fake time no matter if/which promise polyfill is used, so we don't want anything es6-promise specific like Promise._setAsap. When faking time in PhantomJS 1.9, some polyfilled-promises are never resolved. promise-polyfill is fixed, es6-promise still has the issue.

See the discussion here: https://github.com/sinonjs/sinon/issues/738#issuecomment-238255860 and an example here: faketimers

AFAIK the problem only arises with setTimeout, the only function used by es6-promise and modified by fake timers like sinon.js

stefanpenner commented 8 years ago

@fpirsch oh i see, so this is actually to prevent faking the timer used here?

fpirsch commented 8 years ago

I don't know exactly what's going wrong since it's so dependent on software versions. But the point seems to be that promise polyfills need to use clean timers instead of the faked ones.

stefanpenner commented 8 years ago

@fpirsch agreed, im good with this. Thanks! I've been abit busy lately but hope to get a new release out soon.