examples icon indicating copy to clipboard operation
examples copied to clipboard

Karma example is unstable

Open vitalii-retel opened this issue 4 years ago • 7 comments

Description of an issue

It is quite simple. Just clone the repo. Then run with-karma example. Run the tests a few times in a row. Observe that sometimes they fail.

After some investigation it was found that adding additional timeout, after starting the worker, solves the issue (looks like that at least).

this.beforeAll(async () => {
    await worker.start();
    // add timeout, 1s
    await new Promise((resolve) => window.setTimeout(resolve, 1000));
})

Environment

  • os: macOS

vitalii-retel avatar Apr 23 '21 19:04 vitalii-retel

Modifying beforeAll shows the state of the service worker is still installing on the first run:

  this.beforeAll(async () => {
    const registration = await worker.start()
    console.log('registration: ', registration.installing, registration.waiting, registration.active)
  })

Subsequent runs in watch mode work fine.

I would expect the service worker is in the activated state after starting the worker through msw: https://developer.mozilla.org/en-US/docs/Web/API/ServiceWorker/state

Bug in msw?

wwsno avatar Jun 08 '21 07:06 wwsno

Hey, @vitalii-retel. Thanks for reporting this.

The worker's state doesn't seem valid, however, as the worker.start() won't resolve until the worker has been installed, activated, and messaged back the library about that:

https://github.com/mswjs/msw/blob/2213b4ebd78c8459d8d9f39d8dd1d268337b79d8/src/setupWorker/start/createStartHandler.ts#L86-L88

The fact that it doesn't work in Karma while working reliably in browsers, may suggest there is something different going on in the test setup. Of course, there's always room for an issue, but at this point, I doubt that.

Would somebody be willing to investigate this further? Our team would help with the code navigation and reviews.

kettanaito avatar Jun 08 '21 09:06 kettanaito

It seems HeadlessChrome/90.0.4427.0 introduces the instability (puppeteer@8). With HeadlessChrome/90.0.4403.0 I cannot reproduce the issue (puppeteer@7). Maybe someone else can check if they see the same thing?

The integration tests in the msw repository seem to use HeadlessChrome/90.0.4392.0. However I do not know how to change the chrome version in those tests, as it seems to be controlled through the page-with dependency?

wwsno avatar Jun 08 '21 13:06 wwsno

It seems HeadlessChrome/90.0.4427.0 introduces the instability (puppeteer@8). With HeadlessChrome/90.0.4403.0 I cannot reproduce the issue (puppeteer@7). Maybe someone else can check if they see the same thing?

The integration tests in the msw repository seem to use HeadlessChrome/90.0.4392.0. However I do not know how to change the chrome version in those tests, as it seems to be controlled through the page-with dependency?

@wwsno

Did you ever find a reliable answer to this? For now putting a timeout of 50ms works fine for me, but it feels FILTHY!

Maybe related: https://github.com/mswjs/msw/issues/854

rjerue avatar Aug 12 '21 16:08 rjerue

No real solution. Up till now we work around it by calling update after start:

  this.beforeAll(async () => {
    const registration = await worker.start();
    await registration.update();
  })

wwjhu avatar Aug 12 '21 16:08 wwjhu

Thanks for finding that out, @wwsno. Chrome version is pinned by page-with, as you've correctly pointed. It's worth mentioning that we're not using Puppeteer for testing but Playwright instead. Is that issue reproducible with Playwright as well?

I'd like to learn more about the instability that the new version of Chrome introduced. Can somebody please share a link to that issue?

kettanaito avatar Aug 31 '21 13:08 kettanaito

It seems HeadlessChrome/90.0.4427.0 introduces the instability (puppeteer@8). With HeadlessChrome/90.0.4403.0 I cannot reproduce the issue (puppeteer@7). Maybe someone else can check if they see the same thing? The integration tests in the msw repository seem to use HeadlessChrome/90.0.4392.0. However I do not know how to change the chrome version in those tests, as it seems to be controlled through the page-with dependency?

@wwsno

Did you ever find a reliable answer to this? For now putting a timeout of 50ms works fine for me, but it feels FILTHY!

Maybe related: mswjs/msw#854

We are no longer seeing this issue and removed the work-around. Chrome Headless 101.0.4950.0 and [email protected].

wwsno avatar Jul 20 '22 07:07 wwsno