Compare commits

...

4 Commits

Author SHA1 Message Date
ComfyPRBot
587beda065 test: intentionally break color palette test
This is an experiment to observe how GitHub CI/CD test result
comments are formatted when a test fails.

The test now expects a non-existent screenshot file which will
cause the test to fail in CI.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-22 20:38:42 +00:00
ComfyPRBot
81401c61f3 fix: resolve shellcheck warnings in playwright deployment script
- Replace sed with bash parameter expansion for /index.html removal (SC2001)
- Remove unused trace_link variable (SC2034)
2025-12-22 20:22:58 +00:00
snomiao
3af88a8b93 feat: add failure type categorization to Playwright PR comments
Add categorization of test failures by type (screenshot assertions,
expectation failures, timeouts, and other) to help developers quickly
understand what types of issues are occurring.

Changes:
- Add categorizeFailureType() function to detect failure types from error messages
- Track failure type counts in TestCounts interface
- Display "Failure Breakdown" section in PR comments when tests fail
- Show counts for: screenshot, expectation, timeout, and other failures

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-22 19:49:27 +00:00
snomiao
8c0c819471 Improve Playwright PR comment format
This commit improves the Playwright test results comments on PRs to be more concise and actionable:

1. Enhanced extract-playwright-counts.ts:
   - Added interfaces for test results, locations, and attachments
   - Implemented extractFailingTests() to recursively extract failing test details
   - Now extracts test names, file paths, line numbers, errors, and trace paths
   - Returns failingTests array in the JSON output

2. Updated pr-playwright-deploy-and-comment.sh:
   - Made summary more concise (single line with counts)
   - Added "Failed Tests" section showing each failing test with:
     * Direct link to test source code on GitHub
     * Browser configuration where it failed
     * Direct link to Playwright trace viewer
   - Moved browser-specific reports into a collapsible <details> section
   - Reduced overall verbosity while keeping important info upfront

The new format makes it much easier for developers to:
- Quickly see which tests failed
- Jump directly to the failing test code
- Access the Playwright trace viewer (which few people knew existed)

Implements: https://www.notion.so/Implement-Improve-Playwright-PR-comment-format-2d16d73d36508129979ad74391bee39d

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-12-22 19:30:08 +00:00
3 changed files with 305 additions and 26 deletions

View File

@@ -171,7 +171,8 @@ test.describe('Color Palette', () => {
await comfyPage.setSetting('Comfy.ColorPalette', 'dark')
await comfyPage.nextFrame()
await expect(comfyPage.canvas).toHaveScreenshot('default-color-palette.png')
// INTENTIONALLY BROKEN: This should fail - expecting wrong screenshot
await expect(comfyPage.canvas).toHaveScreenshot('WRONG-SCREENSHOT-NAME.png')
})
test('Can add custom color palette', async ({ comfyPage }) => {

View File

@@ -10,8 +10,56 @@ interface TestStats {
finished?: number
}
interface TestLocation {
file: string
line: number
column: number
}
interface TestAttachment {
name: string
path?: string
contentType: string
}
interface TestResult {
status: string
duration: number
errors?: Array<{ message?: string; stack?: string }>
attachments?: TestAttachment[]
}
interface Test {
title: string
location?: TestLocation
results?: TestResult[]
}
interface Suite {
title: string
suites?: Suite[]
tests?: Test[]
}
interface ReportData {
stats?: TestStats
suites?: Suite[]
}
interface FailingTest {
name: string
filePath: string
line: number
error: string
tracePath?: string
failureType?: 'screenshot' | 'expectation' | 'timeout' | 'other'
}
interface FailureTypeCounts {
screenshot: number
expectation: number
timeout: number
other: number
}
interface TestCounts {
@@ -20,12 +68,109 @@ interface TestCounts {
flaky: number
skipped: number
total: number
failingTests?: FailingTest[]
failureTypes?: FailureTypeCounts
}
/**
* Categorize the failure type based on error message
*/
function categorizeFailureType(
error: string,
status: string
): 'screenshot' | 'expectation' | 'timeout' | 'other' {
if (status === 'timedOut') {
return 'timeout'
}
const errorLower = error.toLowerCase()
// Screenshot-related errors
if (
errorLower.includes('screenshot') ||
errorLower.includes('snapshot') ||
errorLower.includes('toHaveScreenshot') ||
errorLower.includes('image comparison') ||
errorLower.includes('pixel') ||
errorLower.includes('visual')
) {
return 'screenshot'
}
// Expectation errors
if (
errorLower.includes('expect') ||
errorLower.includes('assertion') ||
errorLower.includes('toEqual') ||
errorLower.includes('toBe') ||
errorLower.includes('toContain') ||
errorLower.includes('toHave') ||
errorLower.includes('toMatch')
) {
return 'expectation'
}
return 'other'
}
/**
* Recursively extract failing tests from suite structure
*/
function extractFailingTests(
suite: Suite,
failingTests: FailingTest[],
reportDir: string
): void {
// Process tests in this suite
if (suite.tests) {
for (const test of suite.tests) {
if (!test.results) continue
for (const result of test.results) {
if (result.status === 'failed' || result.status === 'timedOut') {
const error =
result.errors?.[0]?.message ||
result.errors?.[0]?.stack ||
'Test failed'
// Find trace attachment
let tracePath: string | undefined
if (result.attachments) {
const traceAttachment = result.attachments.find(
(att) => att.name === 'trace' || att.contentType === 'application/zip'
)
if (traceAttachment?.path) {
tracePath = traceAttachment.path
}
}
const failureType = categorizeFailureType(error, result.status)
failingTests.push({
name: test.title,
filePath: test.location?.file || 'unknown',
line: test.location?.line || 0,
error: error.split('\n')[0], // First line of error
tracePath,
failureType
})
}
}
}
}
// Recursively process nested suites
if (suite.suites) {
for (const nestedSuite of suite.suites) {
extractFailingTests(nestedSuite, failingTests, reportDir)
}
}
}
/**
* Extract test counts from Playwright HTML report
* @param reportDir - Path to the playwright-report directory
* @returns Test counts { passed, failed, flaky, skipped, total }
* @returns Test counts { passed, failed, flaky, skipped, total, failingTests }
*/
function extractTestCounts(reportDir: string): TestCounts {
const counts: TestCounts = {
@@ -33,7 +178,14 @@ function extractTestCounts(reportDir: string): TestCounts {
failed: 0,
flaky: 0,
skipped: 0,
total: 0
total: 0,
failingTests: [],
failureTypes: {
screenshot: 0,
expectation: 0,
timeout: 0,
other: 0
}
}
try {
@@ -54,6 +206,22 @@ function extractTestCounts(reportDir: string): TestCounts {
counts.failed = stats.unexpected || 0
counts.flaky = stats.flaky || 0
counts.skipped = stats.skipped || 0
// Extract failing test details
if (reportJson.suites) {
for (const suite of reportJson.suites) {
extractFailingTests(suite, counts.failingTests, reportDir)
}
}
// Count failure types
if (counts.failingTests) {
for (const test of counts.failingTests) {
const type = test.failureType || 'other'
counts.failureTypes![type]++
}
}
return counts
}
}
@@ -86,6 +254,22 @@ function extractTestCounts(reportDir: string): TestCounts {
counts.failed = stats.unexpected || 0
counts.flaky = stats.flaky || 0
counts.skipped = stats.skipped || 0
// Extract failing test details
if (reportData.suites) {
for (const suite of reportData.suites) {
extractFailingTests(suite, counts.failingTests!, reportDir)
}
}
// Count failure types
if (counts.failingTests) {
for (const test of counts.failingTests) {
const type = test.failureType || 'other'
counts.failureTypes![type]++
}
}
return counts
}
} catch (e) {
@@ -113,6 +297,22 @@ function extractTestCounts(reportDir: string): TestCounts {
counts.failed = stats.unexpected || 0
counts.flaky = stats.flaky || 0
counts.skipped = stats.skipped || 0
// Extract failing test details
if (reportData.suites) {
for (const suite of reportData.suites) {
extractFailingTests(suite, counts.failingTests!, reportDir)
}
}
// Count failure types
if (counts.failingTests) {
for (const test of counts.failingTests) {
const type = test.failureType || 'other'
counts.failureTypes![type]++
}
}
return counts
}
} catch (e) {

View File

@@ -252,6 +252,10 @@ else
total_flaky=0
total_skipped=0
total_tests=0
total_screenshot_failures=0
total_expectation_failures=0
total_timeout_failures=0
total_other_failures=0
# Parse counts and calculate totals
IFS='|' read -r -a counts_array <<< "$all_counts"
@@ -265,6 +269,10 @@ else
flaky=$(echo "$counts_json" | jq -r '.flaky // 0')
skipped=$(echo "$counts_json" | jq -r '.skipped // 0')
total=$(echo "$counts_json" | jq -r '.total // 0')
screenshot=$(echo "$counts_json" | jq -r '.failureTypes.screenshot // 0')
expectation=$(echo "$counts_json" | jq -r '.failureTypes.expectation // 0')
timeout=$(echo "$counts_json" | jq -r '.failureTypes.timeout // 0')
other=$(echo "$counts_json" | jq -r '.failureTypes.other // 0')
else
# Fallback parsing without jq
passed=$(echo "$counts_json" | sed -n 's/.*"passed":\([0-9]*\).*/\1/p')
@@ -272,13 +280,21 @@ else
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')
screenshot=0
expectation=0
timeout=0
other=0
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}))
total_screenshot_failures=$((total_screenshot_failures + ${screenshot:-0}))
total_expectation_failures=$((total_expectation_failures + ${expectation:-0}))
total_timeout_failures=$((total_timeout_failures + ${timeout:-0}))
total_other_failures=$((total_other_failures + ${other:-0}))
fi
done
unset IFS
@@ -302,35 +318,98 @@ else
comment="$COMMENT_MARKER
## 🎭 Playwright Test Results
$status_icon **$status_text**
⏰ Completed at: $(date -u '+%m/%d/%Y, %I:%M:%S %p') UTC"
$status_icon **$status_text** • ⏰ $(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 '')"
**$total_passed** ✅ • **$total_failed** $([ $total_failed -gt 0 ] && echo '❌' || echo '✅') • **$total_flaky** $([ $total_flaky -gt 0 ] && echo '⚠️' || echo '✅') • **$total_skipped** ⏭️ • **$total_tests** total"
# Add failure breakdown if there are failures
if [ $total_failed -gt 0 ]; then
comment="$comment
**Failure Breakdown:** 📸 $total_screenshot_failures screenshot • ✓ $total_expectation_failures expectation • ⏱️ $total_timeout_failures timeout • ❓ $total_other_failures other"
fi
fi
comment="$comment
### 📊 Test Reports by Browser"
# Add browser results with individual counts
# Collect all failing tests across browsers
all_failing_tests=""
i=0
IFS=' ' read -r -a browser_array <<< "$BROWSERS"
for counts_json in "${counts_array[@]}"; do
[ -z "$counts_json" ] && { i=$((i + 1)); continue; }
browser="${browser_array[$i]:-}"
if [ "$counts_json" != "{}" ] && [ -n "$counts_json" ]; then
if command -v jq > /dev/null 2>&1; then
failing_tests=$(echo "$counts_json" | jq -r '.failingTests // [] | .[]' 2>/dev/null || echo "")
if [ -n "$failing_tests" ]; then
# Process each failing test
while IFS= read -r test_json; do
[ -z "$test_json" ] && continue
test_name=$(echo "$test_json" | jq -r '.name // "Unknown test"')
test_file=$(echo "$test_json" | jq -r '.filePath // "unknown"')
test_line=$(echo "$test_json" | jq -r '.line // 0')
trace_path=$(echo "$test_json" | jq -r '.tracePath // ""')
# Build GitHub source link (assumes ComfyUI_frontend repo)
source_link="https://github.com/$GITHUB_REPOSITORY/blob/$BRANCH_NAME/$test_file#L$test_line"
# Build trace viewer link if trace exists
if [ -n "$trace_path" ] && [ "$trace_path" != "null" ]; then
# Extract trace filename from path
trace_file=$(basename "$trace_path")
url="${url_array[$i]:-}"
if [ "$url" != "failed" ] && [ -n "$url" ]; then
base_url="${url%/index.html}"
trace_viewer_link="${base_url}/trace/?trace=${base_url}/data/${trace_file}"
fi
fi
# Format failing test entry
if [ -n "$all_failing_tests" ]; then
all_failing_tests="$all_failing_tests
"
fi
if [ -n "$trace_viewer_link" ]; then
all_failing_tests="${all_failing_tests}- **[$test_name]($source_link)** \`$browser\` • [View trace]($trace_viewer_link)"
else
all_failing_tests="${all_failing_tests}- **[$test_name]($source_link)** \`$browser\`"
fi
done < <(echo "$counts_json" | jq -c '.failingTests[]?' 2>/dev/null || echo "")
fi
fi
fi
i=$((i + 1))
done
unset IFS
# Add failing tests section if there are failures
if [ $total_failed -gt 0 ] && [ -n "$all_failing_tests" ]; then
comment="$comment
### ❌ Failed Tests
$all_failing_tests"
fi
comment="$comment
<details>
<summary>📊 Test Reports by Browser</summary>
"
# Add browser results with individual counts
i=0
IFS=' ' read -r -a url_array <<< "$urls"
for counts_json in "${counts_array[@]}"; do
[ -z "$counts_json" ] && { i=$((i + 1)); continue; }
browser="${browser_array[$i]:-}"
url="${url_array[$i]:-}"
if [ "$url" != "failed" ] && [ -n "$url" ]; then
# Parse individual browser counts
if [ "$counts_json" != "{}" ] && [ -n "$counts_json" ]; then
@@ -347,7 +426,7 @@ $status_icon **$status_text**
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
@@ -356,21 +435,20 @@ $status_icon **$status_text**
else
counts_str=""
fi
comment="$comment
- **${browser}**: [View Report](${url})${counts_str}"
- **${browser}**: [View Report](${url})${counts_str}"
else
comment="$comment
- **${browser}**: Deployment failed"
- **${browser}**: Deployment failed"
fi
i=$((i + 1))
done
unset IFS
comment="$comment
---
🎉 Click on the links above to view detailed test results for each browser configuration."
</details>"
post_comment "$comment"
fi