Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Needs to save references to nextTick and friends to avoid side-effects from stubbing #292

Closed
fatso83 opened this issue Jun 8, 2016 · 2 comments
Labels

Comments

@fatso83
Copy link

fatso83 commented Jun 8, 2016

We got a bug report our way in sinonjs/fake-timers#66, which basically boils down to that tape ends up using stubbed timing functions and gets stuck. Mocha solves this issue by saving references to all global timing related functions, such as setTimeout, setInterval, nextTick, ..., so that later modifications will not interfere with its internal use of them.

This would be the easiest fix.

Relevant lines that uses nextTick:

@yoshuawuyts
Copy link

Yeah, seems like a reasonable way of resolving this - though one might also argue that clocks should never be messed with. But it probably has its uses, so hey

See Also:

@ljharb
Copy link
Collaborator

ljharb commented Jun 8, 2016

Absolutely tape should be robust against modification of globals - thanks for the report!

Please file one on through for dominictarr/through@903501a#diff-168726dbe96b3ce427e7fedce31bb0bcR49 - that's a separate repo.

Your given example though is indeed a cached version of process.nextTick - there's one example of setTimeout in tape code, which I'll fix shortly.

@ljharb ljharb closed this as completed in f2351ca Jun 8, 2016
@ljharb ljharb added the bug label Jun 8, 2016
ljharb added a commit that referenced this issue Jun 20, 2016
 - [New] make object-inspect depth configurable for expected/actual (#293)
 - [New] add message defaults to .ok() and .notOk() (#261)
 - [Robustness] be robust against the global `setTimeout` changing (#292)
 - [Deps] update `glob`, `object-inspect`
 - [Dev Deps] update `js-yaml`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants