-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix(ci): enhance Playwright installation steps with system dependencies and cache checks #654
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -200,8 +200,18 @@ jobs: | |||||||||||||||||||||||||
| - name: Install dependencies | ||||||||||||||||||||||||||
| run: npm ci | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - name: Install Playwright system dependencies | ||||||||||||||||||||||||||
| run: npx playwright install-deps chromium | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - name: Install Playwright Chromium | ||||||||||||||||||||||||||
| run: npx playwright install --with-deps chromium | ||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||
| echo "📦 Installing Chromium..." | ||||||||||||||||||||||||||
| npx playwright install --with-deps chromium | ||||||||||||||||||||||||||
| echo "✅ Install command completed (exit code: $?)" | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| echo "✅ Install command completed (exit code: $?)" | |
| EXIT_CODE=$? | |
| echo "✅ Install command completed (exit code: $EXIT_CODE)" |
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The verbose debugging output (lines 208-214) is useful for troubleshooting Playwright installation issues, but appears to be temporary diagnostic code.
Best practices for CI debugging instrumentation:
- Add context in the PR description explaining what issue is being investigated
- Remove once resolved: This verbose output adds noise to CI logs and should be removed after the issue is fixed
- Consider conditional debugging: Use a workflow input or environment variable to enable/disable verbose mode rather than having it always-on
If this is addressing a specific installation failure, please document the issue in the PR description and create a follow-up task to remove this debugging code once the root cause is identified and fixed.
| echo "✅ Install command completed (exit code: $?)" | |
| echo "📁 Checking browser cache..." | |
| ls -lR ~/.cache/ms-playwright/ 2>/dev/null || echo "Cache directory not found" | |
| echo "🔍 Searching for chromium executable..." | |
| find ~/.cache/ms-playwright -name "*chromium*" -o -name "*chrome*" 2>/dev/null | head -10 || echo "No chromium files found" |
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR title mentions "enhance Playwright installation steps" and "cache checks", but there is no explanation in the PR description about:
- What problem is being solved: Is there a current failure in Playwright browser installation? If so, what are the symptoms?
- Why these changes are needed: What issue do the debugging outputs help diagnose?
- Expected outcomes: Is this a permanent change or temporary diagnostic instrumentation?
- When to remove: If this is debugging code, when will it be removed?
Without this context, reviewers cannot assess:
- Whether the approach is correct for the actual problem
- If the changes should be permanent or temporary
- What success criteria indicate the issue is resolved
Please update the PR description to document the issue being addressed and the expected outcome of these changes.
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The separate "Install Playwright system dependencies" step is redundant. The npx playwright install --with-deps firefox command already installs both the browser binary AND all required system dependencies.
Running npx playwright install-deps firefox beforehand duplicates this work, adds unnecessary CI execution time, and provides no benefit. The --with-deps flag is specifically designed to be a single command that handles everything.
Recommendation: Remove the separate "Install Playwright system dependencies" step entirely.
| - name: Install Playwright system dependencies | |
| run: npx playwright install-deps firefox |
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exit code check on line 376 is misleading. The $? variable contains the exit code of the LAST command, which is the echo statement itself (not the npx playwright install command). This will always show exit code 0 even if Playwright installation failed.
To correctly capture the installation exit code, use:
npx playwright install --with-deps firefox
EXIT_CODE=$?
echo "✅ Install command completed (exit code: $EXIT_CODE)"However, if the primary goal is debugging, consider that a failed npx playwright install command will already cause the step to fail and stop the workflow. The exit code check adds limited value unless you're implementing custom error handling.
| echo "✅ Install command completed (exit code: $?)" | |
| EXIT_CODE=$? | |
| echo "✅ Install command completed (exit code: $EXIT_CODE)" |
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The separate "Install Playwright system dependencies" step is redundant. The npx playwright install --with-deps webkit command already installs both the browser binary AND all required system dependencies.
Running npx playwright install-deps webkit beforehand duplicates this work, adds unnecessary CI execution time, and provides no benefit. The --with-deps flag is specifically designed to be a single command that handles everything.
Recommendation: Remove the separate "Install Playwright system dependencies" step entirely.
| - name: Install Playwright system dependencies | |
| run: npx playwright install-deps webkit |
Copilot
AI
Feb 4, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exit code check on line 542 is misleading. The $? variable contains the exit code of the LAST command, which is the echo statement itself (not the npx playwright install command). This will always show exit code 0 even if Playwright installation failed.
To correctly capture the installation exit code, use:
npx playwright install --with-deps webkit
EXIT_CODE=$?
echo "✅ Install command completed (exit code: $EXIT_CODE)"However, if the primary goal is debugging, consider that a failed npx playwright install command will already cause the step to fail and stop the workflow. The exit code check adds limited value unless you're implementing custom error handling.
| echo "✅ Install command completed (exit code: $?)" | |
| echo "📁 Checking browser cache..." | |
| ls -lR ~/.cache/ms-playwright/ 2>/dev/null || echo "Cache directory not found" | |
| echo "🔍 Searching for webkit executable..." | |
| find ~/.cache/ms-playwright -name "*webkit*" -o -name "*MiniBrowser*" 2>/dev/null | head -10 || echo "No webkit files found" | |
| EXIT_CODE=$? | |
| echo "✅ Install command completed (exit code: $EXIT_CODE)" | |
| echo "📁 Checking browser cache..." | |
| ls -lR ~/.cache/ms-playwright/ 2>/dev/null || echo "Cache directory not found" | |
| echo "🔍 Searching for webkit executable..." | |
| find ~/.cache/ms-playwright -name "*webkit*" -o -name "*MiniBrowser*" 2>/dev/null | head -10 || echo "No webkit files found" | |
| exit "$EXIT_CODE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The separate "Install Playwright system dependencies" step is redundant. The
npx playwright install --with-deps chromiumcommand already installs both the browser binary AND all required system dependencies.Running
npx playwright install-deps chromiumbeforehand duplicates this work, adds unnecessary CI execution time, and provides no benefit. The--with-depsflag is specifically designed to be a single command that handles everything.Recommendation: Remove the separate "Install Playwright system dependencies" step entirely.