Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions .github/workflows/e2e-tests-split.yml
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,18 @@ jobs:
- name: Install dependencies
run: npm ci

- name: Install Playwright system dependencies
run: npx playwright install-deps chromium

Comment on lines +203 to +205
Copy link

Copilot AI Feb 4, 2026

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 chromium command already installs both the browser binary AND all required system dependencies.

Running npx playwright install-deps chromium 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.

Suggested change
- name: Install Playwright system dependencies
run: npx playwright install-deps chromium

Copilot uses AI. Check for mistakes.
- 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: $?)"
Copy link

Copilot AI Feb 4, 2026

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 210 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 chromium
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.

Suggested change
echo "✅ Install command completed (exit code: $?)"
EXIT_CODE=$?
echo "✅ Install command completed (exit code: $EXIT_CODE)"

Copilot uses AI. Check for mistakes.
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"
Comment on lines +210 to +214
Copy link

Copilot AI Feb 4, 2026

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:

  1. Add context in the PR description explaining what issue is being investigated
  2. Remove once resolved: This verbose output adds noise to CI logs and should be removed after the issue is fixed
  3. 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.

Suggested change
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 uses AI. Check for mistakes.
Comment on lines +203 to +214
Copy link

Copilot AI Feb 4, 2026

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:

  1. What problem is being solved: Is there a current failure in Playwright browser installation? If so, what are the symptoms?
  2. Why these changes are needed: What issue do the debugging outputs help diagnose?
  3. Expected outcomes: Is this a permanent change or temporary diagnostic instrumentation?
  4. 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 uses AI. Check for mistakes.

- name: Run Chromium tests (Shard ${{ matrix.shard }}/${{ matrix.total-shards }})
run: |
Expand Down Expand Up @@ -356,8 +366,18 @@ jobs:
- name: Install dependencies
run: npm ci

- name: Install Playwright system dependencies
run: npx playwright install-deps firefox

Comment on lines +369 to +371
Copy link

Copilot AI Feb 4, 2026

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.

Suggested change
- name: Install Playwright system dependencies
run: npx playwright install-deps firefox

Copilot uses AI. Check for mistakes.
- name: Install Playwright Firefox
run: npx playwright install --with-deps firefox
run: |
echo "📦 Installing Firefox..."
npx playwright install --with-deps firefox
echo "✅ Install command completed (exit code: $?)"
Copy link

Copilot AI Feb 4, 2026

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.

Suggested change
echo "✅ Install command completed (exit code: $?)"
EXIT_CODE=$?
echo "✅ Install command completed (exit code: $EXIT_CODE)"

Copilot uses AI. Check for mistakes.
echo "📁 Checking browser cache..."
ls -lR ~/.cache/ms-playwright/ 2>/dev/null || echo "Cache directory not found"
echo "🔍 Searching for firefox executable..."
find ~/.cache/ms-playwright -name "*firefox*" 2>/dev/null | head -10 || echo "No firefox files found"

- name: Run Firefox tests (Shard ${{ matrix.shard }}/${{ matrix.total-shards }})
run: |
Expand Down Expand Up @@ -512,8 +532,18 @@ jobs:
- name: Install dependencies
run: npm ci

- name: Install Playwright system dependencies
run: npx playwright install-deps webkit

Comment on lines +535 to +537
Copy link

Copilot AI Feb 4, 2026

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.

Suggested change
- name: Install Playwright system dependencies
run: npx playwright install-deps webkit

Copilot uses AI. Check for mistakes.
- name: Install Playwright WebKit
run: npx playwright install --with-deps webkit
run: |
echo "📦 Installing WebKit..."
npx playwright install --with-deps webkit
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"

Comment on lines +542 to 547
Copy link

Copilot AI Feb 4, 2026

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.

Suggested change
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"

Copilot uses AI. Check for mistakes.
- name: Run WebKit tests (Shard ${{ matrix.shard }}/${{ matrix.total-shards }})
run: |
Expand Down
Loading