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

Use Lolex as implementation for fake timers #5165

Closed
SimenB opened this issue Dec 23, 2017 · 16 comments · Fixed by #7776
Closed

Use Lolex as implementation for fake timers #5165

SimenB opened this issue Dec 23, 2017 · 16 comments · Fixed by #7776

Comments

@SimenB
Copy link
Member

SimenB commented Dec 23, 2017

Do you want to request a feature or report a bug?
Feature, I suppose.

What is the current behavior?
We currently implement fake timers ourselves. See docs and implementation.

If the current behavior is a bug, please provide the steps to reproduce and
either a repl.it demo through https://repl.it/languages/jest or a minimal
repository on GitHub that we can yarn install and yarn test.

N/A

What is the expected behavior?

Our current implementation has a couple of holes (see #3465 and #5147) both of which are covered by Lolex.

I think we would benefit from basically proxying Lolex through Jest's API to keep the "batteries included" feature.

I hit one issue when I spent some minutes looking into it to see if it was feasible while still keeping Jest's API mostly the same, sinonjs/fake-timers#146. Might be other issues, I haven't dug too deeply into it.

Please provide your exact Jest configuration and mention your Jest, node,
yarn/npm version and operating system.

N/A

@SimenB
Copy link
Member Author

SimenB commented Dec 23, 2017

Looked a tiny bit more (still no code written).

  • We currently have a runWithRealTimers method which is not really possible with Lolex' API, but it's not documented, so not a big deal*
  • Lolex exposes no way to clear timers. Probably worth an issue, but we can reset the properties manually.
  • No way of running micro tasks (nextTick, setImmediate) separate from macro tasks (timers), but I'm not sure if that's an issue in practice?*

*beyond the fact it's a breaking change

@benjamingr
Copy link
Contributor

@SimenB

  • lolex can expose runWithRealTimers if you make a compelling use case (I can see why I'd want that, especially for Node with microticks. It shouldn't be too hard to do with lolex - although it kind of breaks the abstraction.
  • lolex exposes .uninstall that clears timers, .runAll to run all timers (although we've found that in practice setInterval can break this because it will keep running unless cleared).
  • We can expose a way to run microtasks (setImmediate is a macrotask by the way) in lolex, lolex doesn't run microtasks (nextTick) by default though - you have to tell it to fake nextTick (just an API choice since people found it confusing with promises).

In any case, we can help from the lolex side :)

@SimenB
Copy link
Member Author

SimenB commented Dec 24, 2017

  • I'm not sure about use cases - I've never used it. An entirely unscientific GitHub search shows just type definitions and forks of jest, no actual usage. @cpojer is this used at FB?
  • Ah, of course. It also resets the mocks back to their original implementation, but calling uninstall followed by install should work fine.
  • Great!

setImmediate is a macrotask by the way

You learn something every day :D (it's correct in Jest's docs, I just skimmed it a bit too quick when trying to wrap my head around how we mock timers).

@Haroenv
Copy link
Contributor

Haroenv commented Dec 24, 2017

I’d love this, especially since it will also allow mocking for Date, which isn’t currently possible yet

@cpojer
Copy link
Member

cpojer commented Dec 24, 2017

This sounds exciting. I can provide data about FB in about 10 days but not earlier.

It’s important we retain all useful functionality. FakeTimers also exposes some legacy globals which I’d be happy to kill completely and codemod internally at FB.

At FB, we overwrite native Promise with the promise npm module, which uses asap, which uses setImmediate. This allows us to drive Promises using fake timers, which we used to do in the days before awesome async support in Jest. We need to make sure that a new fake timers implementation will retain the possibility to do this as we are unable to codemod all these calls automatically and probably have a couple hundred test suites depend on this behavior.

Shall we move forward with a PR to see how far we can get and what kind of changes would be required to lolex? The most important invariants are that it doesn’t regress performance at all and that the size of dependencies/modules stays mostly the same.

@SimenB
Copy link
Member Author

SimenB commented Dec 24, 2017

Shall we move forward with a PR to see how far we can get and what kind of changes would be required to lolex?

I have some done locally that I can push, but "from all of us to all of you" started on the tv, so I got distracted :D

@benjamingr
Copy link
Contributor

Hey, thanks @cpojer for weighing in:

At FB, we overwrite native Promise with the promise npm module, which uses asap, which uses setImmediate.

FWIW, you can overwrite native Promise with bluebird, which exposes setScheduler directly and would let you decide when promises resolve regardless. Note that this is tricky (what about async functions`?)

We might be able to add support for such scheduling in core (Node.js) - it sounds like a pretty compelling use case (controlling the order of things firing) which might be a better long term solution for async/await.

@SimenB
Copy link
Member Author

SimenB commented Dec 24, 2017

My branch with a single WIP commit https://github.com/SimenB/jest/tree/lolex

@SimenB
Copy link
Member Author

SimenB commented Dec 25, 2017

Another difference is in how runOnlyPendingTimers and runToLast work. In Jest we only run scheduled timers ignoring new ones added when doing so, while Lolex runs all new timers scheduled if they occur within the timeframe of the original pending timers.

I think I prefer Lolex' behavior, as it more closely simulates what happens in a real environment, but it's a difference nonetheless.

@SimenB
Copy link
Member Author

SimenB commented Dec 25, 2017

Opened up a PR with my branch, as I've pushed some more, and commenting on a branch is pretty weird: #5171. Not sure if it makes sense to keep this issue open or move the discussion to that PR.

@benjamingr
Copy link
Contributor

benjamingr commented May 14, 2018

@cpojer

At FB, we overwrite native Promise with the promise npm module, which uses asap, which uses setImmediate.

You can alternatively overwrite native Promise with Bluebird and call setScheduler directly to make schedulers work as you please.

asap may decide not to use setImmediate in a patch version at any time in the future and break all your tests - there is no guarantee for this behavior.

I also intend to bring this use case up at the upcoming Node.js collaborator summit if that's OK for something I'd like native promises to let you do, would that be fine with you?

@mgol
Copy link

mgol commented Nov 19, 2018

What is blocking this PR currently?

@SimenB
Copy link
Member Author

SimenB commented Nov 19, 2018

@mgol making sure it does not break FB's test suite 🙂 Or, it will, but make sure that they're able to migrate.

Current try there is #7300, which is less behavior breaking. Hopefully we'll be able to land that without too much issue. Then we can land #5171 🙂

@SimenB
Copy link
Member Author

SimenB commented May 3, 2020

2.5 years later... 🙌

@reisandbeans
Copy link

@SimenB thank you! I was waiting for this for quite a while now!

Quick question (as the docs are a bit new and I can't find this information anywhere): I know that sinon fake timers offers you the chance to choose what you want to mock (for example, mocking just Date, but not setTimeout).

This was very useful as some third party libraries like express have some internal calls to methods like setImmediate or something on those lines, and it's quite hard to know when you manually have to advance time or not in your tests. So if I explicitly want to mock just the Date object to force the timestamps in my snapshots to always match, I would do something like sinon.useFakeTimers(now, 'Date');

Would something like that be possible with jest fake timers?

@jeysal
Copy link
Contributor

jeysal commented Jun 7, 2020

@Tinadim there's no official way provided by Jest, but you can still use the API directly for advanced use cases I'd think https://github.com/sinonjs/fake-timers/tree/cb4c2f6e2eaddfd2c1be49f14e7f6d51f6633280#var-clock--faketimersinstallconfig

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants