From 8c0c819471847e62e0a3653d92aedbe1b4ebb3b7 Mon Sep 17 00:00:00 2001 From: snomiao Date: Mon, 22 Dec 2025 19:30:08 +0000 Subject: [PATCH] Improve Playwright PR comment format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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
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 --- scripts/cicd/extract-playwright-counts.ts | 121 +++++++++++++++++- .../cicd/pr-playwright-deploy-and-comment.sh | 101 +++++++++++---- 2 files changed, 198 insertions(+), 24 deletions(-) diff --git a/scripts/cicd/extract-playwright-counts.ts b/scripts/cicd/extract-playwright-counts.ts index ff6f44db30..89926ab98c 100755 --- a/scripts/cicd/extract-playwright-counts.ts +++ b/scripts/cicd/extract-playwright-counts.ts @@ -10,8 +10,48 @@ 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 } interface TestCounts { @@ -20,12 +60,64 @@ interface TestCounts { flaky: number skipped: number total: number + failingTests?: FailingTest[] +} + +/** + * 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 + } + } + + 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 + }) + } + } + } + } + + // 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 +125,8 @@ function extractTestCounts(reportDir: string): TestCounts { failed: 0, flaky: 0, skipped: 0, - total: 0 + total: 0, + failingTests: [] } try { @@ -54,6 +147,14 @@ 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) + } + } + return counts } } @@ -86,6 +187,14 @@ 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) + } + } + return counts } } catch (e) { @@ -113,6 +222,14 @@ 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) + } + } + return counts } } catch (e) { diff --git a/scripts/cicd/pr-playwright-deploy-and-comment.sh b/scripts/cicd/pr-playwright-deploy-and-comment.sh index 840203f44a..9940b44475 100755 --- a/scripts/cicd/pr-playwright-deploy-and-comment.sh +++ b/scripts/cicd/pr-playwright-deploy-and-comment.sh @@ -302,35 +302,93 @@ 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" 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 + trace_link="" + 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=$(echo "$url" | sed 's|/index.html||') + trace_link="${base_url}/data/${trace_file}" + 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 + +
+📊 Test Reports by Browser + +" + + # 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 +405,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 +414,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." +
" post_comment "$comment" fi