feat(testing): implement playwright parallelization improvements #82
Conversation
- Optimize local development with 50% CPU worker allocation - Add CI sharding support for 4x faster test execution - Maintain single worker in CI for stability by default - Add comprehensive parallelization documentation Performance improvements: - Local development: ~6 minutes (50% workers) - CI with sharding: ~4 minutes (4x parallel jobs) - CI standard: ~15 minutes (stable single worker) Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds configurable sharded E2E execution to the GitHub Actions workflow with a two-phase Setup→e2e-tests flow (artifact upload/download, secret validation gating), updates Playwright local worker defaults, switches Playwright action to yarn scoped to the app subtree, and documents parallelization and sharding. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Workflow Caller
participant GH as GitHub Actions
participant Setup as Setup Job
participant Artifacts as Artifact Store
participant E2E as e2e-tests (matrix)
participant Runner as Playwright Runner
Note over GH,Setup: Workflow input includes shard-count (default 4)
User->>GH: call reusable workflow
GH->>Setup: run Setup job (validate secrets, build, upload artifacts)
Setup->>Artifacts: upload build artifacts
Setup-->>GH: outputs can-run-tests (true/false)
alt can-run-tests == 'true'
GH->>E2E: start matrix jobs (shard 1..N)
loop per shard
E2E->>Artifacts: download build artifacts
E2E->>Runner: run tests with --shard=Index/N
Runner-->>E2E: report results
alt shard == 1
E2E->>GH: post PR comment with shard summary
end
end
else can-run-tests == 'false'
GH->>E2E: skip e2e-tests (gated)
E2E-->>GH: generate skipped summary
end
E2E-->>GH: aggregate test-results outputs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Comment |
github-actions
bot
requested review from
andrest50,
audigregorie,
dealako and
mauriciozanettisalomao
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive Playwright test parallelization improvements to enhance both local development experience and CI execution speed while maintaining stability. The changes introduce optional CI sharding for 4x faster execution and optimize local development with 50% CPU worker allocation.
- Local development optimization with 50% CPU workers for ~6 minute execution times
- Optional CI sharding support enabling 4-way parallel execution reducing CI time from ~15 to ~4 minutes
- Comprehensive documentation with performance comparisons and best practices
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
apps/lfx-pcc/playwright.config.ts |
Updates worker configuration for local development optimization |
.github/workflows/e2e-tests.yml |
Adds sharding support with matrix strategy for parallel CI execution |
docs/architecture/testing/e2e-testing.md |
Adds comprehensive parallelization documentation with performance comparisons |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
.github/workflows/e2e-tests.yml
Outdated
| enable-sharding: | ||
| description: 'Enable test sharding for faster parallel execution' | ||
| required: false | ||
| default: true |
There was a problem hiding this comment.
The default value for enable-sharding should be false to maintain backwards compatibility and stability. Setting it to true by default could introduce unexpected behavior for existing workflows that weren't designed for sharding.
| default: true | |
| default: false |
.github/workflows/e2e-tests.yml
Outdated
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| shard: ${{ inputs.enable-sharding && fromJSON('[1, 2, 3, 4]') || fromJSON('[0]') }} |
There was a problem hiding this comment.
The hardcoded shard array [1, 2, 3, 4] should be dynamically generated based on inputs.shard-count to respect the configurable shard count parameter. Consider using a range function or generating the array based on the input value.
| shard: ${{ inputs.enable-sharding && fromJSON('[1, 2, 3, 4]') || fromJSON('[0]') }} | |
| shard: ${{ inputs.enable-sharding && fromJSON('[' + join(fromJSON(format('[{0}]', join(',', range(1, inputs.shard-count + 1)))), ',') + ']') || fromJSON('[0]') }} |
✅ E2E Tests PassedBrowser: chromium All E2E tests passed successfully across 4 parallel shards. Test Configuration
|
- Replace npm commands with yarn for monorepo consistency - Fix version detection to work with workspace structure - Use correct directory context for playwright operations - Cache keys now resolve to actual versions (e.g. 1.54.2) instead of null This fixes the GitHub Actions cache miss issue where browser binaries weren't being cached properly, significantly reducing CI execution time. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-tests.yml (1)
261-276: Use step outputs (exit codes) instead of probing files for pass/fail.
.last-run.jsonisn’t reliable. Capture exit codes from both paths and summarize accordingly.- name: Run E2E tests (All browsers) - id: run-tests-all + id: run-tests-all if: ${{ inputs.browser == 'all' && steps.validate-secrets.outputs.can_run_tests == 'true' }} working-directory: apps/lfx-pcc continue-on-error: true run: | if [ -n "$TEST_USERNAME" ] && [ -n "$TEST_PASSWORD" ]; then echo "🔐 Running authenticated E2E tests on all browsers" echo "🚀 Playwright will automatically start the dev server on localhost:4200" echo "📋 Using secrets from AWS Secrets Manager" if [ "${{ inputs.enable-sharding }}" == "true" ]; then echo "🔀 Running with sharding: ${{ matrix.shard }}/${{ inputs.shard-count }}" yarn ${{ inputs.test-command }} --reporter=list --shard=${{ matrix.shard }}/${{ inputs.shard-count }} else yarn ${{ inputs.test-command }} --reporter=list fi - echo "test_exit_code=$?" >> $GITHUB_OUTPUT + echo "test_exit_code=$?" >> $GITHUB_OUTPUT else echo "⚠️ No test credentials provided. Skipping E2E tests." echo "Set TEST_USERNAME and TEST_PASSWORD secrets to enable E2E tests." echo "test_exit_code=0" >> $GITHUB_OUTPUT exit 0 fi @@ - - name: Run E2E tests (Specific browser) + - name: Run E2E tests (Specific browser) + id: run-tests-specific if: ${{ inputs.browser != 'all' && steps.validate-secrets.outputs.can_run_tests == 'true' }} working-directory: apps/lfx-pcc + continue-on-error: true run: | if [ -n "$TEST_USERNAME" ] && [ -n "$TEST_PASSWORD" ]; then echo "🔐 Running authenticated E2E tests on ${{ inputs.browser }}" echo "🚀 Playwright will automatically start the dev server on localhost:4200" echo "📋 Using secrets from AWS Secrets Manager" if [ "${{ inputs.enable-sharding }}" == "true" ]; then echo "🔀 Running with sharding: ${{ matrix.shard }}/${{ inputs.shard-count }}" yarn ${{ inputs.test-command }} --project=${{ inputs.browser }} --reporter=list --shard=${{ matrix.shard }}/${{ inputs.shard-count }} else yarn ${{ inputs.test-command }} --project=${{ inputs.browser }} --reporter=list fi + echo "test_exit_code=$?" >> $GITHUB_OUTPUT else echo "⚠️ No test credentials provided. Skipping E2E tests." echo "Set TEST_USERNAME and TEST_PASSWORD secrets to enable E2E tests." exit 0 fi @@ - name: Generate test results summary id: test-results if: always() working-directory: apps/lfx-pcc run: | if [ "${{ steps.validate-secrets.outputs.can_run_tests }}" == "false" ]; then echo "⏭️ E2E tests skipped (missing required secrets)" echo "results=skipped" >> $GITHUB_OUTPUT elif [ -z "$TEST_USERNAME" ] || [ -z "$TEST_PASSWORD" ]; then echo "⏭️ E2E tests skipped (no test credentials)" echo "results=skipped" >> $GITHUB_OUTPUT - elif [ -f "test-results/.last-run.json" ]; then - echo "✅ E2E tests completed successfully" - echo "results=success" >> $GITHUB_OUTPUT else - echo "❌ E2E tests failed" - echo "results=failure" >> $GITHUB_OUTPUT + EXIT_CODE="${{ steps.run-tests-all.outputs.test_exit_code }}" + if [ -z "$EXIT_CODE" ]; then + EXIT_CODE="${{ steps.run-tests-specific.outputs.test_exit_code }}" + fi + if [ "$EXIT_CODE" = "0" ]; then + echo "✅ E2E tests completed successfully" + echo "results=success" >> $GITHUB_OUTPUT + else + echo "❌ E2E tests failed" + echo "results=failure" >> $GITHUB_OUTPUT + fi fiAlso applies to: 285-299, 320-337
🧹 Nitpick comments (3)
apps/lfx-pcc/playwright.config.ts (1)
21-22: Allow local override of workers via env var (keeps CI=1).Useful when 50% is still too heavy/light on some dev machines.
- /* Opt out of parallel tests on CI, optimize for local development. */ - workers: process.env.CI ? 1 : '50%', + /* Opt out of parallel tests on CI; allow local override via PW_WORKERS for devs. */ + workers: process.env.CI ? 1 : (process.env.PW_WORKERS || '50%'),docs/architecture/testing/e2e-testing.md (2)
75-83: Clarify default: sharding disabled by default; enable per workflow call.Avoids confusion with “stability first” guidance.
-#### Default Single Worker (Recommended) +#### Default Single Worker (Default & Recommended) - **Stability First**: Single worker ensures reproducible results - **Resource Efficiency**: Optimal for standard GitHub runners - **Debugging Friendly**: Sequential execution simplifies failure analysis + +Note: In CI, sharding is disabled by default. Enable it explicitly in the caller workflow: +`with: enable-sharding: true`.
94-99: Time-bound performance note.Benchmarks drift; add a date to set expectations.
-**Performance Comparison**: +**Performance Comparison** (as of 2025-09-11):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/e2e-tests.yml(4 hunks)apps/lfx-pcc/playwright.config.ts(1 hunks)docs/architecture/testing/e2e-testing.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer TypeScript interfaces over union types when modeling shapes and contracts
Files:
apps/lfx-pcc/playwright.config.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-pcc/playwright.config.ts
**/*.{ts,tsx,js,jsx,html,scss,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Include required license headers on all source files
Files:
apps/lfx-pcc/playwright.config.ts
🧠 Learnings (1)
📚 Learning: 2025-09-05T18:09:48.792Z
Learnt from: CR
PR: linuxfoundation/lfx-v2-ui#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T18:09:48.792Z
Learning: Applies to **/{*.spec.ts,*-robust.spec.ts} : Maintain dual E2E test architecture: content-based (*.spec.ts) and structural (*-robust.spec.ts) tests
Applied to files:
docs/architecture/testing/e2e-testing.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: E2E Tests / Playwright E2E Tests (Shard 4/4)
- GitHub Check: E2E Tests / Playwright E2E Tests (Shard 2/4)
- GitHub Check: E2E Tests / Playwright E2E Tests (Shard 3/4)
- GitHub Check: E2E Tests / Playwright E2E Tests (Shard 1/4)
- Remove backward compatibility for enable-sharding - Always use 4 parallel shards for faster execution - Split into setup job (validate, build once) and test jobs (run shards) - Build application once in setup, share via artifacts - Each test job downloads artifacts and runs 1 shard - Reduces total execution time by ~12 minutes (3 min build × 4 shards) - PR comments only from shard 1 to avoid spam - Maintain proper AWS secrets handling in each test job 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/e2e-tests.yml (1)
12-45: Restore opt-in sharding and honor shard-count dynamicallyThe matrix is hardcoded to 4 and there’s no enable-sharding input, which conflicts with the “stability-first” goal and ignores shard-count. Add an enable-sharding boolean (default false) and generate shards from 1..N.
workflow_call: inputs: + enable-sharding: + description: 'Enable test sharding for faster parallel execution' + required: false + default: false + type: boolean node-version: description: 'Node.js version to use' @@ - strategy: - fail-fast: false - matrix: - shard: [1, 2, 3, 4] + strategy: + fail-fast: false + matrix: + shard: ${{ inputs.enable-sharding && fromJSON(format('[{0}]', join(range(1, inputs['shard-count'] + 1), ','))) || fromJSON('[1]') }}Note: range(start, end) is inclusive-exclusive; adding 1 ensures inclusion of shard-count. If you prefer, we can move matrix generation to a tiny “prepare-matrix” job that outputs JSON for arbitrary counts.
Also applies to: 184-187
🧹 Nitpick comments (5)
.github/actions/setup-playwright/action.yml (2)
18-19: Harden version extraction and cache key when @playwright/test is missingIf the devDependency is absent or jq returns null/empty, the cache key becomes invalid and subsequent steps may fail. Guard and fail fast with a clear message.
- PLAYWRIGHT_VERSION=$(cat apps/lfx-pcc/package.json | jq -r '.devDependencies["@playwright/test"]' | sed 's/[\^~]//') - echo "PLAYWRIGHT_VERSION=$PLAYWRIGHT_VERSION" >> $GITHUB_ENV + PLAYWRIGHT_VERSION=$(jq -r '.devDependencies["@playwright/test"] // empty' apps/lfx-pcc/package.json | sed 's/[\^~]//') + if [ -z "$PLAYWRIGHT_VERSION" ] || [ "$PLAYWRIGHT_VERSION" = "null" ]; then + echo "❌ Could not determine @playwright/test version from apps/lfx-pcc/package.json"; exit 1 + fi + echo "PLAYWRIGHT_VERSION=$PLAYWRIGHT_VERSION" >> "$GITHUB_ENV"
36-41: Prefer working-directory over manual cd for clarity and fewer pitfallsUsing the step’s working-directory is cleaner and avoids accidental directory persistence if the script grows.
- shell: bash - run: | - cd apps/lfx-pcc + shell: bash + working-directory: apps/lfx-pcc + run: | if [ "${{ inputs.browser }}" == "all" ]; then yarn playwright install --with-deps else yarn playwright install --with-deps ${{ inputs.browser }} fi- shell: bash - run: | - cd apps/lfx-pcc + shell: bash + working-directory: apps/lfx-pcc + run: | if [ "${{ inputs.browser }}" == "all" ]; then yarn playwright install-deps else yarn playwright install-deps ${{ inputs.browser }} fiAlso applies to: 48-53
.github/workflows/e2e-tests.yml (3)
238-238: Remove trailing whitespace (yamllint error)Line 238 has trailing spaces per YAMLlint. Clean it to keep CI lint happy.
- +
300-308: Consistent tolerance: align continue-on-error between both test paths“All browsers” path uses continue-on-error, specific browser path does not. Pick one policy; for parity, consider enabling it here too if that’s the intent.
- - name: Run E2E tests (Specific browser) - if: ${{ inputs.browser != 'all' }} + - name: Run E2E tests (Specific browser) + if: ${{ inputs.browser != 'all' }} working-directory: apps/lfx-pcc - run: | + continue-on-error: true + run: |
167-175: Artifact scope: reconsider uploading node_modules/.cache/This can be large and slow downloads/uploads. Since setup-node caches Yarn, prefer excluding this unless it materially speeds tests.
name: build-artifacts path: | apps/lfx-pcc/dist/ - node_modules/.cache/ retention-days: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/actions/setup-playwright/action.yml(2 hunks).github/workflows/e2e-tests.yml(8 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/e2e-tests.yml
[error] 238-238: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Setup Dependencies and Build
- Add packages/shared/dist/ to artifact upload - Include .turbo/ cache directory for Turborepo optimization - Ensures test jobs have all required built packages to run successfully The shared package build was missing from artifacts, causing test jobs to fail when trying to import from @lfx-pcc/shared. 🤖 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
.github/workflows/e2e-tests.yml (4)
327-369: Remove hardcoded “4 shards” and fix hyphenated input references in PR comment.- details = 'All E2E tests passed successfully across 4 parallel shards.'; + const shardCount = ${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }}; + details = `All E2E tests passed successfully${shardCount > 1 ? ' across ' + shardCount + ' parallel shards' : ''}.`;- **Shards:** 4 parallel executions + **Shards:** ${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }} parallel execution${{ (inputs['enable-sharding'] && inputs['shard-count'] || 1) > 1 && 's' || '' }}- - **Shards:** ${{ inputs.shard-count }} + - **Shards:** ${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }}
181-181: Use bracket notation for hyphenated input keys in the job name.- name: Playwright E2E Tests (Shard ${{ matrix.shard }}/${{ inputs.shard-count }}) + name: Playwright E2E Tests (Shard ${{ matrix.shard }}/${{ inputs['shard-count'] }})
163-166: Fix input reference with hyphen (must use bracket notation).- if: steps.validate-secrets.outputs.can_run_tests == 'true' && !inputs.skip-build + if: steps.validate-secrets.outputs.can_run_tests == 'true' && !inputs['skip-build']
40-44: Add an explicit enable-sharding input (default false) to keep stability-first behavior.Right now sharding is always active later via the matrix. Introduce a boolean toggle so callers must opt-in.
skip-build: description: 'Skip building the application (use existing build)' required: false default: false type: boolean + enable-sharding: + description: 'Enable test sharding for faster parallel execution' + required: false + default: false + type: boolean shard-count: description: 'Number of shards for parallel execution (2-8)' required: false default: 4 type: number
🧹 Nitpick comments (3)
.github/workflows/e2e-tests.yml (3)
229-239: Honor base-url input in environment instead of hardcoding localhost.Propagate inputs['base-url'] to PCC_BASE_URL and logs so callers can target non-local hosts.
- echo "PCC_BASE_URL=http://localhost:4200" >> $GITHUB_ENV + echo "PCC_BASE_URL=${{ inputs['base-url'] }}" >> $GITHUB_ENV
240-240: Trim trailing whitespace (yamllint).There’s trailing space on this blank line; remove to satisfy linters.
- +
179-186: Optional: add per-shard concurrency to auto-cancel superseded runs on PRs.e2e-tests: name: Playwright E2E Tests (Shard ${{ matrix.shard }}/${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }}) runs-on: ubuntu-latest timeout-minutes: 15 needs: [setup] if: ${{ needs.setup.outputs.can-run-tests == 'true' }} + concurrency: + group: e2e-${{ github.ref }}-${{ matrix.shard }} + cancel-in-progress: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/e2e-tests.yml(8 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/e2e-tests.yml
[error] 240-240: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: E2E Tests / Setup Dependencies and Build
🔇 Additional comments (2)
.github/workflows/e2e-tests.yml (2)
63-71: Good gating: single-setup with secret validation and explicit output.The setup job cleanly centralizes auth + validation and exposes a simple boolean for downstream jobs. Nice.
281-285: Composite action usage looks solid.Scoped browser input and local action path are consistent with the setup phase.
| - name: Upload build artifacts | ||
| if: steps.validate-secrets.outputs.can_run_tests == 'true' | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: build-artifacts | ||
| path: | | ||
| packages/shared/dist/ | ||
| apps/lfx-pcc/dist/ | ||
| node_modules/.cache/ | ||
| .turbo/ | ||
| retention-days: 1 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard artifact upload when skipping build (or ignore if empty).
If skip-build is true, this step may warn/fail due to missing paths. Guard with the same condition or set if-no-files-found: ignore.
- - name: Upload build artifacts
- if: steps.validate-secrets.outputs.can_run_tests == 'true'
+ - name: Upload build artifacts
+ if: steps.validate-secrets.outputs.can_run_tests == 'true' && !inputs['skip-build']
uses: actions/upload-artifact@v4
with:
name: build-artifacts
path: |
packages/shared/dist/
apps/lfx-pcc/dist/
node_modules/.cache/
.turbo/
retention-days: 1
+ if-no-files-found: ignore📝 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.
| - name: Upload build artifacts | |
| if: steps.validate-secrets.outputs.can_run_tests == 'true' | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: build-artifacts | |
| path: | | |
| packages/shared/dist/ | |
| apps/lfx-pcc/dist/ | |
| node_modules/.cache/ | |
| .turbo/ | |
| retention-days: 1 | |
| - name: Upload build artifacts | |
| if: steps.validate-secrets.outputs.can_run_tests == 'true' && !inputs['skip-build'] | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: build-artifacts | |
| path: | | |
| packages/shared/dist/ | |
| apps/lfx-pcc/dist/ | |
| node_modules/.cache/ | |
| .turbo/ | |
| retention-days: 1 | |
| if-no-files-found: ignore |
🤖 Prompt for AI Agents
.github/workflows/e2e-tests.yml lines 167-178: the artifact upload step can fail
or warn when build was skipped because the paths don't exist; either guard the
step with the same skip-build condition (e.g. include the skip-build check
alongside the existing if condition) so it only runs when a build actually
produced files, or keep the step unconditional but add with: if-no-files-found:
ignore to prevent failures when paths are missing.
| # E2E tests - runs in parallel shards after setup | ||
| e2e-tests: | ||
| name: Playwright E2E Tests (Shard ${{ matrix.shard }}/${{ inputs.shard-count }}) | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 15 | ||
| needs: [setup] | ||
| if: ${{ needs.setup.outputs.can-run-tests == 'true' }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| shard: [1, 2, 3, 4] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Bug: matrix hardcodes 4 shards, ignores shard-count, and can’t be disabled.
This will under/over-run tests vs the provided denominator and forces sharding even when not desired. Generate the shard array from shard-count and gate it behind enable-sharding.
- e2e-tests:
- name: Playwright E2E Tests (Shard ${{ matrix.shard }}/${{ inputs.shard-count }})
+ e2e-tests:
+ name: Playwright E2E Tests (Shard ${{ matrix.shard }}/${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }})
runs-on: ubuntu-latest
timeout-minutes: 15
needs: [setup]
if: ${{ needs.setup.outputs.can-run-tests == 'true' }}
strategy:
fail-fast: false
matrix:
- shard: [1, 2, 3, 4]
+ shard: ${{ inputs['enable-sharding'] && fromJSON(format('[{0}]', join(', ', range(1, inputs['shard-count'] + 1)))) || fromJSON('[1]') }}To confirm the exact semantics of GitHub’s range() (end-exclusive vs inclusive), please verify:
🌐 Web query:
In GitHub Actions expressions, does range(start, end) include the end value? Provide an official docs citation and a minimal matrix example using range to build [1..N].
💡 Result:
Short answer: range(start, end) is end-exclusive (it does NOT include end). Official docs: Evaluate expressions in workflows and actions (GitHub Docs). [1]
Minimal example (builds [1..N]; here N=4 so use range(1, 5)):
name: range-matrix
on: [push]
jobs:
build:
runs-on: ubuntu-latest
strategy:
matrix:
i: ${{ range(1, 5) }} # produces [1,2,3,4]
steps:
- run: echo "i = ${{ matrix.i }}"
References
[1] Evaluate expressions in workflows and actions — GitHub Docs. (docs.github.com)
Citations:
Bug: don't hardcode 4 shards — generate shard list from inputs.shard-count and gate with enable-sharding
Confirmed: GitHub expressions' range(start, end) is end‑exclusive — build [1..N] with range(1, N+1). Example: matrix: i: ${{ range(1, 5) }} produces [1,2,3,4]. (docs.github.com)
Action (file + lines): .github/workflows/e2e-tests.yml (lines 179–189) — replace the hardcoded
shard: [1, 2, 3, 4]
with a matrix generated from inputs['shard-count'], gated by inputs['enable-sharding'] and falling back to [1] when disabled. Convert the input to a number (fromJSON(inputs['shard-count'])) before adding 1 for the range end.
🤖 Prompt for AI Agents
.github/workflows/e2e-tests.yml lines 179–189: replace the hardcoded shard array
with a generated matrix that uses range(1, N+1) where N is the numeric value of
inputs['shard-count'], gating generation behind inputs['enable-sharding'] and
falling back to a single shard [1] when sharding is disabled; convert the input
string to a number with fromJSON(inputs['shard-count']) (add 1 for the range end
since range is end‑exclusive) and use that expression to populate matrix.shard.
| - name: Download build artifacts | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: build-artifacts | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Also guard artifact download when skipping build.
Without an uploaded artifact this step will fail. Mirror the condition.
- - name: Download build artifacts
- uses: actions/download-artifact@v4
+ - name: Download build artifacts
+ if: ${{ !inputs['skip-build'] }}
+ uses: actions/download-artifact@v4
with:
name: build-artifacts📝 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.
| - name: Download build artifacts | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: build-artifacts | |
| - name: Download build artifacts | |
| if: ${{ !inputs['skip-build'] }} | |
| uses: actions/download-artifact@v4 | |
| with: | |
| name: build-artifacts |
🤖 Prompt for AI Agents
.github/workflows/e2e-tests.yml lines 198-202: the "Download build artifacts"
step unconditionally runs and fails when the build/upload was skipped; mirror
the same condition used on the build/upload step by adding the identical if:
guard to this download step so it only executes when artifacts were actually
produced and uploaded.
| echo "🔐 Running authenticated E2E tests on all browsers" | ||
| echo "🔀 Running with sharding: ${{ matrix.shard }}/${{ inputs.shard-count }}" | ||
| echo "🚀 Playwright will automatically start the dev server on localhost:4200" | ||
| yarn ${{ inputs.test-command }} --reporter=list --shard=${{ matrix.shard }}/${{ inputs.shard-count }} | ||
| echo "test_exit_code=$?" >> $GITHUB_OUTPUT | ||
There was a problem hiding this comment.
Fix hyphenated input references and make shard denominator dynamic (or 1 when disabled).
The current commands use dot-notation for test-command and always divide by inputs.shard-count. Align with enable-sharding and bracket notation to avoid invalid expressions and incorrect partitioning.
- echo "🔀 Running with sharding: ${{ matrix.shard }}/${{ inputs.shard-count }}"
- echo "🚀 Playwright will automatically start the dev server on localhost:4200"
- yarn ${{ inputs.test-command }} --reporter=list --shard=${{ matrix.shard }}/${{ inputs.shard-count }}
+ echo "🔀 Running with sharding: ${{ matrix.shard }}/${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }}"
+ echo "🚀 Playwright will automatically start the dev server on ${{ inputs['base-url'] }}"
+ yarn ${{ inputs['test-command'] }} --reporter=list --shard=${{ matrix.shard }}/${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }}- echo "🔐 Running authenticated E2E tests on ${{ inputs.browser }}"
- echo "🔀 Running with sharding: ${{ matrix.shard }}/${{ inputs.shard-count }}"
- echo "🚀 Playwright will automatically start the dev server on localhost:4200"
- yarn ${{ inputs.test-command }} --project=${{ inputs.browser }} --reporter=list --shard=${{ matrix.shard }}/${{ inputs.shard-count }}
+ echo "🔐 Running authenticated E2E tests on ${{ inputs.browser }}"
+ echo "🔀 Running with sharding: ${{ matrix.shard }}/${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }}"
+ echo "🚀 Playwright will automatically start the dev server on ${{ inputs['base-url'] }}"
+ yarn ${{ inputs['test-command'] }} --project=${{ inputs.browser }} --reporter=list --shard=${{ matrix.shard }}/${{ inputs['enable-sharding'] && inputs['shard-count'] || 1 }}Also applies to: 306-310
🤖 Prompt for AI Agents
.github/workflows/e2e-tests.yml around lines 296-301 (and similarly 306-310):
the workflow uses dot-notation for hyphenated inputs and always uses
inputs.shard-count as the shard denominator; change references like
inputs.test-command and inputs.shard-count to bracket notation
inputs['test-command'] and inputs['shard-count'], and make the shard denominator
dynamic so it uses inputs['shard-count'] only when sharding is enabled
(inputs['enable-sharding'] truthy) otherwise use 1; update both occurrences
(296-301 and 306-310) so the yarn/test command and the --shard argument use
bracket notation and a conditional denominator that defaults to 1 when sharding
is disabled.
Summary
Implements comprehensive Playwright test parallelization improvements to enhance both local development experience and CI execution speed while maintaining stability.
Changes
🚀 Local Development Optimization
⚡ CI Sharding Support
📚 Documentation Enhancement
Performance Improvements
Implementation Details
Playwright Configuration
Workflow Enhancement
Files Changed
apps/lfx-pcc/playwright.config.ts- Local worker optimization.github/workflows/e2e-tests.yml- CI sharding supportdocs/architecture/testing/e2e-testing.md- Comprehensive documentationTesting
Usage Examples
Standard CI (stable):
Fast PR validation:
Benefits
✅ 4x faster CI execution when sharding enabled
✅ Improved developer experience with faster local tests
✅ Maintained stability - single worker remains default
✅ Zero breaking changes - fully backwards compatible
✅ Comprehensive documentation for team adoption
Next Steps
🤖 Generated with Claude Code