Skip to content

Fix Playwright PR commenter false-positives when tests are skipped #5341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
snomiao wants to merge 3 commits into from
Closed

Fix Playwright PR commenter false-positives when tests are skipped #5341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec29b77
Fix Playwright PR commenter false-positives when tests are skipped
snomiao Sep 4, 2025
671f53d
Merge branch 'main' into 5338-playwright-pr-commenter-false-positives
snomiao Sep 5, 2025
5e55f13
Fix: Move chromium deployment info generation to merge-reports job
snomiao Sep 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 66 additions & 45 deletions .github/workflows/pr-playwright-deploy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
# 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
done

echo "" >> comment.md
echo "⏰ Completed at: ${{ steps.completion-time.outputs.time }} UTC" >> comment.md
echo "" >> comment.md
echo "### 📊 Test Reports by Browser" >> comment.md
if [ "$ALL_PASSED" = "true" ]; then
echo "✅ **All tests passed across all browsers!**" >> comment.md
else
echo "❌ **Some tests failed!**" >> comment.md
fi

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="❌"
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

echo "- $status **$browser**: [View Report]($url)" >> comment.md
fi
done
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 "---" >> 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
Expand Down
48 changes: 48 additions & 0 deletions .github/workflows/test-ui.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,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]
Expand Down Expand Up @@ -284,3 +310,25 @@ jobs:
name: playwright-report-chromium
path: ComfyUI_frontend/playwright-report/
retention-days: 30

- name: Save deployment info for merged chromium report
if: always()
run: |
# Determine exit code based on the needs context
# The merge-reports job only runs if sharded tests complete (success, failure, or cancelled)
# We consider it successful only if all sharded tests succeeded
if [ "${{ needs.playwright-tests-chromium-sharded.result }}" = "success" ]; then
EXIT_CODE=0
else
EXIT_CODE=1
fi

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()
with:
name: deployment-info-chromium
path: ComfyUI_frontend/deployment-info-chromium.txt
retention-days: 1
Loading