[Feature]: Add paused option to clock.install() to start with a frozen clock

[dt-mkuster]

🚀 Feature Request

A paused option on page.clock.install() that installs fake timers with the clock already frozen, so no follow-up pauseAt() call with a guessed future timestamp is needed.

Example

await page.clock.install({ time: START, paused: true });

Replaces the current two-step workaround, where the pause target has to be a guessed timestamp ahead of the install time to avoid cannot fast forward to the past:

await page.clock.install({ time: START });
await page.clock.pauseAt(PAUSE); // must be > START by more than the setup drift

Motivation

install() not freezing is surprising — sinon and jest freeze by default, so time silently keeps running and you suspect your own code first. The workaround is also fragile: install({ time: T }) followed by pauseAt(T) intermittently throws cannot fast forward to the past, because real time drifts forward between the two calls. That forces you to pick an arbitrary later timestamp — too close and it flakes, too far and it silently eats into the window you're asserting on.

A paused option removes the second call entirely, so there's no timestamp to guess and no drift to race.

[dgozman]

Maciej Kuster (@dt-mkuster) Could you please share a real test where you'd use this? In our tests/examples, there's usually a page.goto() between install and pauseAt, because you have to install before the page starts loading and schedules any timers.

[dt-mkuster]

I'm using it after page.goto() but not sure if thats changes anything. Usage of the same value in install(time) and pauseAt(time) can cause the error that you cannot fastforward to the past because some time elapsed between functions call.

Shouldnt clock.install throw an error if used after page.goto if thats an issue?

[dgozman]

Well, you can call clock.install() at any moment, but as the big warning in the docs says, it must be done before the page uses timers that you might want to pause/fastForward/etc. Usually, that means installing before the goto() for a fully-controlled test, but sometimes it's fine to install in the middle of a test as well. We don't want to throw "after page.goto" because that's very ambiguous.

It would still be nice to see an actual test where you call these two lines one after another. I do understand that you cannot pass the same time to both, and that's why all the examples in the doc pass different values there. However, the usecase is still unclear, and we cannot make API decisions without understanding the usage first.

[dt-mkuster]

My usecase is that I want to setup a clock that is paused.

And my first thought was, I'm installing fake clock so I need to just call:

page.clock.install()

But after running tests multiple times I realized its flaky because time isn't paused.

So I tried:

page.clock.install({ time: 0 });

Same, flaky so I tried:

page.clock.install({time:0});
page.clock.pauseAt(0)

Started working.

On CI Failed because of exception "Cannot fast forward to the past"

So again I had to debug and realize I cannot pause at the same time because a milisecond can elapse between calls.

This is all about usage, its not anyhow related to the specific test.

I would expect:

1. clock be auto paused because I'm installing mocked one
2. at least have an option that I can pass to install`` function with clock.install({paused:true})` - to avoid race conditions between calling install and pauseAt
3. If its a breaking change and you dont want to change this I would expect that when I call clock.pauseAt(0) there is immediate check that there is provided the same value that I passed to install and get the error immediately with explanation - and now I get it on CI because of race conditions that depends on machine speed.

[dgozman]

Thank you for the response. I read through it, and you are just repeating the same information as before. Unfortunately, this does not move the issue forward.

I have concerns about calling these two methods in a row that I have raised above. We usually assume a page.goto() in between. Since you are not doing that, your usecase does not fit our understanding. Thus, we need a more detailed example with a real test that shows the actual usecase. If you keep repeating that you'd like to combine two functions into a single one, I don't see how we can make progress.

[dt-mkuster]

I think that race condition that throws an error randomly when developer provide a static time that he/she wants to stick with mocked clock is an usecase itself.

I can call both before page.goto() and get randomly an error thrown.

But please here is an example:

test('should not throw an error', async ({ page }) => {
  await page.clock.install({ time: 0 });
  await page.clock.pauseAt(0);
  await page.goto('https://google.com');
});

You can do it in between:

test('should not throw an error', async ({ page }) => {
  await page.clock.install({ time: 0 });
  await page.goto('https://google.com');
  await page.clock.pauseAt(0);
});

Same result.

Once per many runs it will fail.

If you want to reproduce it always just add a setTimeout in between:

test('should not throw an error', async ({ page }) => {
  await page.clock.install({ time: 0 });
  await new Promise((resolve) => setTimeout(resolve, 100));
  await page.clock.pauseAt(0);
  await page.goto('https://google.com');
});

With the approach:

test('should not throw an error', async ({ page }) => {
  await page.clock.install({ time: 0, paused: true }); // NEW OPTION HERE
  await page.goto('https://google.com');
});

this will never happen