fix(ci): restore --with-deps on Playwright install#267
Conversation
PR boundlessfi#265 dropped --with-deps from the Playwright install step. Without it, fresh CI runners that don't have Chromium's system libs (fonts, shared libraries) cached will fail to launch the browser. Putting it back.
|
@Benjtalkshow is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR updates the CI workflow to install Playwright Chromium with system dependencies enabled in the E2E test job. The ChangesE2E Playwright dependencies
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
96-97: Consider caching Playwright browsers to speed up CI runs.Currently, Playwright browsers are downloaded and system dependencies installed on every CI run. Adding a cache step for Playwright browsers could reduce CI execution time and bandwidth usage.
💡 Suggested cache configuration
Add this step before "Install Playwright Chromium":
- name: Cache Playwright browsers uses: actions/cache@v4 with: path: ~/.cache/ms-playwright key: ${{ runner.os }}-playwright-${{ hashFiles('**/pnpm-lock.yaml') }} restore-keys: | ${{ runner.os }}-playwright-Note: With caching, Playwright will skip downloading browsers if they're already cached, but
--with-depswill still ensure system dependencies are installed when needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 96 - 97, Add a cache step before the "Install Playwright Chromium" step to store Playwright browsers under the ~/.cache/ms-playwright path using actions/cache@v4; configure the cache key to include runner.os and a hash of pnpm-lock.yaml (e.g., ${runner.os}-playwright-${hashFiles('**/pnpm-lock.yaml')}) and use a restore-keys prefix like ${runner.os}-playwright- so cached browsers are restored when available; keep the existing "Install Playwright Chromium" step (which runs pnpm exec playwright install chromium --with-deps) so system dependencies are still ensured even when browsers are cached.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 96-97: Add a cache step before the "Install Playwright Chromium"
step to store Playwright browsers under the ~/.cache/ms-playwright path using
actions/cache@v4; configure the cache key to include runner.os and a hash of
pnpm-lock.yaml (e.g., ${runner.os}-playwright-${hashFiles('**/pnpm-lock.yaml')})
and use a restore-keys prefix like ${runner.os}-playwright- so cached browsers
are restored when available; keep the existing "Install Playwright Chromium"
step (which runs pnpm exec playwright install chromium --with-deps) so system
dependencies are still ensured even when browsers are cached.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea42beba-d304-4ead-a483-0d3af86fc33e
📒 Files selected for processing (1)
.github/workflows/ci.yml
PR #265 dropped
--with-depsfrom the Playwright install step in.github/workflows/ci.yml. Without it, fresh CI runners without Chromium's system libs (fonts, shared libraries) cached will fail to launch the browser.Restoring
--with-depsso e2e stays reliable across cold runners.Summary by CodeRabbit