DEVPROD-24935: Migrate from Cypress to Playwright for Parsley#1483
DEVPROD-24935: Migrate from Cypress to Playwright for Parsley#1483minnakt merged 28 commits intoevergreen-ci:mainfrom
Conversation
sophstad
left a comment
There was a problem hiding this comment.
yay 😍 maybe when Spruce is also done we can migrate away from data-cy and the dataCy utils
| Tasks.CheckCodegen, | ||
| Tasks.Compile, | ||
| Tasks.E2E, | ||
| Tasks.E2EPlaywright, |
There was a problem hiding this comment.
[nit] Maybe we can just name this E2E and the other to e2e_cypress?
| use: { | ||
| baseURL: "http://localhost:5173", | ||
| viewport: { width: 1280, height: 800 }, | ||
| video: process.env.CI ? "retain-on-failure" : "off", |
There was a problem hiding this comment.
Maybe we should use on-first-retry instead of retain-on-failure, it seems possible that recording video by default would be slightly slower
| forbidOnly: !!process.env.CI, // Fail if implementer accidentally left test.only in any file. | ||
| retries: process.env.CI ? 2 : 0, | ||
| workers: 1, // Disable parallelism - run tests serially like Cypress. | ||
| use: { |
There was a problem hiding this comment.
Let's try enabling traces, this looks freaking sick
There was a problem hiding this comment.
So I think it is possible to get traces to be served from the HTML report, but the amount of things I have to upload would be kind of a lot. (I wish s3.put would just infer content_type?)
Instead I attached a .zip file which contains the traces and can be served locally via pnpm exec playwright show-report playwright-html-report
| "storybook": "NO_HTTPS=true storybook dev -p 6007", | ||
| "storybook:build": "env-cmd -e local storybook build", | ||
| "test": "vitest --typecheck=false", | ||
| "test": "vitest --typecheck=false --exclude 'playwright/**'", |
There was a problem hiding this comment.
Would it maybe be preferable to scope Vitest using include to only look at the src directory? Either way let's move the option to Vitest config instead of the script
| for (let i = 0; i < conditions.length; i++) { | ||
| if (!conditions[i]) { | ||
| hasOutOfBoundsValue = true; | ||
| console.log(`Out of bounds value: ${i} ${conditions[i]}`); |
There was a problem hiding this comment.
Not sure if we really need this log, looks slightly awk in the logs (or could somehow log on failure only? idk)
There was a problem hiding this comment.
Sorry yeah this is slop
There was a problem hiding this comment.
ha you're okay I think it was copypasta from a cy.log()
| await page.locator(`[aria-label="Collapse navigation"]`).click(); | ||
| }; | ||
|
|
||
| export const validateToast = async ( |
There was a problem hiding this comment.
Would be nice if we can somehow share these utils when Spruce is added
| @@ -0,0 +1 @@ | |||
| export * from "./commands"; | |||
There was a problem hiding this comment.
I'd be okay having all the helper functions here, seems like this is maybe Cypress cruft
| }; | ||
| const toastDataCy = "toast"; | ||
|
|
||
| export const getByDataCy = (page: Page, value: string) => |
There was a problem hiding this comment.
Hm do you think we should use a custom test attribute instead of requiring this helper everywhere? Or maybe just followup work?
| - command: s3.put | ||
| type: system | ||
| params: | ||
| role_arn: ${ASSUME_ROLE_ARN} | ||
| local_files_include_filter: ["ui/${app_dir}/bin/playwright/html/data/**/*.png"] | ||
| remote_file: ${build_variant}/${task_id}/${execution}/playwright/html/data/ | ||
| bucket: evg-bucket-evergreen-ui | ||
| content_type: image/png | ||
| permissions: public-read | ||
| - command: s3.put | ||
| type: system | ||
| params: | ||
| role_arn: ${ASSUME_ROLE_ARN} | ||
| local_files_include_filter: ["ui/${app_dir}/bin/playwright/html/data/**/*.webm"] | ||
| remote_file: ${build_variant}/${task_id}/${execution}/playwright/html/data/ | ||
| bucket: evg-bucket-evergreen-ui | ||
| content_type: video/webm | ||
| permissions: public-read | ||
| - command: s3.put | ||
| type: system | ||
| params: | ||
| role_arn: ${ASSUME_ROLE_ARN} | ||
| local_files_include_filter: ["ui/${app_dir}/bin/playwright/html/data/**/*.md"] | ||
| remote_file: ${build_variant}/${task_id}/${execution}/playwright/html/data/ | ||
| bucket: evg-bucket-evergreen-ui | ||
| content_type: text/markdown | ||
| permissions: public-read |
There was a problem hiding this comment.
I think maybe we should be grabbing these assets from bin/playwright/test-results instead of /html. That makes them not have hashed asset names, and allows us to directly download the trace.zip files which is ideal. We could also use view remote traces except for a CORS issue I haven't figured out yet 😿
I tried it on this patch (just look at changes in .evergreen/attach.yml)
| permissions: private | ||
| visibility: signed | ||
| display_name: "Playwright HTML Report (zip)" | ||
| - command: s3.put |
There was a problem hiding this comment.
Similar changes in the patch I linked -- it is slightly easier imo to just upload the whole /html directory and use preserive_path... and I added a special display name upload so the index file is clear. What do you think?
| // Intercept GraphQL queries to detect mutations; match both relative and absolute paths. | ||
| await page.route("**/graphql/query", async (route) => { | ||
| const request = route.request(); | ||
| try { | ||
| const postData = request.postDataJSON(); | ||
| if (postData?.query?.startsWith("mutation")) { | ||
| mutationDispatched = true; | ||
| } | ||
| } catch (e) { | ||
| console.error("Failed to parse GraphQL request:", e); | ||
| } | ||
| await route.continue(); | ||
| }); |
There was a problem hiding this comment.
Nice (but I want to rip it out 😈)
| body.txt-e | ||
|
|
||
| # Playwright | ||
| /test-results/ |
There was a problem hiding this comment.
I believe this is being written to /bin so can be removed (playwright-report and blob-report may be the same)
| "postversion": "pnpm deploy-utils-postversion", | ||
| "prepare": "pnpm --dir ../.. prepare", | ||
| "preview": "serve -s dist -p 4173", | ||
| "preview": "serve -s dist -p 5173", |
There was a problem hiding this comment.
I think this should also be updated in env-cmdrc.json
sophstad
left a comment
There was a problem hiding this comment.
This is looking pretty good! Main question is whether there's any way to include the test name instead of just the filename for failed assets 🤔 if not that's okay too
|
added test names to uploaded files |
| remote_file: ${build_variant}/${task_id}/${execution}/report/ | ||
| bucket: evg-bucket-evergreen-ui | ||
| content_type: text/html | ||
| visibility: none |
| bucket: evg-bucket-evergreen-ui | ||
| content_type: image/png | ||
| permissions: public-read | ||
| display_name: "screenshot:" |
There was a problem hiding this comment.
[super nit] The names here seem prefixed with the type so could remove if you want, up to you
| display_name: "screenshot:" |
| bucket: evg-bucket-evergreen-ui | ||
| content_type: video/webm | ||
| permissions: public-read | ||
| display_name: "video:" |
There was a problem hiding this comment.
Here too (giving you github suggestions to make it easy to apply)
| display_name: "video:" |
| bucket: evg-bucket-evergreen-ui | ||
| content_type: application/zip | ||
| permissions: public-read | ||
| display_name: "trace:" |
There was a problem hiding this comment.
| display_name: "trace:" |
DEVPROD-24935
Description
Migrates tests from Cypress to Playwright. Sorry for how big this PR is 🧍♂️
Instructions:
pnpm playwright:ui, which will open the Playwright UICypress configuration still exists since Spruce still has Cypress tests.
Screenshots