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

Fix Playwright 'failed to launch browsers' error #959

Merged
merged 2 commits into from Nov 19, 2021

Conversation

Iinh
Copy link
Contributor

@Iinh Iinh commented Nov 18, 2021

The Playwright CI test failed in my last PR: https://app.circleci.com/pipelines/github/mozilla/glean-dictionary/2768/workflows/727abea5-be8a-46af-b145-63f799b235de/jobs/5342

I updated the script to install Playwright per the recommendation in the error message.

Pull Request checklist

  • The pull request has a descriptive title (and a reference to an issue it
    fixes, if applicable)
  • All tests and linter checks are passing
  • The pull request is free of merge conflicts

package.json Outdated
@@ -16,7 +16,7 @@
"storybook": "start-storybook --static-dir ./public -p 6006",
"test": "npm-run-all format:check lint test:jest test:playwright",
"test:jest": "jest tests src --no-cache",
"test:playwright": "npx playwright test",
"test:playwright": "npx playwright install && npx playwright test",
Copy link
Contributor

@wlach wlach Nov 18, 2021

Choose a reason for hiding this comment

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

This doesn't seem quite correct to me, you shouldn't need to install playwritght to run the tests. It sounds like there's a mismatch between the playwright version we have installed and the docker image defined here.

It looks like playwright recommends that you keep the version of the docker container in sync with the version of the library you have installed:

microsoft/playwright#10410 (comment)

However, I honestly don't know how to keep the docker container definition in sync with what's in our package.json offhand, since dependabot doesn't send updates for the former.

I'd suggest we do two things here:

  1. Add the npx playwright install step to the circleci configuration above.
  2. Add a small note to the README.md about npx playwright install and npx playwright test (maybe just below our notes about storybook)

@Iinh Iinh requested a review from wlach November 19, 2021 13:37
Comment on lines +91 to +93
```bash
npx playwright install
```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

adding this step because it's mentioned as part of installation in Playwright docs: https://playwright.dev/docs/intro#installation

Copy link
Contributor

@wlach wlach left a comment

Choose a reason for hiding this comment

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

Perfect, thank you @Iinh!

@wlach wlach merged commit 46fa4f6 into main Nov 19, 2021
@wlach wlach deleted the update-playwright-script branch November 19, 2021 14:53
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.

None yet

2 participants