clock.performance.now returns incorrect value within clock.setInterval callback

[rrogowski]

• FakeTimers version : 7.1.2
• Environment : NodeJS
• Example URL : N/A; see test below
• Other libraries you are using: Jest

When passing fractional milliseconds to clock.setInterval and clock.tick, the return value of clock.performance.now is sometimes incorrect when called within the callback passed to clock.setInterval.

Here is an example test running within Jest:

import { Clock, install } from '@sinonjs/fake-timers';

let clock: Clock;

beforeAll(() => {
  clock = install();
});

afterAll(() => {
  clock.uninstall();
});

test('fake timers bug', () => {
  const timestamps: number[] = [];
  clock.setInterval(() => {
    timestamps.push(clock.performance.now());
  }, 16.6);

  clock.tick(16.6);
  clock.tick(16.6);

  expect(clock.performance.now()).toBe(33.2); // Passes
  expect(timestamps).toStrictEqual([16.6, 33.2]); // Fails
});

What did you expect to happen?

Each timestamp should be a multiple of 16.6.

What actually happens

The timestamp generated on the second iteration of the callback is off by 1 millisecond. The value 32.2 is returned rather than 33.2.

Interestingly, when calling clock.performance.now outside of the clock.setInterval callback, the correct timestamp is returned.

How to reproduce

See test sample above.

[rrogowski]

Here's another (potentially simpler) example I came up with:

import { Clock, install } from '@sinonjs/fake-timers';

let clock: Clock;

beforeAll(() => {
  clock = install();
});

afterAll(() => {
  clock.uninstall();
});

test('fake timers bug', () => {
  const callback = jest.fn();
  clock.setInterval(callback, 16.6);

  clock.tick(1660);

  expect(callback).toHaveBeenCalledTimes(100); // Fails
});

which fails with:

For my current use case, I don't care as much about the performance timestamp being 100% accurate inside the setInterval callback. I mostly care that the callback is fired exactly as many times as it should be (i.e. the second test example). But it'd be nice to have both! 😄

[benjamingr]

Thanks for the report!

Timers don't work with sub-millisecond durations - WebIDL conversions convert whatever you pass in to an integer and assumes it's milliseconds. (In Node.js we make even fewer guarantees with regards to timer order and granularity).

I think we correctly convert timers to ints here.

[fatso83]

I also generally would not do any precision comparisons with setTimeout nor setInterval. At least that would not match production timers in actual browsers, which is kind of the point, right? My rule of thumb has always been that setTimeout takes somewhere between 0 and 16ms, when doing setTimeout(fn,0). For instance, some browsers rely on the default Windows OS timers; these have a granularity of 15.6 ms. You can see some more data from the wild here:

• https://news.ycombinator.com/item?id=2328510
• https://stackoverflow.com/a/9288080/200987

So ... I am not seeing any clear bug here, as your second example implicitly relies on each timer to be approximately correct? I would add some "timeout padding"/delta for your test there, as it's almost like comparing floating point values.

const acceptableDelta = 30;
clock.tick(100*16.6 + delta);

[fatso83]

Also see point 17 in the spec: https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timers:implementation-defined-2

Optionally, wait a further implementation-defined length of time.

This is intended to allow user agents to pad timeouts as needed to optimize the power usage of the device. For example, some processors have a low-power mode where the granularity of timers is reduced; on such platforms, user agents can slow timers down to fit this schedule instead of requiring the processor to use the more accurate mode with its associated higher power usage.

[rrogowski]

@benjamingr @fatso83 Thank you both for these explanations. I think I understand the behavior a bit more clearly now, and that this is not a bug given the timer specs.

I am wondering what a good alternative would be to testing sub-millisecond durations. Currently, I'm writing an emulator that decrements a timer register at a rate of 60Hz (once every ~16.667s). Would it still be possible to use this library to test that behavior?

Perhaps I am thinking about it wrong. Maybe what I should actually be testing is that the timer register is decremented at a rate of 60Hz on average, rather than exactly 60 times in a single second.

[rrogowski]

To put it more generally, what is a more "correct" way to test code that uses setInterval when the duration is not an integer?

[fatso83]

Not totally sure what to say here. Code that uses a non-integer duration is essentially "wrong" (not spec-conforming), so this is a more general question of how would you test something that is not supported😃

I think your approach of testing the timers schedule updates at an approximately correct frequency should be fine, and it's basically what you did above. In any case, most JS runtimes do not make hard realtime guarantees, so whether your timer in practice gets called 51 times per minute or 62 times per minute is totally browser/node/runtime specific. So you are only really testing that fake-timers schedules a call every parseInt(16.6) millisecond. Not sure if we can do better than that.

[rrogowski]

@fatso83 Thank you for the thoughtful reply! You raise a good point about writing code that does not conform to the spec. I will re-think the code I am writing, which will hopefully make the functionality easier to test. I really appreciate your time on this matter, and I'm happy to close this issue as a non-bug!