microsoft / playwright

Playwright is a framework for Web Testing and Automation. It allows testing Chromium, Firefox and WebKit with a single API.
https://playwright.dev
Apache License 2.0
66.1k stars 3.61k forks source link

[Component Testing]: Service workers support #30981

Closed yannbf closed 3 months ago

yannbf commented 4 months ago

🚀 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: https://github.com/microsoft/playwright/pull/14714

However later on this PR changed the code so that it's always blocked: https://github.com/microsoft/playwright/pull/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 commented 4 months ago

Discussion:

kettanaito commented 4 months ago

@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:

https://github.com/microsoft/playwright/blob/20a23b34850723ffa187a8f54a5ce6b36e33ac9b/packages/playwright-ct-core/src/mount.ts#L43

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

dgozman commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

@dgozman I’ve 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 commented 4 months ago

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.

https://github.com/microsoft/playwright/issues/30981#issuecomment-2127630076 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.

pavelfeldman commented 3 months ago

Enable Service Workers in component tests.

Playwright's core priorities are isolation and speed. You may refer to the isolation issues as as hypothetical and/or highly improbable, but those are our core values and they are likely to stay.

The expectations for the component tests are that they are on the faster side, so we are reusing the browser context between the tests when running component tests. Service workers are naturally scoped to the browser contexts in the browsers, so installing them in a test would leak between the tests and hence affect the isolation. At this point we are lacking native instrumentation to clear the sw registry across the browsers and we don't trust the web-facing APIs to do it reliably.

The core value of MSW is to give the user the identical behavior for the same environment.

I think our approach adheres to this principle. Just as you have different backends for MSW in the Web page (base on SW) and in Node (based on API overrides), we are adding those based on Browser Network Stack instrumentation and potentially based on the HTTP proxy. That would extend MSW-syntax handlers to the full range of our e2e customers that can't use it now due to the SW limitations. In this case, we call Web page, Node.js, Playwright Test different environments that might have diverging behaviors based on the powers of the underlying technologies.

A middle ground

We could try hacking in a way that would allow installing SWs from the testing harness rather than from the web content. That way we could install SW worker once and then control / reset it from the test harness. But this still sounds like a potential source of the isolation issues.

MattyBalaam commented 3 months ago

@pavelfeldman so from a practical poinf of view as a user of Playwright the planned way forward is for a similar API for creating responses as in this PR: https://github.com/microsoft/playwright/pull/31154 ? Is there anything a potential contributer can do to help this process along?

kettanaito commented 3 months ago

Service workers are naturally scoped to the browser contexts in the browsers, so installing them in a test would leak between the tests and hence affect the isolation.

I've provided an example solution above. You can achieve the isolation you wish by serving different components on different paths in the browser. No context would leak then. No performance degradation would be present.

I dislike the direction of this. I really hoped Playwright would be the answer to the in-browser component testing. Now you are cutting off a huge chunk of developers by restricting the service worker API in Playwright. You are making decisions on behalf of your users based on assumptions. I believe that is not how software should be designed, but I trust you know better for your users.

Looks like I'd have to hack something to provide an adequate developer experience for MSW users. I will not recommend the MSW-like fixtures you suggest, I still think that is a bad idea and I highly encourage you to reconsider.

kettanaito commented 3 months ago

At least expose the means to opt-out of this behavior if people wish to. If that breaks their test isolation, they will revert. If it doesn't, they will benefit from seamless API mocking. Everybody wins.

Please make this property parametric:

https://github.com/microsoft/playwright/blob/122818c62c129455862246a2f6cac57e8fb39321/packages/playwright-ct-core/src/mount.ts#L43

Thank you.

foxaltus commented 3 months ago

I dislike the direction of this. I really hoped Playwright would be the answer to the in-browser component testing. Now you are cutting off a huge chunk of developers by restricting the service worker API in Playwright. You are making decisions on behalf of your users based on assumptions. I believe that is not how software should be designed, but I trust you know better for your users.

💯 the Playwright team seems to underestimate the number of developers looking to move away from js-dom. We've been been longing for component testing in Playwright, but the lack of support for MSW is the last blocker.

I'm also not sure whether the Playwright teams appreciates that MSW is quickly becoming the norm for testing FE apps (including for Microsoft, it seems).

kettanaito commented 3 months ago

@foxaltus, Microsoft is one of the reasons why MSW is becoming the norm. I have no doubts that the team appreciates and loves MSW. They have their concerns about their software, I have some about mine. In the end, what matters is the best experience for the developer. I don't believe the current direction leads to that experience, and so I would very much love to see the conversation continue.