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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fake timers don't work for performance.now() #4004

Open
6 tasks done
satelllte opened this issue Aug 22, 2023 · 7 comments
Open
6 tasks done

Fake timers don't work for performance.now() #4004

satelllte opened this issue Aug 22, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@satelllte
Copy link

satelllte commented Aug 22, 2023

Describe the bug

Hi 馃憢

I'm not sure if this a bug or not, but I noticed that Vitest fake timers don't work with High precision timing Web API, specifically for performance.now().

Is it possible to support it?

Reproduction

Code snippet:

satelllte/adsr#46

Stdout example:

> npm run test -- fakeTimers.test.ts

stdout | fakeTimers.test.ts > fake timers > works for performance.now?
(before) performance.now() [1] |  1802.5662189722061
(before) performance.now() [2] |  1803.9706729650497
(before) performance.now() [3] |  1804.0636450052261
(after) performance.now() [1] |  1804.601235985756
(after) performance.now() [2] |  1804.7059119939804
(after) performance.now() [3] |  1804.8976829051971

System Info

System:
    OS: macOS 13.4.1
    CPU: (8) x64 Intel(R) Core(TM) i7-8557U CPU @ 1.70GHz
    Memory: 151.96 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.16.1 - ~/.nvm/versions/node/v18.16.1/bin/node
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.1/bin/npm
  Browsers:
    Chrome: 116.0.5845.96
    Chrome Canary: 118.0.5963.0
    Safari: 16.5.2
  npmPackages:
    vitest: 0.34.1 => 0.34.1

Used Package Manager

npm

Validations

@sheremet-va
Copy link
Member

It doesn't do this by default yet, but you can enable it with fake timers config:

import { configDefaults } from 'vitest/config'
export default {
  test: {
    toFake: [...configDefaults.toFake, 'performance']
  }
}

@sheremet-va sheremet-va added enhancement New feature or request and removed pending triage labels Aug 23, 2023
@satelllte
Copy link
Author

@sheremet-va amazing, thank you so much!

It works now with this config in my particular case (100% TypeScript):

/* vitest.config.ts */
import {defineConfig, configDefaults} from 'vitest/config';

export default defineConfig({
  test: {
    fakeTimers: {
      toFake: [...(configDefaults.fakeTimers.toFake ?? []), 'performance'],
    },
  },
});

Reference: satelllte/adsr@aceb6e1

Stdout:

(before) performance.now() [1] |  0
(before) performance.now() [2] |  0
(before) performance.now() [3] |  0
(after) performance.now() [1] |  5000
(after) performance.now() [2] |  5000
(after) performance.now() [3] |  5000

@gautelo
Copy link

gautelo commented Feb 13, 2024

This one really got me as well. Apparently, while the vitest documentation for toFake does mention that it omits nextTick() and queueMicrotask(), it doesn't mention that it also omits 'performance'. Nor does it mention any of the other methods included by fake timers by default. If you have a look the FakeMethod type it's showing 15 method names, yet vitest's toFake defaults to using only 7 of them.

Leaves me uneasy knowing you're not passing through defaults, but using your own and not documenting discrepancies appropriately. Like, how can I trust any defaults after this? :(

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Feb 14, 2024

It looks like there is a slight difference with jest about what's faked by default:
https://jestjs.io/docs/jest-object#jestusefaketimersfaketimersconfig

Fake timers will swap out Date, performance.now(), queueMicrotask(), setImmediate(), clearImmediate(), setInterval(), clearInterval(), setTimeout(), clearTimeout() with an implementation that gets its time from the fake clock.

In Node environment process.hrtime, process.nextTick() and in JSDOM environment requestAnimationFrame(), cancelAnimationFrame(), requestIdleCallback(), cancelIdleCallback() will be replaced as well.

while Vitest has:

toFake: [
'setTimeout',
'clearTimeout',
'setInterval',
'clearInterval',
'setImmediate',
'clearImmediate',
'Date',
],

(Probably new API like performance gets added after this default is defined in #1261?)

I don't think we can just suddenly align this with jest (also nextTick issue), so probably adding documentation + jsdoc is the best we can do for now.

@gautelo
Copy link

gautelo commented Feb 14, 2024

Vite and vitest is opinionated. That's a fine choice, but it does require that the opinions are well documented, so clearing this up in the docs would be good, and sufficient.

@sheremet-va
Copy link
Member

sheremet-va commented Feb 14, 2024

(Probably new API like performance gets added after this default is defined in #1261?)

Actually, the reason why we define those keys as default is because this was specified as the default in fake-timers README or docs. We never intended to be opinionated. Ideally, we should probably just remove it from defaults.

@gautelo
Copy link

gautelo commented Feb 14, 2024

Always hard knowing where the flakiness starts and ends in the JS ecosystem, unfortunately.. :( Sorry to direct my frustration at you if indeed the issue started in fake-timers readme and not here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

No branches or pull requests

4 participants