Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Karma example is unstable #56

Closed
vitalii-retel opened this issue Apr 23, 2021 · 7 comments · Fixed by #90
Closed

Karma example is unstable #56

vitalii-retel opened this issue Apr 23, 2021 · 7 comments · Fixed by #90

Comments

@vitalii-retel
Copy link

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
@wwsno
Copy link

wwsno commented Jun 8, 2021

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?

@kettanaito
Copy link
Member

kettanaito commented Jun 8, 2021

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.

@wwsno
Copy link

wwsno commented Jun 8, 2021

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?

@rjerue
Copy link

rjerue commented Aug 12, 2021

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

@wwjhu
Copy link

wwjhu commented Aug 12, 2021

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();
  })

@kettanaito
Copy link
Member

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?

@wwsno
Copy link

wwsno commented Jul 20, 2022

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].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants