🔧 Dissociate pinned e2e setup#4811
Conversation
Bundles Sizes Evolution
|
b9a543b to
15d3506
Compare
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 15d3506 | Docs | Datadog PR Page | Give us feedback! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15d3506dec
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "test:unit:bs": "node --env-file-if-exists=.env ./scripts/test/bs-wrapper.ts karma start test/unit/karma.bs.conf.js", | ||
| "test:e2e:setup": "yarn build && yarn build:apps && yarn playwright install --with-deps && yarn dlx -p playwright@1.40.1 playwright install chromium firefox webkit", | ||
| "test:e2e:setup": "yarn build && yarn build:apps && yarn playwright install --with-deps", | ||
| "test:e2e:setup:pinned": "volta run --node 20 yarn dlx -p playwright@1.40.1 playwright install chromium firefox webkit", |
There was a problem hiding this comment.
Avoid requiring Volta for the pinned setup
issue: In a checkout where Yarn is available but volta is not on PATH, this newly documented setup path exits before installing the pinned browsers (yarn test:e2e:setup:pinned currently fails with command not found: volta). Since the repo setup instructions only require Yarn and this script is now the way to prepare the firefox-pinned/webkit-pinned projects locally, contributors who do not use Volta cannot run those projects; wrap the Node 20 workaround in a repo-managed script or document/enforce Volta before relying on this command.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
They will figure it out!
Motivation
The default
test:e2e:setupscript mixed two concerns: building the repo and installing Playwright browsers, and additionally forcing a pinned Playwright version viayarn dlxfor Chromium, Firefox, and WebKit. That made the common setup path heavier and less obvious when a pinned install is actually needed.Changes
test:e2e:setupnow runsyarn build,yarn build:apps, andyarn playwright install --with-depsonly (aligned with the workspace Playwright dependency).test:e2e:setup:pinnedis a separate script that preserves the previous pinned Playwright install (playwright@1.40.1viavolta run --node 20) for workflows that still require that exact pin.Test instructions
yarn test:e2e:setupand confirm it completes without invoking the pinneddlxPlaywright install.yarn test:e2e:setup:pinnedafter the main setup (or as needed) and confirm browsers install as before.Checklist
Made with Cursor