Skip to content

feat(ci): Add Windows runner for Playwright E2E tests #7769

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.

Sign up for GitHub

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

Closed
SBALAVIGNESH123 wants to merge 3 commits into from
Closed

feat(ci): Add Windows runner for Playwright E2E tests #7769

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5161400
feat(ci): Add Windows runner for Playwright E2E tests
SBALAVIGNESH123 Dec 27, 2025
5c09a3a
fix(ci): address windows ci feedback
SBALAVIGNESH123 Dec 27, 2025
17611f2
feat(ci): shard windows tests to prevent timeouts
SBALAVIGNESH123 Dec 28, 2025
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
48 changes: 45 additions & 3 deletions .github/workflows/ci-tests-e2e.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,51 @@ jobs:
path: ./playwright-report/
retention-days: 30

playwright-tests-windows:
timeout-minutes: 20
# needs: setup -- removed dependency on setup job cache
runs-on: windows-latest
permissions:
contents: read
strategy:
fail-fast: false
matrix:
shard: [1, 2, 3, 4, 5, 6, 7, 8]
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider renaming for consistency.

The Linux chromium job uses matrix.shardIndex (line 54), while this Windows job uses matrix.shard. For maintainability, consider using shardIndex here as well.

🔎 Proposed change for consistency 
       matrix:
-        shard: [1, 2, 3, 4, 5, 6, 7, 8]
+        shardIndex: [1, 2, 3, 4, 5, 6, 7, 8]

Then update references on lines 178 and 188:

-          pnpm exec playwright test --project=chromium --reporter=blob --shard=${{ matrix.shard }}/8
+          pnpm exec playwright test --project=chromium --reporter=blob --shard=${{ matrix.shardIndex }}/8
-          name: blob-report-windows-${{ matrix.shard }}
+          name: blob-report-windows-${{ matrix.shardIndex }}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
.github/workflows/ci-tests-e2e.yaml around line 159: the Windows job defines the
shard matrix key as "shard" while the Linux job uses "shardIndex", causing
inconsistency; rename the matrix key from "shard" to "shardIndex" and update any
references that use matrix.shard to matrix.shardIndex (notably at lines 178 and
188) so the variable name is consistent across jobs.

steps:
- name: Checkout repository
uses: actions/checkout@v5

- name: Setup ComfyUI server
uses: ./.github/actions/setup-comfyui-server
with:
launch_server: true
- name: Setup nodejs, pnpm, reuse built frontend
uses: ./.github/actions/setup-frontend
with:
include_build_step: true # Build explicitly on Windows since we aren't restoring cache
- name: Setup Playwright
uses: ./.github/actions/setup-playwright

- name: Run Playwright tests (Windows - chromium)
id: playwright
run: |
pnpm exec playwright test --project=chromium --reporter=blob --shard=${{ matrix.shard }}/8
env:
PLAYWRIGHT_BLOB_OUTPUT_DIR: ./blob-report
shell: bash

- name: Upload Blob Report
uses: actions/upload-artifact@v4
if: always()
Comment on lines +184 to +185
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Align upload condition with Linux sharded job.

The Windows job uses if: always(), but the Linux chromium job uses if: ${{ !cancelled() }} (line 86). The !cancelled() condition is more appropriate as it uploads reports on success or failure but skips uploading if the workflow is cancelled, saving storage costs.

🔎 Proposed fix 
       - name: Upload Blob Report
         uses: actions/upload-artifact@v4
-        if: always()
+        if: ${{ !cancelled() }}
         with:
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
uses: actions/upload-artifact@v4
if: always()
uses: actions/upload-artifact@v4
if: ${{ !cancelled() }}
🤖 Prompt for AI Agents 
In .github/workflows/ci-tests-e2e.yaml around lines 184-185 the Windows job uses
"if: always()", which uploads artifacts even when the workflow is cancelled;
change the condition to "if: ${{ !cancelled() }}" to match the Linux chromium
job and ensure artifacts are uploaded on success or failure but skipped when the
workflow is cancelled.

with:
# Use a unique name for each shard's blob report
name: blob-report-windows-${{ matrix.shard }}
path: ./blob-report
retention-days: 1

# Merge sharded test reports
merge-reports:
needs: [playwright-tests-chromium-sharded]
needs: [playwright-tests-chromium-sharded, playwright-tests-windows]
runs-on: ubuntu-latest
if: ${{ !cancelled() }}
steps:
Expand All @@ -166,7 +208,7 @@ jobs:
uses: actions/download-artifact@v4
with:
path: ./all-blob-reports
pattern: blob-report-chromium-*
pattern: blob-report-*
merge-multiple: true

- name: Merge into HTML Report
Expand Down Expand Up @@ -215,7 +257,7 @@ jobs:

# Deploy and comment for non-forked PRs only
deploy-and-comment:
needs: [playwright-tests, merge-reports]
needs: [playwright-tests, merge-reports, playwright-tests-windows]
runs-on: ubuntu-latest
if: always() && github.event_name == 'pull_request' && github.event.pull_request.head.repo.fork == false
permissions:
Expand Down