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(build): playwright cannot find browser executables in a new env #3721

Closed

Conversation

flyingImer
Copy link
Collaborator

For a new development environment, pnpm build fails due to missing browser deps for playwright tests with error messages like:

Error: browserType.launch: Executable doesn't exist at /<...>/.cache/ms-playwright/<...>

It is required to install manually with pnpm exec playwright install within apps/wing-console/console/app, which breaks the pnpm build flow.

This fix is a try to ensure the browser deps are always installed and being the latest before running tests. It works fine for local development. Although the installation "runs" before every test run, turbo does a pretty good job to cut corners:

@wingconsole/app:pre-test: cache hit (outputs already on disk), replaying logs 00d0d93700261d4e

Just not sure about if this is okay for the CI environment, please suggest

Checklist

  • Title matches Winglang's style guide
  • Description explains motivation and solution
  • Tests added (always)
  • Docs updated (only required for features)
  • Added pr/e2e-full label if this feature requires end-to-end testing

By submitting this pull request, I confirm that my contribution is made under the terms of the Wing Cloud Contribution License.

Signed-off-by: monada-bot[bot] <[email protected]>
@monadabot monadabot added the ⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good! label Aug 7, 2023
@monadabot monadabot requested a review from a team as a code owner August 7, 2023 04:32
@skyrpex
Copy link
Contributor

skyrpex commented Aug 7, 2023

Hey @flyingImer thanks for the PR. This is something I noticed from other people as well, and wanted to fix the issue.

@polamoros and I checked out the solution you worked on and has a problem: in Ubuntu (and maybe other OS) the playwright install will ask for sudo permissions. Additionally, playwright install --with-deps may be necessary, depending on your system.

For now, I think the best solution is to rename the Console/App test to test:playwright and run it only in the root script test:ci. This way, it won't bother people running build or test, but still we be run in the CI.

@skyrpex skyrpex closed this Aug 7, 2023
mergify bot pushed a commit that referenced this pull request Aug 7, 2023
Renames the `@wingconsole/app#test` script to `@wingconsole/app#test:playwright`, so it doesn't run with the root scripts `build` or `test`. The problem is that in order to run Playwright tests, users need to run `playwright install` or `playwright install --with-deps` first, and may require sudo permissions.

Additionally, moves the playwright tests from `tests/` to `test/`, which is the standard directory for both playwright and our turbo configuration.

Supersedes #3721.
@flyingImer flyingImer deleted the try-fix-console-playwright branch August 7, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ pr/review-mutation PR has been mutated and will not auto-merge. Clear this label if the changes look good!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants