From cf093d02a7ab301791cd0c8a2060c3079177dee2 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Fri, 12 Sep 2025 23:06:28 -0700 Subject: [PATCH 1/4] ADR: PrimeVue Fork Decision (#5230) * ADR: Add PrimeVue fork decision record Adds ADR-0003 documenting the decision to fork PrimeVue as a monorepo workspace package. Key rationale includes transform coordinate system conflicts and virtual canvas scroll interference that require component-level modifications. * ADR: Reject PrimeVue fork decision - Change status from Proposed to Rejected - Document rationale: implementation complexity with dual monorepos, maintenance burden, alternative solutions available - Add specific code citations and repository links - Include alternative approach using shadcn/ui for selective replacement --- docs/adr/0004-fork-primevue-ui-library.md | 62 +++++++++++++++++++++++ docs/adr/README.md | 1 + 2 files changed, 63 insertions(+) create mode 100644 docs/adr/0004-fork-primevue-ui-library.md diff --git a/docs/adr/0004-fork-primevue-ui-library.md b/docs/adr/0004-fork-primevue-ui-library.md new file mode 100644 index 000000000..02bd18736 --- /dev/null +++ b/docs/adr/0004-fork-primevue-ui-library.md @@ -0,0 +1,62 @@ +# 4. Fork PrimeVue UI Library + +Date: 2025-08-27 + +## Status + +Rejected + +## Context + +ComfyUI's frontend requires modifications to PrimeVue components that cannot be achieved through the library's customization APIs. Two specific technical incompatibilities have been identified with the transform-based canvas architecture: + +**Screen Coordinate Hit-Testing Conflicts:** +- PrimeVue components use `getBoundingClientRect()` for screen coordinate calculations that don't account for CSS transforms +- The Slider component directly uses raw `pageX/pageY` coordinates ([lines 102-103](https://github.com/primefaces/primevue/blob/master/packages/primevue/src/slider/Slider.vue#L102-L103)) without transform-aware positioning +- This breaks interaction in transformed coordinate spaces where screen coordinates don't match logical element positions + +**Virtual Canvas Scroll Interference:** +- LiteGraph's infinite canvas uses scroll coordinates semantically for graph navigation via the `DragAndScale` coordinate system +- PrimeVue overlay components automatically trigger `scrollIntoView` behavior which interferes with this virtual positioning +- This issue is documented in [PrimeVue discussion #4270](https://github.com/orgs/primefaces/discussions/4270) where the feature request was made to disable this behavior + +**Historical Overlay Issues:** +- Previous z-index positioning conflicts required manual workarounds (commit `6d4eafb0`) where PrimeVue Dialog components needed `autoZIndex: false` and custom mask styling, later resolved by removing PrimeVue's automatic z-index management entirely + +**Minimal Update Overhead:** +- Analysis of git history shows only 2 PrimeVue version updates in 2+ years, indicating that upstream sync overhead is negligible for this project + +**Future Interaction System Requirements:** +- The ongoing canvas architecture evolution will require more granular control over component interaction and event handling as the transform-based system matures +- Predictable need for additional component modifications beyond current identified issues + +## Decision + +We will **NOT** fork PrimeVue. After evaluation, forking was determined to be unnecessarily complex and costly. + +**Rationale for Rejection:** + +- **Significant Implementation Complexity**: PrimeVue is structured as a monorepo ([primefaces/primevue](https://github.com/primefaces/primevue)) with significant code in a separate monorepo ([PrimeUIX](https://github.com/primefaces/primeuix)). Forking would require importing both repositories whole and selectively pruning or exempting components from our workspace tooling, adding substantial complexity. + +- **Alternative Solutions Available**: The modifications we identified (e.g., scroll interference issues, coordinate system conflicts) have less costly solutions that don't require maintaining a full fork. For example, coordinate issues could be addressed through event interception and synthetic event creation with scaled values. + +- **Maintenance Burden**: Ongoing maintenance and upgrades would be very painful, requiring manual conflict resolution and keeping pace with upstream changes across multiple repositories. + +- **Limited Tooling Support**: There isn't adequate tooling that provides the granularity needed to cleanly manage a PrimeVue fork within our existing infrastructure. + +## Consequences + +### Alternative Approach + +- **Use PrimeVue as External Dependency**: Continue using PrimeVue as a standard npm dependency +- **Targeted Workarounds**: Implement specific solutions for identified issues (coordinate system conflicts, scroll interference) without forking the entire library +- **Selective Component Replacement**: Use libraries like shadcn/ui to replace specific problematic PrimeVue components and adjust them to match our design system +- **Upstream Engagement**: Continue engaging with PrimeVue community for feature requests and bug reports +- **Maintain Flexibility**: Preserve ability to upgrade PrimeVue versions without fork maintenance overhead + +## Notes + +- Technical issues documented in the Context section remain valid concerns +- Solutions will be pursued through targeted fixes rather than wholesale forking +- Future re-evaluation possible if PrimeVue's architecture significantly changes or if alternative tooling becomes available +- This decision prioritizes maintainability and development velocity over maximum customization control diff --git a/docs/adr/README.md b/docs/adr/README.md index 00e50a639..5f6e5c2cf 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -13,6 +13,7 @@ An Architecture Decision Record captures an important architectural decision mad | [0001](0001-merge-litegraph-into-frontend.md) | Merge LiteGraph.js into ComfyUI Frontend | Accepted | 2025-08-05 | | [0002](0002-monorepo-conversion.md) | Restructure as a Monorepo | Accepted | 2025-08-25 | | [0003](0003-crdt-based-layout-system.md) | Centralized Layout Management with CRDT | Proposed | 2025-08-27 | +| [0004](0004-fork-primevue-ui-library.md) | Fork PrimeVue UI Library | Rejected | 2025-08-27 | ## Creating a New ADR From 0e01ca0a980f0a1bf5bbeac90972a53454711e84 Mon Sep 17 00:00:00 2001 From: Alexander Brown Date: Fri, 12 Sep 2025 23:34:39 -0700 Subject: [PATCH 2/4] a11y: Bigger click/touch target for Slider track (#5524) * a11y: Bigger click/touch target for Slider track * a11y: z-index fix and bigger thumb too. --- src/components/ui/slider/Slider.vue | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/ui/slider/Slider.vue b/src/components/ui/slider/Slider.vue index 3ce2eb3d8..a7d27299e 100644 --- a/src/components/ui/slider/Slider.vue +++ b/src/components/ui/slider/Slider.vue @@ -49,7 +49,8 @@ const forwarded = useForwardPropsEmits(delegatedProps, emits) :class=" cn( 'bg-node-stroke relative grow overflow-hidden rounded-full', - 'cursor-pointer', + 'cursor-pointer overflow-visible', + `before:absolute before:-inset-2 before:block before:bg-transparent`, 'data-[orientation=horizontal]:h-0.5 data-[orientation=horizontal]:w-full', 'data-[orientation=vertical]:h-full data-[orientation=vertical]:w-0.5' ) @@ -69,6 +70,7 @@ const forwarded = useForwardPropsEmits(delegatedProps, emits) cn( 'bg-node-component-surface-highlight ring-node-component-surface-selected block size-3.5 shrink-0 rounded-full shadow-sm transition-[color,box-shadow]', 'cursor-grab', + 'before:absolute before:-inset-1 before:block before:bg-transparent before:rounded-full', 'hover:ring-2 focus-visible:ring-2 focus-visible:outline-hidden disabled:pointer-events-none disabled:opacity-50', { 'cursor-grabbing': pressed } ) From 90b1b47dd009cb7b4a006ed9f9abff2030b084c0 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Sat, 13 Sep 2025 00:31:36 -0700 Subject: [PATCH 3/4] [test] Add Vue Node header component test (#5457) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add node header component test * [refactor] use separate const declarations instead of mutable variable in test - addresses @DrJKL's code style suggestion Replace mutable `let icon` with descriptive `const expandedIcon` and `const collapsedIcon` variables for better code clarity and immutability in the chevron icon test. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * [config] remove unnecessary vitest exclude patterns - addresses @DrJKL's configuration review Remove redundant exclude patterns from vitest config as they are already covered by vitest's default exclusions. Simplifies configuration while maintaining functionality. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * [config] remove unnecessary vitest exclude patterns - addresses @DrJKL's configuration review Remove redundant exclude patterns from vitest config as they are already covered by vitest's default exclusions. Simplifies configuration while maintaining functionality. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../vueNodes/components/NodeHeader.spec.ts | 129 ++++++++++++++++++ 1 file changed, 129 insertions(+) create mode 100644 src/renderer/extensions/vueNodes/components/NodeHeader.spec.ts diff --git a/src/renderer/extensions/vueNodes/components/NodeHeader.spec.ts b/src/renderer/extensions/vueNodes/components/NodeHeader.spec.ts new file mode 100644 index 000000000..240a51071 --- /dev/null +++ b/src/renderer/extensions/vueNodes/components/NodeHeader.spec.ts @@ -0,0 +1,129 @@ +import { mount } from '@vue/test-utils' +import { createPinia } from 'pinia' +import PrimeVue from 'primevue/config' +import InputText from 'primevue/inputtext' +import { describe, expect, it } from 'vitest' +import { createI18n } from 'vue-i18n' + +import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' +import enMessages from '@/locales/en/main.json' + +import NodeHeader from './NodeHeader.vue' + +const makeNodeData = (overrides: Partial = {}): VueNodeData => ({ + id: '1', + title: 'KSampler', + type: 'KSampler', + mode: 0, + selected: false, + executing: false, + widgets: [], + inputs: [], + outputs: [], + flags: { collapsed: false }, + ...overrides +}) + +const mountHeader = ( + props?: Partial['$props']> +) => { + const i18n = createI18n({ + legacy: false, + locale: 'en', + messages: { en: enMessages } + }) + return mount(NodeHeader, { + global: { + plugins: [PrimeVue, i18n, createPinia()], + components: { InputText } + }, + props: { + nodeData: makeNodeData(), + readonly: false, + collapsed: false, + ...props + } + }) +} + +describe('NodeHeader.vue', () => { + it('emits collapse when collapse button is clicked', async () => { + const wrapper = mountHeader() + const btn = wrapper.get('[data-testid="node-collapse-button"]') + await btn.trigger('click') + expect(wrapper.emitted('collapse')).toBeTruthy() + }) + + it('shows the current node title and updates when prop changes', async () => { + const wrapper = mountHeader({ + nodeData: makeNodeData({ title: 'Original' }) + }) + // Title visible via EditableText in view mode + expect(wrapper.get('[data-testid="node-title"]').text()).toContain( + 'Original' + ) + + // Update prop title; should sync displayTitle + await wrapper.setProps({ nodeData: makeNodeData({ title: 'Updated' }) }) + expect(wrapper.get('[data-testid="node-title"]').text()).toContain( + 'Updated' + ) + }) + + it('allows renaming via double click and emits update:title on confirm', async () => { + const wrapper = mountHeader({ nodeData: makeNodeData({ title: 'Start' }) }) + + // Enter edit mode + await wrapper.get('[data-testid="node-header-1"]').trigger('dblclick') + + // Edit and confirm (EditableText uses blur or enter to emit) + const input = wrapper.get('[data-testid="node-title-input"]') + await input.setValue('My Custom Sampler') + await input.trigger('keyup.enter') + await input.trigger('blur') + + // NodeHeader should emit update:title with trimmed value + const e = wrapper.emitted('update:title') + expect(e).toBeTruthy() + expect(e?.[0]).toEqual(['My Custom Sampler']) + }) + + it('cancels rename on escape and keeps previous title', async () => { + const wrapper = mountHeader({ nodeData: makeNodeData({ title: 'KeepMe' }) }) + + await wrapper.get('[data-testid="node-header-1"]').trigger('dblclick') + const input = wrapper.get('[data-testid="node-title-input"]') + await input.setValue('Should Not Save') + await input.trigger('keyup.escape') + + // Should not emit update:title + expect(wrapper.emitted('update:title')).toBeFalsy() + + // Title remains the original + expect(wrapper.get('[data-testid="node-title"]').text()).toContain('KeepMe') + }) + + it('honors readonly: hides collapse button and prevents editing', async () => { + const wrapper = mountHeader({ readonly: true }) + + // Collapse button should be hidden + const btn = wrapper.find('[data-testid="node-collapse-button"]') + expect(btn.exists()).toBe(true) + // v-show hides via display:none + expect((btn.element as HTMLButtonElement).style.display).toBe('none') + // In unit test, presence is fine; simulate double click should not create input + await wrapper.get('[data-testid="node-header-1"]').trigger('dblclick') + const input = wrapper.find('[data-testid="node-title-input"]') + expect(input.exists()).toBe(false) + }) + + it('renders correct chevron icon based on collapsed prop', async () => { + const wrapper = mountHeader({ collapsed: false }) + const expandedIcon = wrapper.get('i') + expect(expandedIcon.classes()).toContain('pi-chevron-down') + + await wrapper.setProps({ collapsed: true }) + const collapsedIcon = wrapper.get('i') + expect(collapsedIcon.classes()).toContain('pi-chevron-right') + }) +}) From 48d01745cd21b0fcf856c8fd1f8eaa6548e74345 Mon Sep 17 00:00:00 2001 From: snomiao Date: Sat, 13 Sep 2025 17:18:18 +0900 Subject: [PATCH 4/4] feat: add test count display to Playwright PR comments (#5458) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add test count display to Playwright PR comments - Add extract-playwright-counts.mjs script to parse test results from Playwright reports - Update pr-playwright-deploy-and-comment.sh to extract and display test counts - Show overall summary with passed/failed/flaky/skipped counts - Display per-browser test counts inline with report links - Use dynamic status icons based on test results (✅/❌/⚠️) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * feat: include skipped test count in per-browser display - Add skipped test extraction for individual browser reports - Update per-browser display format to show all four counts: (✅ passed / ❌ failed / ⚠️ flaky / ⏭️ skipped) - Provides complete test result visibility at a glance 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * fix: improve test count extraction reliability in CI - Use absolute paths for script and report directories - Add debug logging to help diagnose extraction issues - Move counts display after View Report link as requested - Format: [View Report](url) • ✅ passed / ❌ failed / ⚠️ flaky / ⏭️ skipped 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * fix: generate JSON reports alongside HTML for test count extraction - Add JSON reporter to Playwright test runs - Generate report.json alongside HTML reports - Store JSON report in playwright-report directory - This enables accurate test count extraction from CI artifacts The HTML reports alone don't contain easily extractable test statistics as they use a React app with dynamically loaded data. JSON reports provide direct access to test counts. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * fix: correct JSON reporter syntax for Playwright tests - Use proper syntax for JSON reporter with outputFile option - Run separate commands for HTML and JSON report merging - Specify output path directly in reporter configuration - Ensures report.json is created in playwright-report directory This fixes the "No such file or directory" error when trying to move report.json file, as it wasn't being created in the first place. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * Revert "fix: correct JSON reporter syntax for Playwright tests" This reverts commit 605d7cc1e2c33a2f40eaf27ff023b0b8ac4ed87d. * fix: use correct Playwright reporter syntax with comma-separated list - Use --reporter=html,json syntax (comma-separated, not space) - Move test-results.json to playwright-report/report.json after generation - Remove incorrect PLAYWRIGHT_JSON_OUTPUT_NAME env variable - Add || true to prevent failure if JSON file doesn't exist The JSON reporter outputs to test-results.json by default when using the comma-separated reporter list syntax. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * fix: improve test count extraction reliability in CI - Use separate --reporter flags for list, html, and json - Set PLAYWRIGHT_JSON_OUTPUT_NAME env var to specify JSON output path - Run HTML and JSON report generation separately for merged reports - Ensures report.json is created in playwright-report directory The combined reporter syntax wasn't creating the JSON file properly. Using separate reporter flags with env var ensures JSON is generated. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * Update scripts/cicd/pr-playwright-deploy-and-comment.sh Co-authored-by: Alexander Brown * refactor: convert extraction script to TypeScript and use tsx - Convert extract-playwright-counts.mjs to TypeScript (.ts) - Add proper TypeScript types for better type safety - Use tsx for execution instead of node - Auto-install tsx in CI if not available - Better alignment with the TypeScript codebase This provides better type safety and consistency with the rest of the codebase while maintaining the same functionality. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude * chore(pr-playwright-deploy-and-comment.sh): move tsx installation check to the beginning of the script for better organization and efficiency * [auto-fix] Apply ESLint and Prettier fixes --------- Co-authored-by: Claude Co-authored-by: Alexander Brown Co-authored-by: GitHub Action --- .github/workflows/test-ui.yaml | 15 +- scripts/cicd/extract-playwright-counts.ts | 183 ++++++++++++++++++ .../cicd/pr-playwright-deploy-and-comment.sh | 152 ++++++++++++++- 3 files changed, 340 insertions(+), 10 deletions(-) create mode 100755 scripts/cicd/extract-playwright-counts.ts diff --git a/.github/workflows/test-ui.yaml b/.github/workflows/test-ui.yaml index f8f6cf955..eaaaefee0 100644 --- a/.github/workflows/test-ui.yaml +++ b/.github/workflows/test-ui.yaml @@ -229,7 +229,13 @@ jobs: - name: Run Playwright tests (${{ matrix.browser }}) id: playwright - run: npx playwright test --project=${{ matrix.browser }} --reporter=html + run: | + # Run tests with both HTML and JSON reporters + PLAYWRIGHT_JSON_OUTPUT_NAME=playwright-report/report.json \ + npx playwright test --project=${{ matrix.browser }} \ + --reporter=list \ + --reporter=html \ + --reporter=json working-directory: ComfyUI_frontend - uses: actions/upload-artifact@v4 @@ -275,7 +281,12 @@ jobs: merge-multiple: true - name: Merge into HTML Report - run: npx playwright merge-reports --reporter html ./all-blob-reports + run: | + # Generate HTML report + npx playwright merge-reports --reporter=html ./all-blob-reports + # Generate JSON report separately with explicit output path + PLAYWRIGHT_JSON_OUTPUT_NAME=playwright-report/report.json \ + npx playwright merge-reports --reporter=json ./all-blob-reports working-directory: ComfyUI_frontend - name: Upload HTML report diff --git a/scripts/cicd/extract-playwright-counts.ts b/scripts/cicd/extract-playwright-counts.ts new file mode 100755 index 000000000..ff6f44db3 --- /dev/null +++ b/scripts/cicd/extract-playwright-counts.ts @@ -0,0 +1,183 @@ +#!/usr/bin/env tsx +import fs from 'fs' +import path from 'path' + +interface TestStats { + expected?: number + unexpected?: number + flaky?: number + skipped?: number + finished?: number +} + +interface ReportData { + stats?: TestStats +} + +interface TestCounts { + passed: number + failed: number + flaky: number + skipped: number + total: number +} + +/** + * Extract test counts from Playwright HTML report + * @param reportDir - Path to the playwright-report directory + * @returns Test counts { passed, failed, flaky, skipped, total } + */ +function extractTestCounts(reportDir: string): TestCounts { + const counts: TestCounts = { + passed: 0, + failed: 0, + flaky: 0, + skipped: 0, + total: 0 + } + + try { + // First, try to find report.json which Playwright generates with JSON reporter + const jsonReportFile = path.join(reportDir, 'report.json') + if (fs.existsSync(jsonReportFile)) { + const reportJson: ReportData = JSON.parse( + fs.readFileSync(jsonReportFile, 'utf-8') + ) + if (reportJson.stats) { + const stats = reportJson.stats + counts.total = stats.expected || 0 + counts.passed = + (stats.expected || 0) - + (stats.unexpected || 0) - + (stats.flaky || 0) - + (stats.skipped || 0) + counts.failed = stats.unexpected || 0 + counts.flaky = stats.flaky || 0 + counts.skipped = stats.skipped || 0 + return counts + } + } + + // Try index.html - Playwright HTML report embeds data in a script tag + const indexFile = path.join(reportDir, 'index.html') + if (fs.existsSync(indexFile)) { + const content = fs.readFileSync(indexFile, 'utf-8') + + // Look for the embedded report data in various formats + // Format 1: window.playwrightReportBase64 + let dataMatch = content.match( + /window\.playwrightReportBase64\s*=\s*["']([^"']+)["']/ + ) + if (dataMatch) { + try { + const decodedData = Buffer.from(dataMatch[1], 'base64').toString( + 'utf-8' + ) + const reportData: ReportData = JSON.parse(decodedData) + + if (reportData.stats) { + const stats = reportData.stats + counts.total = stats.expected || 0 + counts.passed = + (stats.expected || 0) - + (stats.unexpected || 0) - + (stats.flaky || 0) - + (stats.skipped || 0) + counts.failed = stats.unexpected || 0 + counts.flaky = stats.flaky || 0 + counts.skipped = stats.skipped || 0 + return counts + } + } catch (e) { + // Continue to try other formats + } + } + + // Format 2: window.playwrightReport + dataMatch = content.match(/window\.playwrightReport\s*=\s*({[\s\S]*?});/) + if (dataMatch) { + try { + // Use Function constructor instead of eval for safety + const reportData = new Function( + 'return ' + dataMatch[1] + )() as ReportData + + if (reportData.stats) { + const stats = reportData.stats + counts.total = stats.expected || 0 + counts.passed = + (stats.expected || 0) - + (stats.unexpected || 0) - + (stats.flaky || 0) - + (stats.skipped || 0) + counts.failed = stats.unexpected || 0 + counts.flaky = stats.flaky || 0 + counts.skipped = stats.skipped || 0 + return counts + } + } catch (e) { + // Continue to try other formats + } + } + + // Format 3: Look for stats in the HTML content directly + // Playwright sometimes renders stats in the UI + const statsMatch = content.match( + /(\d+)\s+passed[^0-9]*(\d+)\s+failed[^0-9]*(\d+)\s+flaky[^0-9]*(\d+)\s+skipped/i + ) + if (statsMatch) { + counts.passed = parseInt(statsMatch[1]) || 0 + counts.failed = parseInt(statsMatch[2]) || 0 + counts.flaky = parseInt(statsMatch[3]) || 0 + counts.skipped = parseInt(statsMatch[4]) || 0 + counts.total = + counts.passed + counts.failed + counts.flaky + counts.skipped + return counts + } + + // Format 4: Try to extract from summary text patterns + const passedMatch = content.match(/(\d+)\s+(?:tests?|specs?)\s+passed/i) + const failedMatch = content.match(/(\d+)\s+(?:tests?|specs?)\s+failed/i) + const flakyMatch = content.match(/(\d+)\s+(?:tests?|specs?)\s+flaky/i) + const skippedMatch = content.match(/(\d+)\s+(?:tests?|specs?)\s+skipped/i) + const totalMatch = content.match( + /(\d+)\s+(?:tests?|specs?)\s+(?:total|ran)/i + ) + + if (passedMatch) counts.passed = parseInt(passedMatch[1]) || 0 + if (failedMatch) counts.failed = parseInt(failedMatch[1]) || 0 + if (flakyMatch) counts.flaky = parseInt(flakyMatch[1]) || 0 + if (skippedMatch) counts.skipped = parseInt(skippedMatch[1]) || 0 + if (totalMatch) { + counts.total = parseInt(totalMatch[1]) || 0 + } else if ( + counts.passed || + counts.failed || + counts.flaky || + counts.skipped + ) { + counts.total = + counts.passed + counts.failed + counts.flaky + counts.skipped + } + } + } catch (error) { + console.error(`Error reading report from ${reportDir}:`, error) + } + + return counts +} + +// Main execution +const reportDir = process.argv[2] + +if (!reportDir) { + console.error('Usage: extract-playwright-counts.ts ') + process.exit(1) +} + +const counts = extractTestCounts(reportDir) + +// Output as JSON for easy parsing in shell script +console.log(JSON.stringify(counts)) + +export { extractTestCounts } diff --git a/scripts/cicd/pr-playwright-deploy-and-comment.sh b/scripts/cicd/pr-playwright-deploy-and-comment.sh index 767a7f514..aeab37c8e 100755 --- a/scripts/cicd/pr-playwright-deploy-and-comment.sh +++ b/scripts/cicd/pr-playwright-deploy-and-comment.sh @@ -58,6 +58,12 @@ if ! command -v wrangler > /dev/null 2>&1; then } fi +# Check if tsx is available, install if not +if ! command -v tsx > /dev/null 2>&1; then + echo "Installing tsx..." >&2 + npm install -g tsx >&2 || echo "Failed to install tsx" >&2 +fi + # Deploy a single browser report, WARN: ensure inputs are sanitized before calling this function deploy_report() { dir="$1" @@ -159,12 +165,16 @@ else echo "Available reports:" ls -la reports/ 2>/dev/null || echo "Reports directory not found" - # Deploy all reports in parallel and collect URLs + # Deploy all reports in parallel and collect URLs + test counts temp_dir=$(mktemp -d) pids="" i=0 - # Start parallel deployments + # Store current working directory for absolute paths + SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" + BASE_DIR="$(pwd)" + + # Start parallel deployments and count extractions for browser in $BROWSERS; do if [ -d "reports/playwright-report-$browser" ]; then echo "Found report for $browser, deploying in parallel..." @@ -172,11 +182,26 @@ else url=$(deploy_report "reports/playwright-report-$browser" "$browser" "$cloudflare_branch") echo "$url" > "$temp_dir/$i.url" echo "Deployment result for $browser: $url" + + # Extract test counts using tsx (TypeScript executor) + EXTRACT_SCRIPT="$SCRIPT_DIR/extract-playwright-counts.ts" + REPORT_DIR="$BASE_DIR/reports/playwright-report-$browser" + + if command -v tsx > /dev/null 2>&1 && [ -f "$EXTRACT_SCRIPT" ]; then + echo "Extracting counts from $REPORT_DIR using $EXTRACT_SCRIPT" >&2 + counts=$(tsx "$EXTRACT_SCRIPT" "$REPORT_DIR" 2>&1 || echo '{}') + echo "Extracted counts for $browser: $counts" >&2 + echo "$counts" > "$temp_dir/$i.counts" + else + echo "Script not found or tsx not available: $EXTRACT_SCRIPT" >&2 + echo '{}' > "$temp_dir/$i.counts" + fi ) & pids="$pids $!" else echo "Report not found for $browser at reports/playwright-report-$browser" echo "failed" > "$temp_dir/$i.url" + echo '{}' > "$temp_dir/$i.counts" fi i=$((i + 1)) done @@ -186,8 +211,9 @@ else wait $pid done - # Collect URLs in order + # Collect URLs and counts in order urls="" + all_counts="" i=0 for browser in $BROWSERS; do if [ -f "$temp_dir/$i.url" ]; then @@ -200,37 +226,147 @@ else else urls="$urls $url" fi + + if [ -f "$temp_dir/$i.counts" ]; then + counts=$(cat "$temp_dir/$i.counts") + echo "Read counts for $browser from $temp_dir/$i.counts: $counts" >&2 + else + counts="{}" + echo "No counts file found for $browser at $temp_dir/$i.counts" >&2 + fi + if [ -z "$all_counts" ]; then + all_counts="$counts" + else + all_counts="$all_counts|$counts" + fi + i=$((i + 1)) done # Clean up temp directory rm -rf "$temp_dir" + # Calculate total test counts across all browsers + total_passed=0 + total_failed=0 + total_flaky=0 + total_skipped=0 + total_tests=0 + + # Parse counts and calculate totals + IFS='|' + set -- $all_counts + for counts_json; do + if [ "$counts_json" != "{}" ] && [ -n "$counts_json" ]; then + # Parse JSON counts using simple grep/sed if jq is not available + if command -v jq > /dev/null 2>&1; then + passed=$(echo "$counts_json" | jq -r '.passed // 0') + failed=$(echo "$counts_json" | jq -r '.failed // 0') + flaky=$(echo "$counts_json" | jq -r '.flaky // 0') + skipped=$(echo "$counts_json" | jq -r '.skipped // 0') + total=$(echo "$counts_json" | jq -r '.total // 0') + else + # Fallback parsing without jq + passed=$(echo "$counts_json" | sed -n 's/.*"passed":\([0-9]*\).*/\1/p') + failed=$(echo "$counts_json" | sed -n 's/.*"failed":\([0-9]*\).*/\1/p') + flaky=$(echo "$counts_json" | sed -n 's/.*"flaky":\([0-9]*\).*/\1/p') + skipped=$(echo "$counts_json" | sed -n 's/.*"skipped":\([0-9]*\).*/\1/p') + total=$(echo "$counts_json" | sed -n 's/.*"total":\([0-9]*\).*/\1/p') + fi + + total_passed=$((total_passed + ${passed:-0})) + total_failed=$((total_failed + ${failed:-0})) + total_flaky=$((total_flaky + ${flaky:-0})) + total_skipped=$((total_skipped + ${skipped:-0})) + total_tests=$((total_tests + ${total:-0})) + fi + done + unset IFS + + # Determine overall status + if [ $total_failed -gt 0 ]; then + status_icon="❌" + status_text="Some tests failed" + elif [ $total_flaky -gt 0 ]; then + status_icon="⚠️" + status_text="Tests passed with flaky tests" + elif [ $total_tests -gt 0 ]; then + status_icon="✅" + status_text="All tests passed!" + else + status_icon="🕵🏻" + status_text="No test results found" + fi + # Generate completion comment comment="$COMMENT_MARKER ## 🎭 Playwright Test Results -✅ **Tests completed successfully!** +$status_icon **$status_text** -⏰ Completed at: $(date -u '+%m/%d/%Y, %I:%M:%S %p') UTC +⏰ Completed at: $(date -u '+%m/%d/%Y, %I:%M:%S %p') UTC" + + # Add summary counts if we have test data + if [ $total_tests -gt 0 ]; then + comment="$comment + +### 📈 Summary +- **Total Tests:** $total_tests +- **Passed:** $total_passed ✅ +- **Failed:** $total_failed $([ $total_failed -gt 0 ] && echo '❌' || echo '') +- **Flaky:** $total_flaky $([ $total_flaky -gt 0 ] && echo '⚠️' || echo '') +- **Skipped:** $total_skipped $([ $total_skipped -gt 0 ] && echo '⏭️' || echo '')" + fi + + comment="$comment ### 📊 Test Reports by Browser" - # Add browser results + # Add browser results with individual counts i=0 - for browser in $BROWSERS; do + IFS='|' + set -- $all_counts + for counts_json; do + # Get browser name + browser=$(echo "$BROWSERS" | cut -d' ' -f$((i + 1))) # Get URL at position i url=$(echo "$urls" | cut -d' ' -f$((i + 1))) if [ "$url" != "failed" ] && [ -n "$url" ]; then + # Parse individual browser counts + if [ "$counts_json" != "{}" ] && [ -n "$counts_json" ]; then + if command -v jq > /dev/null 2>&1; then + b_passed=$(echo "$counts_json" | jq -r '.passed // 0') + b_failed=$(echo "$counts_json" | jq -r '.failed // 0') + b_flaky=$(echo "$counts_json" | jq -r '.flaky // 0') + b_skipped=$(echo "$counts_json" | jq -r '.skipped // 0') + b_total=$(echo "$counts_json" | jq -r '.total // 0') + else + b_passed=$(echo "$counts_json" | sed -n 's/.*"passed":\([0-9]*\).*/\1/p') + b_failed=$(echo "$counts_json" | sed -n 's/.*"failed":\([0-9]*\).*/\1/p') + b_flaky=$(echo "$counts_json" | sed -n 's/.*"flaky":\([0-9]*\).*/\1/p') + b_skipped=$(echo "$counts_json" | sed -n 's/.*"skipped":\([0-9]*\).*/\1/p') + b_total=$(echo "$counts_json" | sed -n 's/.*"total":\([0-9]*\).*/\1/p') + fi + + if [ -n "$b_total" ] && [ "$b_total" != "0" ]; then + counts_str=" • ✅ $b_passed / ❌ $b_failed / ⚠️ $b_flaky / ⏭️ $b_skipped" + else + counts_str="" + fi + else + counts_str="" + fi + comment="$comment -- ✅ **${browser}**: [View Report](${url})" +- ✅ **${browser}**: [View Report](${url})${counts_str}" else comment="$comment - ❌ **${browser}**: Deployment failed" fi i=$((i + 1)) done + unset IFS comment="$comment