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
794 stars 103 forks source link

fix global performance node 16 #412

Closed benjamingr closed 2 years ago

benjamingr commented 2 years ago

Purpose (TL;DR) - mandatory

Fixes performance on Node 16 which has a global performance but not a global Performance

cc @fatso83 @SimenB @itayperry

codecov[bot] commented 2 years ago

Codecov Report

Merging #412 (44309b4) into master (1b1ac4a) will decrease coverage by 0.11%. The diff coverage is 87.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #412      +/-   ##
==========================================
- Coverage   94.29%   94.17%   -0.12%     
==========================================
  Files           1        1              
  Lines         613      618       +5     
==========================================
+ Hits          578      582       +4     
- Misses         35       36       +1     
Flag Coverage Δ
unit 94.17% <87.50%> (-0.12%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/fake-timers-src.js 94.17% <87.50%> (-0.12%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1b1ac4a...44309b4. Read the comment docs.

benjamingr commented 2 years ago

Let's merge and open another PR to discuss versioning?

fatso83 commented 2 years ago

I am merging and ignoring the codecov failures due to dropped coverage, but it would be good if that was analyzed somehow so that we understand why. I am guessing the various "if x is present, run this, else that" is only covered if the tests are run in several runtimes (which it is), but are the codecov results just one of these runs or the aggregated result? If the former, then it's hard to do something about it, and if the latter, it is weird that we get a drop.

jakub-g commented 1 year ago

FWIW for future readers: using fake-timers@9.1.2 (via jest) with nodejs 16.3.0:

jest.useFakeTimers({
    now: ...
});

I still have

TypeError: Cannot assign to read only property 'performance' of object '[object global]'

Solution: either upgrade to nodejs 16.5.0+, or use

jest.useFakeTimers({
    now: ...
    doNotFake: ['performance'],
});