Skip to content
Merged
Show file tree
Hide file tree
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
37 changes: 30 additions & 7 deletions .github/workflows/playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,33 @@ on:

jobs:
e2e:
name: E2E (Playwright, ${{ matrix.browser }})
name: ${{ matrix.jobName }}
runs-on: ubuntu-latest
timeout-minutes: 35
strategy:
fail-fast: false
matrix:
browser:
- chromium
- webkit
include:
- browser: chromium
jobName: E2E (Playwright, chromium)
workers: 2
shardIndex: 1
shardTotal: 1
- browser: webkit
jobName: E2E (Playwright, webkit, shard 1/3)
workers: 1
shardIndex: 1
shardTotal: 3
- browser: webkit
jobName: E2E (Playwright, webkit, shard 2/3)
workers: 1
shardIndex: 2
shardTotal: 3
- browser: webkit
jobName: E2E (Playwright, webkit, shard 3/3)
workers: 1
shardIndex: 3
shardTotal: 3

steps:
- name: Checkout
Expand Down Expand Up @@ -54,20 +72,25 @@ jobs:
- name: Run Playwright Tests
env:
CI: 'true'
run: npm run test:e2e -- --project=${{ matrix.browser }}
run: |
SHARD_ARGS=''
if [ '${{ matrix.shardTotal }}' != '1' ]; then
SHARD_ARGS="--shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }}"
fi
npm run test:e2e -- --project=${{ matrix.browser }} --workers=${{ matrix.workers }} $SHARD_ARGS

- name: Upload Playwright report
uses: actions/upload-artifact@v6.0.0
if: ${{ failure() }}
with:
name: playwright-report-${{ matrix.browser }}
name: playwright-report-${{ matrix.browser }}-${{ matrix.shardIndex }}of${{ matrix.shardTotal }}
path: playwright-report
if-no-files-found: ignore

- name: Upload Playwright test results
uses: actions/upload-artifact@v6.0.0
if: ${{ failure() }}
with:
name: test-results-${{ matrix.browser }}
name: test-results-${{ matrix.browser }}-${{ matrix.shardIndex }}of${{ matrix.shardTotal }}
path: test-results
if-no-files-found: ignore
144 changes: 144 additions & 0 deletions docs/webkit-open-pr-drawer-triage.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
# WebKit Open PR Drawer Triage Handoff

Date: 2026-03-29
Repository: develop
Branch: bananas

## Scope

This document captures investigation and fixes for WebKit failures in Playwright tests in playwright/github-pr-drawer.spec.ts.

Original failing subset:

- Open PR drawer confirms and submits component/styles filepaths
- Open PR drawer validates unsafe filepaths
- Open PR drawer allows dotted file segments that are not traversal

## Original symptoms

- Test flow appeared to continue behind a confirmation dialog in WebKit.
- Assertions expecting browser dialog events failed (browserDialogSeen false).
- In some runs, Open PR status remained at the default neutral text:
Configure repository, file paths, and branch details.

## Root cause summary

- The app uses a shared in-page dialog opened by showModal for Open PR confirmation when confirmBeforeSubmit is wired.
- In WebKit, top-layer dialog timing and interaction can be flaky for test automation.
- The old test logic and app behavior supported two confirmation paths (HTML dialog and native confirm), which increased test complexity and flake risk.

## Relevant app wiring (for context)

- PR drawer submit uses confirmBeforeSubmit when available in src/modules/github-pr-drawer.js.
- App passes confirmBeforeSubmit to confirmAction in src/app.js.
- confirmAction opens the shared dialog element with id clear-confirm-dialog via showModal.
- Native window.confirm fallback has been removed.

## Changes made

Files:

- playwright/github-pr-drawer.spec.ts
- src/app.js
- src/modules/github-pr-drawer.js

1. Simplified tests to one confirmation path: HTML dialog only.
2. Removed dual-path test handling (native browser dialog vs in-page dialog).
3. Removed manual loop-based visibility fallback logic in tests.
4. Kept stable submit/confirm interactions and path-input stabilization in PR drawer tests.
5. Removed fallbackConfirmText usage from app call sites and PR drawer payload.
6. Removed native window.confirm fallback logic from confirmAction.
7. Unsupported dialog environments now no-op silently instead of showing a fallback prompt.

## Current status

Latest runs are green:

Command:
CI=true npx playwright test playwright/github-pr-drawer.spec.ts --project=webkit --headed --grep "Open PR drawer confirms and submits component/styles filepaths"
Result:
1 passed

Command:
CI=true npx playwright test playwright/github-pr-drawer.spec.ts --project=webkit --headed --grep "Open PR drawer confirms and submits component/styles filepaths|Open PR drawer allows dotted file segments that are not traversal|Open PR drawer validates unsafe filepaths"
Result:
3 passed

Command:
npx playwright test playwright/github-pr-drawer.spec.ts --project=chromium
Result:
15 passed

Command:
CI=true npx playwright test playwright/github-pr-drawer.spec.ts --project=webkit
Result:
15 passed

## Local changes currently present

- playwright/github-pr-drawer.spec.ts (modal-only confirmation helper refactor)
- src/app.js (removed native confirm fallback and fallbackConfirmText usage)
- src/modules/github-pr-drawer.js (removed fallbackConfirmText in submit confirmation payload)
- playwright.config.ts (workers changed from CI 2 to CI 1)
- package.json (added test:e2e:webkit using CI=true)

Note: playwright.config.ts worker change may be unrelated to this issue. Treat it as separate unless intentionally part of CI stabilization.

## Evergreen policy alignment

- Project policy is evergreen browsers only.
- HTML dialog support is assumed.
- No compatibility fallback is maintained for non-supporting/legacy environments.

## If this flakes again

1. Re-run only failing test first with headed mode and trace.
2. Confirm clear-confirm-dialog opens and confirm button click is delivered.
3. Confirm request flow occurs by adding temporary request counters for:

- git refs create
- contents upsert
- pulls create

4. If status remains default, verify submit click dispatch with a temporary event probe in test page context.
5. Keep assertions outcome-based (status/request side effects), not browser-event-path assumptions.

## Suggested next validation

1. Run full github-pr-drawer.spec.ts on WebKit.
2. Run same spec on Chromium and Firefox to ensure no regressions from helper changes.
3. If stable, keep helper approach and remove any temporary probes.

## Minimal rollback plan

Use this only if WebKit flakiness returns and you need to quickly de-risk test helper changes.

1. In playwright/github-pr-drawer.spec.ts, rollback only interaction mechanics first:

- replace DOM evaluate click calls with standard Playwright click calls.

2. Keep path validation stabilizers unless proven harmful:

- keep explicit toHaveValue checks.
- keep blur before submit in validation tests.

3. If still flaky, narrow rollback to failing tests only:

- inline confirmation flow in the failing tests and temporarily bypass shared helper usage.

4. Do not reintroduce native confirm fallback as a flake workaround.
5. Do not mix rollback with config churn:

- avoid changing playwright.config.ts in the same rollback commit unless worker count is confirmed root cause.

Quick validation after each rollback step:

Command:
CI=true npx playwright test playwright/github-pr-drawer.spec.ts --project=webkit --headed --grep "Open PR drawer confirms and submits component/styles filepaths"

Then:

Command:
CI=true npx playwright test playwright/github-pr-drawer.spec.ts --project=webkit --headed --grep "Open PR drawer confirms and submits component/styles filepaths|Open PR drawer allows dotted file segments that are not traversal|Open PR drawer validates unsafe filepaths"

Stop rollback at first stable step and keep the smallest diff that remains green.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"check-types": "tsc -p tsconfig.json",
"lint:playwright": "eslint playwright playwright.config.ts src/index.html",
"test:e2e": "npm run build && PLAYWRIGHT_WEB_SERVER_MODE=preview PLAYWRIGHT_PORT=8081 playwright test",
"test:e2e:webkit": "npm run build && PLAYWRIGHT_INCLUDE_WEBKIT=true PLAYWRIGHT_WEB_SERVER_MODE=preview PLAYWRIGHT_PORT=8081 playwright test --project=webkit",
"test:e2e:dev": "playwright test",
"test:e2e:headed": "npm run build && PLAYWRIGHT_WEB_SERVER_MODE=preview PLAYWRIGHT_PORT=8081 playwright test --headed",
"test": "echo \"Error: no test specified\" && exit 1",
Expand Down
5 changes: 3 additions & 2 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { defineConfig, devices } from '@playwright/test'

const env = process.env
const isCI = env?.CI === 'true'
const includeWebKit = env?.PLAYWRIGHT_INCLUDE_WEBKIT === 'true'
const HOST = env?.PLAYWRIGHT_HOST ?? '127.0.0.1'
const PORT = Number(env?.PLAYWRIGHT_PORT ?? 4174)
const baseURL = env?.PLAYWRIGHT_BASE_URL ?? `http://${HOST}:${PORT}`
Expand All @@ -20,7 +21,7 @@ const projects = [
},
]

if (isCI) {
if (isCI || includeWebKit) {
projects.push({
name: 'webkit',
use: { ...devices['Desktop Safari'] },
Expand All @@ -31,7 +32,7 @@ export default defineConfig({
testDir: 'playwright',
timeout: isCI ? 120_000 : 20_000,
retries: isCI ? 1 : 0,
workers: isCI ? 2 : undefined,
workers: isCI ? 1 : undefined,
expect: {
timeout: isCI ? 90_000 : 15_000,
},
Expand Down
Loading