chore: migrate examples from Cypress to Playwright#841
chore: migrate examples from Cypress to Playwright#84150rayn wants to merge 6 commits intohistoire-dev:mainfrom
Conversation
|
|
✅ Deploy Preview for histoire-examples-svelte3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-controls ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-examples-vue3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for histoire-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
histoire
@histoire/app
@histoire/controls
@histoire/plugin-nuxt
@histoire/plugin-percy
@histoire/plugin-screenshot
@histoire/plugin-svelte
@histoire/plugin-vue
@histoire/shared
@histoire/vendors
commit: |
There was a problem hiding this comment.
Code Review
This pull request migrates the test suite from Cypress to Playwright across the monorepo, updating dependencies and configuration files accordingly. The review identified potential issues with the strictness of Playwright's toHaveText assertions compared to the original Cypress tests, suggesting the use of textContent or useInnerText: true for exact matches. Additionally, a suggestion was made to improve the robustness of Playwright version parsing in the GitHub Action.
| for (const { variant, expected } of cases) { | ||
| test(`renders source code for the ${variant} variant`, async ({ page }) => { | ||
| await page.goto(`/story/src-components-codegen-story-vue?variantId=${variant}`) | ||
| await expect(page.getByTestId('story-source-code')).toHaveText(expected) |
There was a problem hiding this comment.
The toHaveText() assertion performs a substring match and normalizes whitespace. The original Cypress test used should('have.text', ...) which performs an exact match on the element's textContent. This change weakens the test, as it would pass even if there were extra content or different whitespace. To maintain the same level of strictness, it's better to assert against the textContent directly.
| await expect(page.getByTestId('story-source-code')).toHaveText(expected) | |
| expect(await page.getByTestId('story-source-code').textContent()).toEqual(expected) |
|
|
||
| const iframe = page.frameLocator('iframe[data-test-id="preview-iframe"]') | ||
| await expect(iframe.getByText('Hello world!')).toBeVisible() | ||
| await expect(page.getByTestId('story-source-code')).toHaveText('<Demo message="Hello world!" />') |
There was a problem hiding this comment.
The toHaveText() assertion performs a substring match, which is less strict than the original Cypress test's exact match. This could allow regressions to go unnoticed. For an exact match of the content, please assert against the element's textContent.
| await expect(page.getByTestId('story-source-code')).toHaveText('<Demo message="Hello world!" />') | |
| await expect(page.getByTestId('story-source-code')).toHaveText('<Demo message="Hello world!" />', { useInnerText: true }) |
|
|
||
| const iframe = page.frameLocator('iframe[data-test-id="preview-iframe"]') | ||
| await expect(iframe.getByText('Meow!')).toBeVisible() | ||
| await expect(page.getByTestId('story-source-code')).toHaveText('<Demo message="Meow!" />') |
There was a problem hiding this comment.
The toHaveText() assertion performs a substring match, which is less strict than the original Cypress test's exact match. This could allow regressions to go unnoticed. For an exact match of the content, please assert against the element's textContent.
| await expect(page.getByTestId('story-source-code')).toHaveText('<Demo message="Meow!" />') | |
| await expect(page.getByTestId('story-source-code')).toHaveText('<Demo message="Meow!" />', { useInnerText: true }) |
Why I started this
Cypress treats Webkit as second-class (we just merged a Safari-only shortcut fix in #839),
start-server-and-testis a babysitter, and every iframe interaction needs agetIframeBody = () => cy.get('iframe').its('contentDocument.body').should('not.be.empty').then(cy.wrap)dance. Playwright handles iframes natively (page.frameLocator(...)) and starts the preview server itself.What changed
examples/{svelte4,vue3,nuxt4}translated to Playwright Teststart-server-and-testshim andcypress-parallel.mjsdeleted.github/actions/test-examplecomposite action — three workflow files now ~10 lines eachnr testruns Playwright;nr test:devisplaywright test --uiBenchmarks (measured locally on M3, no CI)
fixmeupstream)cypress-parallel.mjs5-way matrix shim to be tractableWhat's marked
test.fixmeFive svelte4 controls/state-sync tests are
fixmebecauseHst.Checkbox/Hst.Selectfromhistoire-plugin-svelterender as the placeholder text"HstCheckbox"instead of mounting the underlying Vue control under Svelte 5 compat. Cypress fails identically on the same build — the migration is faithful, the bug is inpackages/histoire-plugin-svelte/src/client/Wrap.svelte. I'll file an issue for that separately.Heads-up
playwright.config.tsyet (chromium only) — easy to add later.playwright.config.tshas 23 lines duplicated × 3 examples. I tried hoisting intoexamples/playwright.shared.tsbut pnpm's workspace + Playwright's TS resolver couldn't agree. Left inline for now.pnpm run buildfor every example workflow. Splitting that into a build job with artifact-upload + needs is real CI savings — separate PR.Closes none. Cypress was never the user-facing stack — this only touches the example apps.