chore: update playwright to 1.57.0 and fix traces #3181
Conversation
| ...(testFilter ? ["--grep", testFilter] : []), | ||
| ...(testExcludeFilter ? ["--grep-invert", testExcludeFilter] : []), |
There was a problem hiding this comment.
No more -gv:
> npx playwright test --help
...
-g, --grep <grep> Only run tests matching this regular expression (default: ".*")
--global-timeout <timeout> Maximum time this test suite can run in milliseconds (default: unlimited)
--grep-invert <grep> Only run tests that do not match this regular expression
...
shouples
changed the title
tests/e2e/playwright.config.ts
Outdated
| export default defineConfig({ | ||
| testDir: path.normalize(path.join(__dirname, "specs")), | ||
| forbidOnly: !!process.env.CI, | ||
| // maxFailures: 1, // uncomment for local dev/debugging purposes |
There was a problem hiding this comment.
Not strictly necessary for this PR, but helpful to have as a reference
| if (!electronApp) { | ||
| throw new Error("electronApp is null - failed to launch VS Code"); | ||
| } | ||
|
|
||
| const page = await electronApp.firstWindow(); | ||
| if (!page) { | ||
| // shouldn't happen since we waited for the workbench above | ||
| throw new Error("Failed to get first window from VS Code"); | ||
| } |
There was a problem hiding this comment.
No need for these since electronApp will holler loudly if it can't start or we can't get the first window
| * With Playwright v1.50.0+, the behavior of `electronApp.close()` changes so it waits for the | ||
| * Electron process to exit, but if there are any lingering handles or requests, that may never | ||
| * happen, causing the test worker to hang and eventually time out. |
There was a problem hiding this comment.
Wait until the process has exited before returning from ElectronApplication.close() (like for a normal browser - Browser.close() -> gracefullyClose).
| test( | ||
| "should complete the browser-based Confluent Cloud sign-in flow", | ||
| { tag: [Tag.Smoke, Tag.CCloud] }, | ||
| async ({ page, electronApp, openExtensionSidebar }) => { |
There was a problem hiding this comment.
openExtensionSidebar is an auto-used fixture, so we don't need to include it here
There was a problem hiding this comment.
Pull request overview
This PR updates Playwright from version 1.45.0 to 1.57.0 and addresses multiple issues related to test tracing, particularly fixing CSS/font file capture in trace snapshots. The update required adapting to breaking changes in Playwright v1.46.0 (plugin structure) and v1.50.0 (Electron app shutdown behavior).
Key Changes:
- Updated Playwright dependency to 1.57.0 with breaking change adaptations
- Implemented manual trace management to properly capture Electron app traces
- Added graceful shutdown handling with forced cleanup to prevent worker timeouts
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Bumped @playwright/test from 1.45.0 to 1.57.0 |
| tests/e2e/playwright.config.ts | Disabled automatic tracing in favor of manual implementation |
| tests/e2e/baseTest.ts | Refactored trace management to manual control and added shutdownElectronApp() function |
| tests/e2e/utils/connections.ts | Added TestInfo parameter and screenshot capture for CCloud auth failures |
| tests/e2e/specs/confluent.spec.ts | Updated test to pass TestInfo to setupCCloudConnection |
| src/webview/*.spec.ts | Wrapped plugins array in double-array structure per Playwright 1.46+ requirement |
| Gulpfile.js | Updated test filter flags from shorthand to full names |
…auth flow happens
Summary of Changes
Follow-up from #3076 to bump our Playwright version from 1.45.0 to 1.57.0. Closes https://github.com/confluentinc/vscode/security/dependabot/35
This required a couple of nontrivial changes based on Playwright release versions:
electronApp.close()to prevent worker teardown timeout (more context below)Click-testing instructions
expect(1).toBe(2);anywhere in the existing testsnpx gulp e2ewith optional-tfor specific test(s)Additional Context
playwright.config.tsas well asbaseTest.ts, where we only need one.We can't only use the playwright.config.ts setup since it doesn't Just Work ™️ with Electron, so we get even more useless traces:
We also can't just use the
tracefixture while starting our own manual tracing without having a clean way of stopping the trace and ensuring it's added to the test results, so we're better off manually starting and stopping the trace within theelectronAppfixture. (While also making sure we don't start tracing too late and drop/miss elements.)Note
Due to the fact that we aren't using the
tracefixture anymore and are manually implementing our own traces, we also lose the screenshots for the CCloud-auth/Chromium context in that top "timeline"-like section of the trace viewer. This is because it was combining multiple trace contexts into one, and trying to manually reimplement that isout of scope.
Instead, we'll capture a screenshot for any failure that happens during the CCloud sign-in browser flow, which will show up on the individual test result pages:
electronApp.close()electronApp.close()would wait for other Node processes/handles/etc to close down.Pull request checklist
Please check if your PR fulfills the following (if applicable):
Tests
Release notes