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
62.73k stars 3.38k forks source link

feature(trace-viewer): embedded mode support PoC #30885

Open ruifigueira opened 2 weeks ago

ruifigueira commented 2 weeks ago

Companion PR of https://github.com/microsoft/playwright-vscode/pull/483

github-actions[bot] commented 2 weeks ago

Test results for "tests 1"

18 failed :x: [chromium-library] › library/trace-viewer.spec.ts:240:1 › should have network requests :x: [chromium-library] › library/trace-viewer.spec.ts:689:1 › should highlight expect failure :x: [chromium-library] › library/trace-viewer.spec.ts:240:1 › should have network requests :x: [chromium-library] › library/trace-viewer.spec.ts:689:1 › should highlight expect failure :x: [chromium-library] › library/trace-viewer.spec.ts:240:1 › should have network requests :x: [chromium-library] › library/trace-viewer.spec.ts:689:1 › should highlight expect failure :x: [firefox-library] › library/trace-viewer.spec.ts:240:1 › should have network requests :x: [firefox-library] › library/trace-viewer.spec.ts:689:1 › should highlight expect failure :x: [playwright-test] › playwright.ct-react.spec.ts:253:5 › should pass "key" attribute from JSX in variable :x: [playwright-test] › ui-mode-test-output.spec.ts:78:5 › should show console messages for test :x: [chromium-library] › library/trace-viewer.spec.ts:240:1 › should have network requests :x: [chromium-library] › library/trace-viewer.spec.ts:689:1 › should highlight expect failure :x: [playwright-test] › ui-mode-test-output.spec.ts:78:5 › should show console messages for test :x: [playwright-test] › ui-mode-test-output.spec.ts:78:5 › should show console messages for test :x: [playwright-test] › ui-mode-test-output.spec.ts:78:5 › should show console messages for test :x: [webkit-library] › library/trace-viewer.spec.ts:240:1 › should have network requests :x: [webkit-library] › library/trace-viewer.spec.ts:689:1 › should highlight expect failure :x: [playwright-test] › ui-mode-test-output.spec.ts:78:5 › should show console messages for test

1 flaky :warning: [playwright-test] › ui-mode-test-screencast.spec.ts:21:5 › should show screenshots

26994 passed, 610 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] commented 2 weeks ago

Test results for "tests 1"

1 failed :x: [playwright-test] › playwright.ct-react.spec.ts:253:5 › should pass "key" attribute from JSX in variable

2 flaky :warning: [chromium-library] › library/trace-viewer.spec.ts:1027:1 › should ignore 304 responses
:warning: [playwright-test] › ui-mode-test-screencast.spec.ts:21:5 › should show screenshots

27010 passed, 610 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

pavelfeldman commented 1 week ago

I'm supportive for those, let me know when they are no longer drafts!

ruifigueira commented 1 week ago

@pavelfeldman I removed the draft, but this is still a very basic implementation just to check if the integration was possible.

I found out some issues with this approach: when the trace viewer iframe gets the focus keyboard shortcuts stop working, and I ned to click outside it to open the command palette for instance.

github-actions[bot] commented 1 week ago

Test results for "tests 1"

2 failed :x: [firefox-page] › page/page-request-continue.spec.ts:481:3 › continue should not change multipart/form-data body :x: [playwright-test] › playwright.ct-build.spec.ts:23:5 › should work with the empty component list

1 flaky :warning: [playwright-test] › ui-mode-test-screencast.spec.ts:21:5 › should show screenshots

27033 passed, 610 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

pavelfeldman commented 1 week ago

I found out some issues with this approach: when the trace viewer iframe gets the focus keyboard shortcuts stop working, and I ned to click outside it to open the command palette for instance.

That's unfortunate! I was rendering webviews in vscode w/o such issues. Is iframe w/ snapshot breaking it? How is service worker doing, are snapshots rendered fine?

ruifigueira commented 1 week ago

I tried to render the trace viewer html directly without iframe and service worker doesn't work there. Nevertheless, we can just rely on the running server instead of the service worker. The only thing I still need to figure out is on how to make the websocket work, or if not possible, on how to emulate it.

pavelfeldman commented 1 week ago

we can just rely on the running server instead of the service worker.

Unfortunately, we can't. It is essential for SW to fulfill all the requests.

ruifigueira commented 1 week ago

I am figuring that out the hard way :( I was doing some quick and dirty tests and deactivated the sw to rely on the webserver. I thought it was already handling trace requests as a fallback, but it was not, so I had to copy & paste the service worker logic there with necessary adjustments for it to work, but I stumbled on cross-domain snapshot resources that are handled by the sw and I don't see a way out of that...

I'll go back to the iframe solution and see if there's a workaround for it.

ruifigueira commented 1 week ago

Problem explained here, with a workaround proposal:

https://github.com/microsoft/vscode/issues/65452#issuecomment-586036474

ruifigueira commented 1 week ago

That did the trick, key bindings are triggered as expected when trace viewer is focused

github-actions[bot] commented 1 week ago

Test results for "tests 1"

1 failed :x: [playwright-test] › playwright.ct-react.spec.ts:253:5 › should pass "key" attribute from JSX in variable

2 flaky :warning: [firefox-page] › page/workers.spec.ts:106:3 › should clear upon navigation
:warning: [playwright-test] › ui-mode-test-watch.spec.ts:96:5 › should batch watch updates

26363 passed, 609 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] commented 1 week ago

Test results for "tests 1"

1 failed :x: [playwright-test] › playwright.ct-react.spec.ts:253:5 › should pass "key" attribute from JSX in variable

27056 passed, 609 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

ruifigueira commented 1 week ago

The snapshot popout link is not working (window.open doesn't work in vscode). Using vscode.env.openExternalUrl does not work either because it won't share the sw.

We can either disable it (it will still work on non-embedded mode) or delegate it to the server (for instance, it can launch a playwright-controlled browser and inject the trace file for sw to load it before opening the snapshot.html URL).

ruifigueira commented 5 days ago

I found out why I thought I needed to inject the trace file before opening snapshot.html. It's a bug: https://github.com/microsoft/playwright/pull/31033

github-actions[bot] commented 4 days ago

Test results for "tests 1"

10 fatal errors, not part of any test 13776 passed, 284 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.

github-actions[bot] commented 4 days ago

Test results for "tests 1"

2 failed :x: [playwright-test] › runner.spec.ts:118:5 › should ignore subprocess creation error because of SIGINT :x: [installation tests] › playwright-electron-should-work.spec.ts:30:5 › electron should work with special characters in path

27067 passed, 609 skipped :heavy_check_mark::heavy_check_mark::heavy_check_mark:

Merge workflow run.