sinonjs / fake-timers

Fake setTimeout and friends (collectively known as "timers"). Useful in your JavaScript tests. Extracted from Sinon.JS
BSD 3-Clause "New" or "Revised" License
793 stars 103 forks source link

[Feature]: In Node, hijack perf_hooks.performance in addition to global.performance #462

Open kevinyou opened 1 year ago

kevinyou commented 1 year ago

What did you expect to happen? After calling useFakeTimers(), perf_hooks.performance.now() should return a fake time, just like how performance.now() does.

What actually happens 1. perf_hooks.performance.now() is not the same as performance.now(). The latter is correctly hijacked and returns a fake time. 2. perf_hooks.performance.now() returns a negative value -- undefined behavior?

How to reproduce https://github.com/kevinyou/wtf-performance-now-mocking/tree/main/sinon-fake-timers

var perf_hooks = require('perf_hooks');
var FakeTimers = require("@sinonjs/fake-timers");

console.log(perf_hooks.performance === performance); // true
console.log(perf_hooks.performance.now()); // 36.97510700020939
console.log(performance.now()); // 37.08340699970722

var clock = FakeTimers.install({
    now: Date.now(),
    shouldClearNativeTimers: true,
});
console.log(perf_hooks.performance === performance); // now false
console.log(perf_hooks.performance.now()); // -4217337.086744
console.log(performance.now()); // 0
fatso83 commented 1 year ago

This is starting to get a bit exotic ... Think you could get away with simply faking the required exports yourself?

kevinyou commented 1 year ago

Yes, as a workaround I did that (in Jest) and it works:

jest.mock('perf_hooks', () => ({
    performance: {
        now: () => global.performance.now(),
    },
}));

But as a user, it was very confusing until I dug deep to the fake-timers source code. The documentation talks about overriding performance.now() but whether this applies to the one in perf_hooks was unclear. Also, it wasn't until Node v16 that performance became a global so older libraries still access it through perf_hooks.

Maybe the documentation could have a disclaimer about performance in perf_hooks, like in the config.toFake row notes.

benjamingr commented 1 year ago

I think it's probably fine to support it and not a lot of work?

I'm happy to make changes in Node if that'd help

fatso83 commented 1 year ago

@benjamingr I am at a bit of a loss as to how to do this cleanly with just using this library, so if you have a way of doing this cleanly by exposing something in Node, then by all means feel free ❤️

stale[bot] commented 6 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.