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

[Bug]: Jest 27 - useFakeTimers erases performance.mark and friends #12055

Closed
vovacodes opened this issue Nov 9, 2021 · 11 comments · Fixed by #12572
Closed

[Bug]: Jest 27 - useFakeTimers erases performance.mark and friends #12055

vovacodes opened this issue Nov 9, 2021 · 11 comments · Fixed by #12572

Comments

@vovacodes
Copy link

vovacodes commented Nov 9, 2021

Version

27.3.1

Steps to reproduce

window.performance.mark = () => {}
jest.useFakeTimers()
assert(typeof window.performance.mark === 'undefined')

jest.useRealTimers()
assert(typeof window.performance.mark === 'function')

jest.useFakeTimers("legacy")
assert(typeof window.performance.mark === 'function')

Expected behavior

jest.useFakeTimers() to not override global polyfills/mocks of performance.mark

Actual behavior

jest.useFakeTimers() overrides global polyfills/mocks of performance.mark, and sets it to undefined.

Additional context

I found a similar issue in @sinonjs/fake-timers sinonjs/fake-timers#136. Don't know if it's related to jest though.

Environment

System:
    OS: macOS 12.0.1
    CPU: (10) arm64 Apple M1 Pro
Binaries:
    Node: 16.13.0 - /usr/local/bin/node
    Yarn: 3.0.0-rc.9 - /opt/homebrew/bin/yarn
    npm: 8.1.0 - /usr/local/bin/npm
@mrazauskas
Copy link
Contributor

Not clear why this is an issue? Could you describe some use case, please.

Or you try to say that Mock Timers documentation should also mention performance similarly to the docs of @sinonjs/fake-timers?

@vovacodes
Copy link
Author

The issue is that there is no way for a developer currently to mock performance.mark() because useFakeTimers overrides the mock set up in a test setup file, and at the same time it doesn't provide its own mock for the method.

Let me know if more clarification is needed

@SimenB
Copy link
Member

SimenB commented Nov 10, 2021

Is it only an issue for modern timers? If so, this needs to be fixed upstream in sinon I think

@mrazauskas
Copy link
Contributor

If I got it right, Sinon is adding performance to clock.performance.now(). See: https://github.com/sinonjs/fake-timers#clockperformancenow

It seems like window.performance is erased, because this is a read only prop. And then they make it available on clock object.

At least two options: either to expose clock.performance somehow in Jest; or to not erase it. But this also would mean that it will not be mocked. That’s why I was curious about the use case. How to test this? What is the need?

All this is only modern timers related. Legacy timers leave performance. If that works as expected, then perhaps modern timers also should not touch performance. Hm.. It sounds better to expose clock.performance somehow.

@SimenB
Copy link
Member

SimenB commented Nov 10, 2021

They have some node 16 issues, might be related sinonjs/fake-timers#394

@vovacodes
Copy link
Author

The use-case is pretty straight-forward:

  • I have some code that does performance.mark() somewhere deeply in its implementation
  • I want to test some parts of this code and I don't care about performance.mark() and I'm fine with it just being a noop.
  • I put a "global" stub for performance.mark() in my setupFile.js
  • I expect that performance.mark() always will be around so my tests don't crash.

I'd be ok with any way of making it happen, even things like jest.useFakeTimers({ ignore: [window.performance] }) or whatever

@vovacodes
Copy link
Author

Sorry just realized my code snippet in the description was incorrect and confusing, updated it now.

@fatso83
Copy link
Contributor

fatso83 commented Dec 23, 2021

I think this issue could be fixed pretty easily be just making avoiding to fake performance. This is already possible by telling @sinonjs/fake-timers, but we have just forgot to update our docs ... 🙈 Ref sinonjs/fake-timers#409, DefinitelyTyped/DefinitelyTyped#56961 (review) and sinonjs/fake-timers#374.

@SimenB I am not to well versed in Jest's use of fake timers, but I do not see any way of passing options to customise its timer faking. If we could exclude performance this issue would go away.
p.s. Thanks for the ☕

@SimenB
Copy link
Member

SimenB commented Apr 5, 2022

@shmuli9
Copy link

shmuli9 commented Apr 13, 2022

@benjaminhr - please take a look

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants