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

feat: server side mocking #34403

Closed
wants to merge 53 commits into from
Closed

feat: server side mocking #34403

wants to merge 53 commits into from

Conversation

Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented Jan 20, 2025

Implements #30766.

Adds a MockingProxy class with an API like page.route that allows intercepting network traffic from the web server.

@Skn0tt Skn0tt requested review from dgozman and Copilot January 20, 2025 15:29
@Skn0tt Skn0tt self-assigned this Jan 20, 2025
@Skn0tt Skn0tt changed the title feat: MockingProxy feat: server side mocking Jan 20, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 13 out of 28 changed files in this pull request and generated 1 comment.

Files not reviewed (15)
  • packages/playwright-core/src/protocol/debug.ts: Evaluated as low risk
  • packages/playwright/src/util.ts: Evaluated as low risk
  • packages/playwright-core/src/utils/httpServer.ts: Evaluated as low risk
  • docs/src/test-api/class-fixtures.md: Evaluated as low risk
  • docs/src/test-api/class-testoptions.md: Evaluated as low risk
  • packages/playwright-core/src/server/network.ts: Evaluated as low risk
  • packages/playwright-core/src/server/dispatchers/mockingProxy.ts: Evaluated as low risk
  • packages/playwright-core/src/client/browserContext.ts: Evaluated as low risk
  • packages/playwright-core/src/client/network.ts: Evaluated as low risk
  • packages/playwright-core/src/client/page.ts: Evaluated as low risk
  • packages/playwright-core/src/client/playwright.ts: Evaluated as low risk
  • packages/playwright-core/src/server/browserContext.ts: Evaluated as low risk
  • packages/playwright-core/src/client/localUtils.ts: Evaluated as low risk
  • docs/src/api/class-playwright.md: Evaluated as low risk
  • packages/playwright-core/src/server/dispatchers/localUtilsDispatcher.ts: Evaluated as low risk
Comments suppressed due to low confidence (4)

docs/src/api/class-mockingproxyfactory.md:5

  • [nitpick] Rephrase the sentence to: "You can obtain an instance of this class via [property: Playwright.mockingProxy]." for better clarity.
An instance of this class can be obtained via [`property: Playwright.mockingProxy`].

packages/playwright-core/src/client/mockingProxy.ts:148

  • The comment 'silence it' should be 'silence it.' with a period at the end.
await route._innerContinue(true /* isFallback */).catch(() => { });

packages/playwright/src/index.ts:125

  • [nitpick] The variable name _mockingProxy is prefixed with an underscore, which is typically used for private variables. Consider renaming it to mockingProxy.
_mockingProxy: [async ({ mockingProxy: mockingProxyOption, playwright }, use) => {

packages/playwright/src/index.ts:473

  • [nitpick] The error message could be more descriptive. Consider changing it to The 'server' fixture requires 'mockingProxy' to be enabled. Please enable 'mockingProxy' in the configuration.
throw new Error(`The 'server' fixture is only available when 'mockingProxy' is enabled.`);

docs/src/api/class-mockingproxyfactory.md Outdated Show resolved Hide resolved
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Simon Knott <[email protected]>

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@vitalets
Copy link
Contributor

This is awesome direction, that opens new opportunities for server components!
Just a quick question until it's merged - don't you want to release mocking proxy as a separate package? To be able to start mocking proxy instance somewhere on the web instead of localhost.
Otherwise, how is it expected to mock requests for the app on a public url, that can't access localhost?

@Skn0tt
Copy link
Member Author

Skn0tt commented Jan 21, 2025

To be able to start mocking proxy instance somewhere on the web instead of localhost. [...] mock requests for the app on a public url [...]

That's an interesting idea. Could you elaborate on the setup you imagine for this? Would you deploy the proxy alongside your staging environment?

@vitalets
Copy link
Contributor

That's an interesting idea. Could you elaborate on the setup you imagine for this? Would you deploy the proxy alongside your staging environment?

We run our tests on GitHub actions over dev deployment, that is automatically built on vercel. So we pass remote baseURL:

import { defineConfig } from '@playwright/test';

export default defineConfig({
  use: {
    baseURL: process.env.VERCEL_URL, // something like 'http://build-xxxxx.vercel.app',
  },
});

We would be ready to deploy Playwright mocking proxy alongside the main app, to be able to mock server-side requests.
Especially if it would be as easy as npx playwright run-proxy.
But I'm curious is it even possible to sync Playwright test context with external proxy context..

Some other options I'm aware of:

  • setup tunneling (like ngrok) to make runner context accessible externally.
  • embed proxy inside the main app: this was chosen by msw with useRemoteServer(), here is the detailed discussion and limitations.

@Skn0tt
Copy link
Member Author

Skn0tt commented Jan 21, 2025

But I'm curious is it even possible to sync Playwright test context with external proxy context.

We'd need to build that, but it's certainly possible. I'm unsure if it's needed though, because tunneling with ngrok would solve the problem as well. You'd probably write a worker-scoped fixture that uses the @ngrok/ngrok npm package to open a tunnel to the proxy, and then you'd attach the public ngrok URL to all outgoing requests. It'd be ad-hoc, and provided that you have access to smth like ngrok,it means fewer infrastructure changes because you don't need to deploy the Playwright proxy.

embed proxy inside the main app: this was chosen by msw with useRemoteServer()

I'm aware of this ongoing work in MSW, but I think it's subject to the same networking issues. If the MSW "remote server" runs on a different network than the test runner, there needs to be some sort of network link back to the test runner so they can communicate, just like with my approach.

My gut feeling is that deploying the proxy alongside the application would make the feature more complicated to understand, and that tunneling with ngrok is a good solution. Curious what you think.

@vitalets
Copy link
Contributor

Ngrok tunneling is the simplest setup, I agree. I see only two downsides:

  • my company policies may not allow to transfer even test traffic via third-party service
  • we rely on ngrok infrastructure, if there are problems, we may get flaky/failing tests

I'd personally prefer to have a dedicated mocking app, that I fully control. I my project we already have similar setup for Sanity CMS, we automatically deploy Sanity Studio alongside the main app.

In general, I think having kind of Playwright server / Playwright global worker is an interesting option. Besides mocking, it can conditionally perform global setup or other operations, that should be done once across the whole test run (even across shards).

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

Test results for "tests 1"

5 failed
❌ [playwright-test] › tests/playwright.mockingproxy.spec.ts💯5 › all properties are populated @macos-latest-node18-1
❌ [playwright-test] › tests/playwright.mockingproxy.spec.ts💯5 › all properties are populated @ubuntu-latest-node18-1
❌ [playwright-test] › tests/playwright.mockingproxy.spec.ts💯5 › all properties are populated @ubuntu-latest-node20-1
❌ [playwright-test] › tests/playwright.mockingproxy.spec.ts💯5 › all properties are populated @ubuntu-latest-node22-1
❌ [playwright-test] › tests/playwright.mockingproxy.spec.ts💯5 › all properties are populated @windows-latest-node18-1

19 flaky ⚠️ [firefox-page] › tests/page/page-evaluate.spec.ts:403:3 › should throw for too deep reference chain @firefox-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/browsercontext-pages.spec.ts:82:3 › should click the button with offset with page scale @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/browsercontext-viewport-mobile.spec.ts:65:5 › mobile viewport › should detect touch when applying viewport with touches @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/browsercontext-viewport-mobile.spec.ts:124:5 › mobile viewport › respect meta viewport tag @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/browsercontext-viewport-mobile.spec.ts:157:5 › mobile viewport › mouse should work with mobile viewports and cross process navigations @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/defaultbrowsercontext-2.spec.ts:28:3 › should work in persistent context @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/inspector/cli-codegen-1.spec.ts:96:7 › cli codegen › should ignore programmatic events @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/page-event-crash.spec.ts:67:1 › should cancel navigation when page crashes @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/proxy.spec.ts:93:11 › should proxy local network requests › by default › link-local @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:44:14 › page screenshot › should work with a mobile viewport @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:55:14 › page screenshot › should work with a mobile viewport and clip @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:66:14 › page screenshot › should work with a mobile viewport and fullPage @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/screenshot.spec.ts:278:14 › element screenshot › should restore viewport after page screenshot and exception @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/selector-generator.spec.ts:332:7 › selector generator › should prioritize attributes correctly › type @webkit-ubuntu-22.04-node18
⚠️ [webkit-library] › tests/library/video.spec.ts:441:5 › screencast › should work for popups @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-leaks.spec.ts:82:5 › click should not leak @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-leaks.spec.ts:107:5 › fill should not leak @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-leaks.spec.ts:161:5 › waitFor should not leak @webkit-ubuntu-22.04-node18
⚠️ [webkit-page] › tests/page/page-screenshot.spec.ts:868:5 › page screenshot animations › should wait for fonts to load @webkit-ubuntu-22.04-node18

37736 passed, 649 skipped
✔️✔️✔️

Merge workflow run.

@Skn0tt
Copy link
Member Author

Skn0tt commented Jan 28, 2025

I trimmed down the feature a little more to make it easier to iterate on. Closing in favour of #34520

@Skn0tt Skn0tt closed this Jan 28, 2025
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 this pull request may close these issues.

2 participants