mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-01 11:10:00 +00:00
Fix Playwright PR commenter false-positives when tests are skipped
This PR fixes the issue where the PR comment bot incorrectly reports "All tests passed" when Playwright tests are skipped or don't run. Changes: 1. Modified test-ui.yaml to capture and upload test status as deployment-info artifacts 2. Modified pr-playwright-deploy.yaml to properly handle missing test results The fix ensures that: - Test exit codes are properly captured and passed between workflows - When tests are skipped, the comment clearly states "Tests were skipped or not run" - Each browser test result is tracked individually with proper status reporting Fixes #5338 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
snomiao
113
.github/workflows/pr-playwright-deploy.yaml
vendored
113
.github/workflows/pr-playwright-deploy.yaml
vendored
@@ -212,61 +212,82 @@ jobs:
|
||||
echo "## 🎭 Playwright Test Results" >> comment.md
|
||||
echo "" >> comment.md
|
||||
|
||||
# Check if all tests passed
|
||||
ALL_PASSED=true
|
||||
# Check if we have any test results
|
||||
FILES_FOUND=false
|
||||
for file in deployment-info/*.txt; do
|
||||
if [ -f "$file" ]; then
|
||||
browser=$(basename "$file" .txt)
|
||||
info=$(cat "$file")
|
||||
exit_code=$(echo "$info" | cut -d'|' -f2)
|
||||
if [ "$exit_code" != "0" ]; then
|
||||
ALL_PASSED=false
|
||||
break
|
||||
fi
|
||||
FILES_FOUND=true
|
||||
break
|
||||
fi
|
||||
done
|
||||
|
||||
if [ "$ALL_PASSED" = "true" ]; then
|
||||
echo "✅ **All tests passed across all browsers!**" >> comment.md
|
||||
if [ "$FILES_FOUND" = "false" ]; then
|
||||
# No test results found - tests were skipped or not run
|
||||
echo "⏭️ **Tests were skipped or not run**" >> comment.md
|
||||
echo "" >> comment.md
|
||||
echo "⏰ Completed at: ${{ steps.completion-time.outputs.time }} UTC" >> comment.md
|
||||
echo "" >> comment.md
|
||||
echo "No test results available for this PR. This can happen when:" >> comment.md
|
||||
echo "- Tests were skipped due to branch filters" >> comment.md
|
||||
echo "- The test workflow didn't complete successfully" >> comment.md
|
||||
echo "- Tests are still running" >> comment.md
|
||||
else
|
||||
echo "❌ **Some tests failed!**" >> comment.md
|
||||
fi
|
||||
|
||||
echo "" >> comment.md
|
||||
echo "⏰ Completed at: ${{ steps.completion-time.outputs.time }} UTC" >> comment.md
|
||||
echo "" >> comment.md
|
||||
echo "### 📊 Test Reports by Browser" >> comment.md
|
||||
|
||||
for file in deployment-info/*.txt; do
|
||||
if [ -f "$file" ]; then
|
||||
browser=$(basename "$file" .txt)
|
||||
info=$(cat "$file")
|
||||
exit_code=$(echo "$info" | cut -d'|' -f2)
|
||||
url=$(echo "$info" | cut -d'|' -f3)
|
||||
|
||||
# Validate URLs before using them in comments
|
||||
sanitized_url=$(echo "$url" | grep -E '^https://[a-z0-9.-]+\.pages\.dev(/.*)?$' || echo "INVALID_URL")
|
||||
if [ "$sanitized_url" = "INVALID_URL" ]; then
|
||||
echo "Invalid deployment URL detected: $url"
|
||||
url="#" # Use safe fallback
|
||||
# Check if all tests passed
|
||||
ALL_PASSED=true
|
||||
for file in deployment-info/*.txt; do
|
||||
if [ -f "$file" ]; then
|
||||
browser=$(basename "$file" .txt)
|
||||
info=$(cat "$file")
|
||||
exit_code=$(echo "$info" | cut -d'|' -f2)
|
||||
if [ "$exit_code" != "0" ]; then
|
||||
ALL_PASSED=false
|
||||
break
|
||||
fi
|
||||
fi
|
||||
|
||||
if [ "$exit_code" = "0" ]; then
|
||||
status="✅"
|
||||
else
|
||||
status="❌"
|
||||
fi
|
||||
|
||||
echo "- $status **$browser**: [View Report]($url)" >> comment.md
|
||||
done
|
||||
|
||||
if [ "$ALL_PASSED" = "true" ]; then
|
||||
echo "✅ **All tests passed across all browsers!**" >> comment.md
|
||||
else
|
||||
echo "❌ **Some tests failed!**" >> comment.md
|
||||
fi
|
||||
done
|
||||
|
||||
echo "" >> comment.md
|
||||
echo "---" >> comment.md
|
||||
if [ "$ALL_PASSED" = "true" ]; then
|
||||
echo "🎉 Your tests are passing across all browsers!" >> comment.md
|
||||
else
|
||||
echo "⚠️ Please check the test reports for details on failures." >> comment.md
|
||||
echo "" >> comment.md
|
||||
echo "⏰ Completed at: ${{ steps.completion-time.outputs.time }} UTC" >> comment.md
|
||||
echo "" >> comment.md
|
||||
echo "### 📊 Test Reports by Browser" >> comment.md
|
||||
|
||||
for file in deployment-info/*.txt; do
|
||||
if [ -f "$file" ]; then
|
||||
browser=$(basename "$file" .txt)
|
||||
info=$(cat "$file")
|
||||
exit_code=$(echo "$info" | cut -d'|' -f2)
|
||||
url=$(echo "$info" | cut -d'|' -f3)
|
||||
|
||||
# Validate URLs before using them in comments
|
||||
sanitized_url=$(echo "$url" | grep -E '^https://[a-z0-9.-]+\.pages\.dev(/.*)?$' || echo "INVALID_URL")
|
||||
if [ "$sanitized_url" = "INVALID_URL" ]; then
|
||||
echo "Invalid deployment URL detected: $url"
|
||||
url="#" # Use safe fallback
|
||||
fi
|
||||
|
||||
if [ "$exit_code" = "0" ]; then
|
||||
status="✅"
|
||||
else
|
||||
status="❌"
|
||||
fi
|
||||
|
||||
echo "- $status **$browser**: [View Report]($url)" >> comment.md
|
||||
fi
|
||||
done
|
||||
|
||||
echo "" >> comment.md
|
||||
echo "---" >> comment.md
|
||||
if [ "$ALL_PASSED" = "true" ]; then
|
||||
echo "🎉 Your tests are passing across all browsers!" >> comment.md
|
||||
else
|
||||
echo "⚠️ Please check the test reports for details on failures." >> comment.md
|
||||
fi
|
||||
fi
|
||||
|
||||
- name: Comment PR - Tests Complete
|
||||
|
||||
47
.github/workflows/test-ui.yaml
vendored
47
.github/workflows/test-ui.yaml
vendored
@@ -140,6 +140,27 @@ jobs:
|
||||
path: blob-report/
|
||||
retention-days: 1
|
||||
|
||||
# Save test result for deployment info (only on shard 1 to avoid duplicates)
|
||||
- name: Save test result for chromium
|
||||
if: always() && matrix.shardIndex == 1
|
||||
run: |
|
||||
# Use outcome to determine exit code (0 for success, 1 for failure/skipped)
|
||||
if [ "${{ steps.playwright.outcome }}" = "success" ]; then
|
||||
EXIT_CODE=0
|
||||
else
|
||||
EXIT_CODE=1
|
||||
fi
|
||||
# For sharded tests, we use the main chromium project name
|
||||
echo "chromium|${EXIT_CODE}|https://comfyui-playwright-chromium.pages.dev" > deployment-info-chromium.txt
|
||||
working-directory: ComfyUI_frontend
|
||||
|
||||
- uses: actions/upload-artifact@v4
|
||||
if: always() && matrix.shardIndex == 1
|
||||
with:
|
||||
name: deployment-info-chromium
|
||||
path: ComfyUI_frontend/deployment-info-chromium.txt
|
||||
retention-days: 1
|
||||
|
||||
playwright-tests:
|
||||
# Ideally, each shard runs test in 6 minutes, but allow up to 15 minutes
|
||||
timeout-minutes: 15
|
||||
@@ -204,6 +225,32 @@ jobs:
|
||||
path: ComfyUI_frontend/playwright-report/
|
||||
retention-days: 30
|
||||
|
||||
# Save test result for deployment info
|
||||
- name: Save test result for ${{ matrix.browser }}
|
||||
if: always()
|
||||
run: |
|
||||
# Use outcome to determine exit code (0 for success, 1 for failure/skipped)
|
||||
if [ "${{ steps.playwright.outcome }}" = "success" ]; then
|
||||
EXIT_CODE=0
|
||||
else
|
||||
EXIT_CODE=1
|
||||
fi
|
||||
# Generate project name based on browser
|
||||
if [ "${{ matrix.browser }}" = "chromium-0.5x" ]; then
|
||||
PROJECT_NAME="comfyui-playwright-chromium-0-5x"
|
||||
else
|
||||
PROJECT_NAME="comfyui-playwright-${{ matrix.browser }}"
|
||||
fi
|
||||
echo "${{ matrix.browser }}|${EXIT_CODE}|https://${PROJECT_NAME}.pages.dev" > deployment-info-${{ matrix.browser }}.txt
|
||||
working-directory: ComfyUI_frontend
|
||||
|
||||
- uses: actions/upload-artifact@v4
|
||||
if: always()
|
||||
with:
|
||||
name: deployment-info-${{ matrix.browser }}
|
||||
path: ComfyUI_frontend/deployment-info-${{ matrix.browser }}.txt
|
||||
retention-days: 1
|
||||
|
||||
# Merge sharded test reports
|
||||
merge-reports:
|
||||
needs: [playwright-tests-chromium-sharded]
|
||||
|
||||
Reference in New Issue
Block a user