fix(vitest): decouple fake/real time in waitFor()'s check

[samavos]

When running with fake timers, and when polling for promise to complete, make sure we do the poll in a short real time interval, independent of the fake time interval.

This makes it possible to use long time intervals in fake time without needing to wait for the corresponding amount of real time passing. A unit test is added to illustrate this behaviour: with the previous code, this unit test would have taken over a minute to complete, now it is near instantaneous.

[netlify]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	77428e8
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/671f8dba8830fb0008807b63
😎 Deploy Preview	https://deploy-preview-6802--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[sheremet-va]

Please, don't drop the PR template:

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

The first point is especially important as we can't evaluate the usefulness of this change without examples and a proper discussion.

[samavos]

Please, don't drop the PR template:

Apologies, I created the PR with gh from the command-line, which didn't include the template.

I was about to raise an issue, but thought the issue was very clear from a unit test, which I've included. If it's helpful to raise a issue and move the discussion there I'm happy to do that. But as it stands, there is a unit test and description here which would be the information I'd put in the issue.

Would you like to consider discuss here, or shall I move this to an issue?

[sheremet-va]

Would you like to consider discuss here, or shall I move this to an issue?

You can start here.

I was about to raise an issue, but thought the issue was very clear from a unit test, which I've included.

The reasoning doesn't really make it clear to me. The test is very artificial. The interval intentionally calls the real interval here. It is an assertion code, not the test code and shouldn't be affected by the fake timers. The fact that some timers are mocked doesn't imply that we can speed up the timer that was passed down by the user.

[samavos]

I was about to raise an issue, but thought the issue was very clear from a unit test, which I've included.

The reasoning doesn't really make it clear to me. The test is very artificial. The interval intentionally calls the real interval here. It is an assertion code, not the test code and shouldn't be affected by the fake timers. The fact that some timers are mocked doesn't imply that we can speed up the timer that was passed down by the user.

The test is artificial, but it's based on a real use-case in my project. I've just simplified it. The current behaviour causes real problems in a (closed) project I'm working on.

I believe the purpose of the interval parameter is to ensure that time moves forward by that interval for every check that waitFor runs. The problem with this definition is that it is ambiguous: does one mean real time or fake time? My personal definition was purely "whatever mode vitest timers are configured to". Given such a definition, when using fake timers, the real-time interval used is purely an implementation detail.

I'd be interested in knowing what your definition of the interval parameter is?

A way to change this to not introduce any change in behaviour is to add another configuration option for this case; but that of course increases the size of the API. I'd be equally happy with such an approach in my use-case.

[sheremet-va]

I'd be interested in knowing what your definition of the interval parameter is?

interval and timeout are real intervals and timeout. interval is the time when the callback should be called again if it fails.

A way to change this to not introduce any change in behaviour is to add another configuration option for this case; but that of course increases the size of the API. I'd be equally happy with such an approach in my use-case.

You can already provide your own interval and check for vi.isFakeTimers(). Note that it will return true if vi.useFakeTimers was called, but it's possible to provide your own toFake array without mocking interval or timeout.

[samavos]

I'd be interested in knowing what your definition of the interval parameter is?

interval and timeout are real intervals and timeout. interval is the time when the callback should be called again if it fails.

OK, fair enough.

A way to change this to not introduce any change in behaviour is to add another configuration option for this case; but that of course increases the size of the API. I'd be equally happy with such an approach in my use-case.

You can already provide your own interval and check for vi.isFakeTimers(). Note that it will return true if vi.useFakeTimers was called, but it's possible to provide your own toFake array without mocking interval or timeout.

I'm not entirely sure I follow that suggestion. In my case, I have the following scenario:

• A module that uses timers (Date.now(), setTimeout at least), so we want to use fake timers
• A module that can have fairly long wait times on setTimeout (e.g. 1 minute long) or setInterval
• Unit tests for the above, where we want to provoke and test behaviours that result in quite a lot of (fake) time needing to pass

I believe the right answer here is: vi.useFakeTimers(), and in many cases calling advanceTimersByTime() manually does what I need.

In the few cases I'd like to use waitFor(), there isn't any reasonable interval to set: I'd need to set it to be very high (e.g. 30 seconds), but now my unit test actually needs to take >= 30 seconds, rather than the milliseconds it could/should take.

Are you suggesting there is a way to use the API to solve for this case today without any modifications? I haven't quite been able to figure out what that is based on your last comment, sorry.

[sheremet-va]

In the few cases I'd like to use waitFor(), there isn't any reasonable interval to set: I'd need to set it to be very high (e.g. 30 seconds), but now my unit test actually needs to take >= 30 seconds, rather than the milliseconds it could/should take.

Why do you even need such a big interval? It means the callback will be called once every 30 seconds. Don't you need a timeout?

Are you suggesting there is a way to use the API to solve for this case today without any modifications? I haven't quite been able to figure out what that is based on your last comment, sorry.

Yeah, what you did in this PR looks like this in user code:

vi.waitFor(() => {}, {
  interval: vi.isFakeTimers() ? 40 : 30000
})

[samavos]

In the few cases I'd like to use waitFor(), there isn't any reasonable interval to set: I'd need to set it to be very high (e.g. 30 seconds), but now my unit test actually needs to take >= 30 seconds, rather than the milliseconds it could/should take.

Why do you even need such a big interval? It means the callback will be called once every 30 seconds. Don't you need a timeout?

Because we're testing code that really does, in the real world, use long intervals. One case that is hopefully easy to describe: (something using) an exponential backoff timer. Consider something with a backoff that quickly results in the backoff time being a long duration after some error conditions are hit. It is reasonable to check that something will succeed after some amount of backing off.

Are you suggesting there is a way to use the API to solve for this case today without any modifications? I haven't quite been able to figure out what that is based on your last comment, sorry.

Yeah, what you did in this PR looks like this in user code:

vi.waitFor(() => {}, {
  interval: vi.isFakeTimers() ? 40 : 30000
})

Sorry, I don't agree. Are we perhaps talking at cross purposes? There are two separate things here: the amount of fake time that is increased per time we check, and the amount of real time that needs to pass. Consider the unit test code here:

    vi.useFakeTimers()
    await vi.waitFor(() => {
      return new Promise<void>((resolve) => {
        setTimeout(() => {
          resolve()
        }, 60000)
      })
    }, { interval: 30000 })

Your suggestion is that we change the interval there to read:

// ... as above
    }, { interval: vi.isFakeTimers() ? 40 : 30000 })

In this trivialised case, this will obviously set the interval to 40.

• What will then happen (prior to my change) is the test will need to wait for a total of 60 seconds, checking the state of what is being waited for every 40ms. All configuring the interval to do is changes the number of times the condition is checked and timers advanced by the interval, not the amount of real time that passes.
• What I am trying to achieve is having the test not wait the full 60 seconds. My change is one approach that achieves this.

I had a look at dom-testing-library and it doesn't have the same behaviour as vitest does. It's actually even more extreme than my version: it doesn't wait for any real time to advance, just relies on doing an await running other pending microtasks. Their initial implementation for fake-timers is here and the current version here. I'm not sure if you would consider it desirable to have the same semantics as that?

[sheremet-va]

We talked within the team about this issue. We still don't understand the use case. Can you give an actual code example that helps understand this? And can you explain why you need to use vi.waitFor there?