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

chore: upgrade playwright and storybook #192

Merged
merged 11 commits into from
Jun 7, 2024
Merged

Conversation

pmelab
Copy link
Contributor

@pmelab pmelab commented Apr 17, 2024

Description of changes

  • Upgrade Playwright and Storybook to the latest versions.
  • Remove the dependency on e2e for installing playwright.

Motivation and context

Resolve issues where e2e tests could not find playwright browser binaries.

How has this been tested?

  • Manually
  • Unit tests
  • Integration tests

@pmelab pmelab force-pushed the SLB-219-playwright-update branch 2 times, most recently from 8d076a6 to 2f58302 Compare April 18, 2024 06:06
@pmelab pmelab changed the title chore: upgrade playwright chore: upgrade playwright and storybook Apr 18, 2024
@pmelab pmelab force-pushed the SLB-219-playwright-update branch 5 times, most recently from bedf578 to b4f8387 Compare April 18, 2024 12:47
@pmelab pmelab requested a review from Leksat April 18, 2024 15:15
Copy link
Member

@Leksat Leksat left a comment

Choose a reason for hiding this comment

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

Looks great! Just one minor question.

@@ -7,6 +7,7 @@
"license": "ISC",
"type": "module",
"scripts": {
"prep": "playwright install chromium",
Copy link
Member

Choose a reason for hiding this comment

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

Hm. Previously there was a Playwright postinstall script which was downloading binaries during pnpm i. Looks like now it's gone.

Should we get rid of

ENV PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1
in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, is that why playwright is not downloading browsers automatically any more?

Copy link
Member

Choose a reason for hiding this comment

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

Well, it's a long story. Previously, when it was called playwright (not @playwright/test) we had this issue with automatic browsers download in all environments. I went with PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1 and controlled downloads. Then when I faced the missing binaries error on this repo, I thought it's the same issue and added the env var. I did not know that the postinstall hook is already gone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thank you for the clarification. Will clean this up then.

Copy link
Member

Choose a reason for hiding this comment

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

Aha! Now it's clear.

Playwright v1.38.0

Breaking Changes: Playwright no longer downloads browsers automatically

Details: https://github.com/microsoft/playwright/releases/tag/v1.38.0

@pmelab pmelab force-pushed the SLB-219-playwright-update branch from 8808868 to fdf02e6 Compare May 3, 2024 10:04
@pmelab pmelab force-pushed the SLB-219-playwright-update branch from fdf02e6 to 93e9acc Compare June 7, 2024 14:32
@pmelab pmelab merged commit 2b815f7 into release Jun 7, 2024
6 checks passed
@pmelab pmelab deleted the SLB-219-playwright-update branch June 7, 2024 14:54
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