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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Component Testing]: Service workers support #30981

Open
yannbf opened this issue May 23, 2024 · 7 comments
Open

[Component Testing]: Service workers support #30981

yannbf opened this issue May 23, 2024 · 7 comments

Comments

@yannbf
Copy link

yannbf commented May 23, 2024

馃殌 Feature Request

Seems that Playwright CT blocks service workers and is non-configurable: https://github.com/microsoft/playwright/blob/main/packages/playwright-ct-core/src/mount.ts#L43

If there was a way to allow service workers, or maybe specify which service workers could be allowed, it would be fantastic.

Example

Check the logs to see the Service worker registration blocked by Playwright failure:

image

And if I switch from block to allow in this line in /node_modules/@playwright/experimental-ct-core/lib/mount.js, things work as expected:

image

Motivation

Using mocking tools such as MSW has become increasingly more common, and tools like MSW are used and shared across different environments e.g. Node with Vitest, Browser with Storybook, as well as Browser in the user's application. Ideally, users should be able to reuse their mocking setup in Playwright CT tests as well.

Storybook released a new API that helps adopt Playwright CT while reusing their Storybook stories. However for set ups that use MSW, the components will render correctly in Storybook, but the tests will fail in Playwright with a message mentioning that service workers are blocked.

Extra context

Seems like service workers were configurable from this PR:
#14714

However later on this PR changed the code so that it's always blocked:
#15477

Unfortunately there isn't much context in the latter PR, it would be amazing to know whether there are limitations that won't allow the usage of service workers or that maybe there is a chance to just make it configurable back again.

@pavelfeldman pavelfeldman added feature-components open-to-a-pull-request The feature request looks good, we are open to reviewing a PR v1.46 and removed open-to-a-pull-request The feature request looks good, we are open to reviewing a PR labels May 23, 2024
@pavelfeldman
Copy link
Member

Discussion:

  • retrofit MSW apis on top of route
  • allow MSW as a fixture for existing mocks reuse.
  • do that as an experiment inside CT family

@kettanaito
Copy link

@pavelfeldman, I'd love to hear more about the context of intentionally disabling service workers in the Component Testing feature of Playwright.

Removing this single line sounds like a fix to the problem:

serviceWorkers: 'block',

Once we know the history behind this change, we can have a more productive discussions on how to move forward.

@dgozman
Copy link
Contributor

dgozman commented May 29, 2024

@kettanaito Thank you for your interest in this issue! We are always happy to collaborate for the better developer experience.

We disable service workers in component testing so that multiple tests can reuse the same page/context. Otherwise, two tests could assume they are controlled by their own service workers and run into a conflict.

@kettanaito
Copy link

Thanks for providing the insights, @dgozman!

Otherwise, two tests could assume they are controlled by their own service workers and run into a conflict.

Can you share the exact situations when this happened?

I find it extremely unlikely that a component would register a Service Worker. Components, in the context of React or Vue components, often have their own life cycles. It's not recommended to put side effects like navigator.serviceWorker.register() in those life cycles. That's why Service Workers are commonly registered at the root of the entire app, not at the individual component's level.

But even if we do run into the case of two components registering a worker in the same Playwright context, I can't see how that would cause conflicts. The only case when it may be problematic is this:

// component-a.js
navigator.serviceWorker.register('/a.js', { scope: '/a' })
// component-b.js
navigator.serviceWorker.register('/b.js', { scope: '/b' })

Recap: two components registering two different service workers for different paths.

If Playwright runs both of these components in the same context (the same location in the browser), that won't really conflict but instead will result in not working Service Workers at all (the path in the test would be /, which matches none of the workers).

Even at that, this is such an unlikely scenario that I'd ask you to reconsider the decision to disable Service Workers for everyone.

I can also argue that as a developer using Playwright for component testing and running into this edge case of mixed workers, I can fix it by configuring my tests to run the two components in two different Playwright contexts. And on top of that, I'd need a proper test setup anyway to have the paths in the test match the scopes of my workers.

@dgozman
Copy link
Contributor

dgozman commented May 30, 2024

@kettanaito I agree that component tests registering conflicting service workers is unlikely. However, we have not seen anyone wanting to register a service worker for component tests either. Most likely, such a service worker would interfere with the tests, for example by serving stale content from the cache.

The MSW scenario seems like the first usecase where service worker in the component test would be beneficial. How do you imagine this looking from the user perspective?

This comment captures a discussion where we think that registering MSW handlers in the test would be easier to use than registering handlers in the test harness inside the page through the service worker. For example, something like that:

import handlers from '@src/mock/handlers';
import Component from '@src/components/Component';
import { http, HttpResponse } from 'msw';

// Register general mocks for all tests.
test.beforeEach(({ msw }) => msw.use(...handlers));

test('component test', async ({ mount, msw }) => {
  // Register a handler specifically for this test.
  await msw.use(http.get('/special', () => HttpResponse.json('special')));

  const component = await mount(<Component />);
  // ... testing goes here
});

Let me know what you think.

@MattyBalaam
Copy link

MattyBalaam commented Jun 6, 2024

@dgozman I鈥檝e tried before to get MSW to work in our suite of tests so we can migrate our mix of jest(-dom) and Cypress tests over to Playwright and this was a blocker for us to do it.

@kettanaito
Copy link

kettanaito commented Jun 7, 2024

The MSW scenario seems like the first usecase where service worker in the component test would be beneficial. How do you imagine this looking from the user perspective?

I imagine the user would follow the regular browser integration to have MSW set up for the browser during the component test run. Since MSW is enabled for the environment, you are telling Playwright: "okay, for the duration of these component tests, use this network description". Nothing is unique in this use case, the folks who've been using MSW keep using it the same in component testing.

#30981 (comment) captures a discussion where we think that registering MSW handlers in the test would be easier to use than registering handlers in the test harness inside the page through the service worker.

There is no concept of "registering" a request handler. Request handler is just a plain function that describes what to intercept and what responses to produce.

You can, indeed, use the request handler model for anything as our API allows calling handlers in a standalone mode, outside of mocking, for any purpose you really want. This is relevant only to Playwright, not the user (users are not meant to call handlers directly; they rely on integrations like setupWorker or setupServer that provision the best interception algorithm available for their respective environments).

Now, to the thought of routing request handlers through something like page.route(), I'd be against it. That is a workaround to a problem we haven't even acknowledged (because I don't see registering MSW's worker as a problem in any context during component testing). All this will achieve is perceived congruence while different runtime behavior. I really don't want people to report MSW bugs because page.route() behaves differently from how setupWorker depends.

The core value of MSW is to give the user the identical behavior for the same environment. setupWorker behaves identically in a plain browser, in Playwright, in Cypress, in Electron, in Karma, etc. We don't tailor to particular frameworks or libraries, and that's an essential design decision that allows people to have a great API mocking experience.

How to resolve this

Here's my proposal on how to resolve this question so both MSW and Playwright users win.

  1. Enable Service Workers in component tests.
  2. In the Playwright docs on component testing, mention that you should be cautious if your component registers a Service Worker since it will affect the test run. This is an edge case warning to make it clear to Playwright users what would happen (I can argue that people using workers already know this but you can never be too safe).
  3. As a result of these steps, allow both page.route() and setupWorker(). Let the use choose. I have nothing against page.route(). Folks using it know they are using it, they know how it behaves, it's features and limitations. The same goes for setupWorker(). The important bit here is they know what they use. It's not one tech masquerading for another.

And, as with any tooling, let your users tell you what works for them. What happens now is this: people are blocked because they cannot use MSW in component testing for Playwright. They aren't complaining about their component registering an arbitrary service worker and that breaks things for them (they'd be, arguably, at fault for that). The two things we are discussing here is an objective immediate issue and a hypothetical, highly improbable issue sometime in the future. I suggest we fix what's broken, then observe and learn from the users if they ever encounter service workers in tests being an issue. Because I think they won't ever encounter that.

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

No branches or pull requests

5 participants