Skip to content

Commit 06afccc

Browse files
snomiaoclaudeCopilot
committed
fix: Split PR deployment workflow for forked vs non-forked repos (#5425)
* [fix] Consolidate Playwright workflow jobs to fix missing deployment links The issue in PR #5298 was caused by missing deployment-info artifact creation. The deploy-reports job was deploying to Cloudflare but wasn't creating the deployment-info-* artifacts that comment-tests-completed job expected to download. This change consolidates the deployment and commenting into a single job, eliminating the artifact dependency and ensuring links are always available. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor: Split PR deployment workflow for forked vs non-forked repos - Extract deployment logic to reusable script (scripts/cicd/pr-playwright-deploy-and-comment.sh) - Non-forked PRs: Use direct pull_request event in test-ui.yaml (faster) - Forked PRs: Use workflow_run in pr-playwright-deploy.yaml (handles permissions) - Add starting comment for both forked and non-forked PRs - Make Cloudflare tokens optional for starting status comments * refactor: Simplify PR deployment workflow and script - Consolidate workflow into single job with clearer structure - Reduce script from 200+ to ~140 lines - Simplify deployment retry logic and comment generation - Remove redundant checks and unnecessary complexity * fix: Add debugging and wrangler installation to deployment script - Add debug output to identify missing reports - Install wrangler if not available - Show deployment attempts and failures - Log available reports before deployment * chore: Trigger CI to test deployment workflow * fix: Fix browser artifact name mismatch in deployment script - Use dot notation (0.5x) for artifact names as Playwright creates them - Convert to dash notation (0-5x) for Cloudflare project names - Properly handle browser name display in comments * refactor: Convert deployment script to POSIX sh for better compatibility - Replace bash arrays with space-separated strings - Use while loops instead of bash-specific for syntax - Remove bash-specific string manipulation features - Replace local variables (not required in functions) - Ensure compatibility with standard /bin/sh * fix: Fix deployment script output to properly capture URLs - Redirect debug messages to stderr - Only output URL to stdout for proper capture - This fixes the missing deployment links in PR comments * fix: Add input validation to prevent command injection - Validate PR number is numeric only - Sanitize branch name at script start - Validate status parameter values - Use pre-sanitized branch throughout script - Addresses high-severity security issue from PR review * fix: Add null checks and logging to workflow condition - Add explicit null checks for head_repository and repository - Add debug logging to help diagnose workflow trigger issues - Prevents potential failures from undefined repository objects - Addresses medium-severity issue from PR review * fix: Pin wrangler to major version 4 with error handling - Pin wrangler to major version 4 (^4.0.0) for stability - Add error handling if wrangler installation fails - Return 'failed' status if installation fails - Addresses dependency management issue from PR review * perf: Implement parallel deployments to reduce CI time - Deploy all browser reports in parallel using background processes - Use temporary directory to collect deployment results - Wait for all deployments to complete before generating comment - Maintains result order for consistent output - Significantly reduces deployment time from sequential to parallel execution * fix: Use specific comment ID for updates instead of edit-last - Use GitHub API to find exact comment ID - Update specific comment by ID to avoid editing wrong comment - Prevents race conditions if user posts between finding and editing - More reliable comment updates * fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency * fix(workflows/test-ui.yaml): change condition to always run deploy job for pull requests to ensure deployment consistency * fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity * fix(pr-playwright-deploy-and-comment.sh): remove npx prefix from wrangler command for consistency and simplicity * Update scripts/cicd/pr-playwright-deploy-and-comment.sh Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix(pr-playwright-deploy-and-comment.sh): improve regex for URL extraction to include valid characters and ensure correct URL format * chore(pr-playwright-deploy-and-comment.sh): move wrangler installation to the beginning of the script to avoid redundancy and improve efficiency --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 36873fe commit 06afccc

File tree

3 files changed

+360
-245
lines changed

3 files changed

+360
-245
lines changed

‎.github/workflows/pr-playwright-deploy.yaml‎

Lines changed: 57 additions & 245 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
name: PR Playwright Deploy and Comment
1+
name: PR Playwright Deploy (Forks)
22

33
on:
44
workflow_run:
@@ -9,272 +9,84 @@ env:
99
DATE_FORMAT: '+%m/%d/%Y, %I:%M:%S %p'
1010

1111
jobs:
12-
deploy-reports:
12+
deploy-and-comment-forked-pr:
1313
runs-on: ubuntu-latest
14-
if: github.repository == 'Comfy-Org/ComfyUI_frontend' && github.event.workflow_run.event == 'pull_request' && github.event.action == 'completed'
14+
if: |
15+
github.repository == 'Comfy-Org/ComfyUI_frontend' &&
16+
github.event.workflow_run.event == 'pull_request' &&
17+
github.event.workflow_run.head_repository != null &&
18+
github.event.workflow_run.repository != null &&
19+
github.event.workflow_run.head_repository.full_name != github.event.workflow_run.repository.full_name
1520
permissions:
21+
pull-requests: write
1622
actions: read
17-
strategy:
18-
fail-fast: false
19-
matrix:
20-
browser: [chromium, chromium-2x, chromium-0.5x, mobile-chrome]
2123
steps:
22-
- name: Get PR info
23-
id: pr-info
24+
- name: Log workflow trigger info
25+
run: |
26+
echo "Repository: ${{ github.repository }}"
27+
echo "Event: ${{ github.event.workflow_run.event }}"
28+
echo "Head repo: ${{ github.event.workflow_run.head_repository.full_name || 'null' }}"
29+
echo "Base repo: ${{ github.event.workflow_run.repository.full_name || 'null' }}"
30+
echo "Is forked: ${{ github.event.workflow_run.head_repository.full_name != github.event.workflow_run.repository.full_name }}"
31+
32+
- name: Checkout repository
33+
uses: actions/checkout@v4
34+
35+
- name: Get PR Number
36+
id: pr
2437
uses: actions/github-script@v7
2538
with:
2639
script: |
27-
const { data: pullRequests } = await github.rest.pulls.list({
40+
const { data: prs } = await github.rest.pulls.list({
2841
owner: context.repo.owner,
2942
repo: context.repo.repo,
3043
state: 'open',
31-
head: `${context.repo.owner}:${context.payload.workflow_run.head_branch}`,
3244
});
3345
34-
if (pullRequests.length === 0) {
35-
console.log('No open PR found for this branch');
36-
return { number: null, sanitized_branch: null };
37-
}
46+
const pr = prs.find(p => p.head.sha === context.payload.workflow_run.head_sha);
3847
39-
const pr = pullRequests[0];
40-
const branchName = context.payload.workflow_run.head_branch;
41-
const sanitizedBranch = branchName.toLowerCase().replace(/[^a-z0-9-]/g, '-').replace(/--+/g, '-').replace(/^-|-$/g, '');
48+
if (!pr) {
49+
console.log('No PR found for SHA:', context.payload.workflow_run.head_sha);
50+
return null;
51+
}
4252
43-
return {
44-
number: pr.number,
45-
sanitized_branch: sanitizedBranch
46-
};
53+
console.log(`Found PR #${pr.number} from fork: ${context.payload.workflow_run.head_repository.full_name}`);
54+
return pr.number;
4755
48-
- name: Set project name
49-
if: fromJSON(steps.pr-info.outputs.result).number != null
50-
id: project-name
56+
- name: Handle Test Start
57+
if: steps.pr.outputs.result != 'null' && github.event.action == 'requested'
58+
env:
59+
GITHUB_TOKEN: ${{ github.token }}
5160
run: |
52-
if [ "${{ matrix.browser }}" = "chromium-0.5x" ]; then
53-
echo "name=comfyui-playwright-chromium-0-5x" >> $GITHUB_OUTPUT
54-
else
55-
echo "name=comfyui-playwright-${{ matrix.browser }}" >> $GITHUB_OUTPUT
56-
fi
57-
echo "branch=${{ fromJSON(steps.pr-info.outputs.result).sanitized_branch }}" >> $GITHUB_OUTPUT
58-
59-
- name: Download playwright report
60-
if: fromJSON(steps.pr-info.outputs.result).number != null
61+
chmod +x scripts/cicd/pr-playwright-deploy-and-comment.sh
62+
./scripts/cicd/pr-playwright-deploy-and-comment.sh \
63+
"${{ steps.pr.outputs.result }}" \
64+
"${{ github.event.workflow_run.head_branch }}" \
65+
"starting" \
66+
"$(date -u '${{ env.DATE_FORMAT }}')"
67+
68+
- name: Download and Deploy Reports
69+
if: steps.pr.outputs.result != 'null' && github.event.action == 'completed'
6170
uses: actions/download-artifact@v4
6271
with:
6372
github-token: ${{ secrets.GITHUB_TOKEN }}
6473
run-id: ${{ github.event.workflow_run.id }}
65-
name: playwright-report-${{ matrix.browser }}
66-
path: playwright-report
67-
68-
- name: Install Wrangler
69-
if: fromJSON(steps.pr-info.outputs.result).number != null
70-
run: npm install -g wrangler
71-
72-
- name: Deploy to Cloudflare Pages (${{ matrix.browser }})
73-
if: fromJSON(steps.pr-info.outputs.result).number != null
74-
id: cloudflare-deploy
75-
continue-on-error: true
76-
run: |
77-
# Retry logic for wrangler deploy (3 attempts)
78-
RETRY_COUNT=0
79-
MAX_RETRIES=3
80-
SUCCESS=false
74+
pattern: playwright-report-*
75+
path: reports
8176

82-
while [ $RETRY_COUNT -lt $MAX_RETRIES ] && [ $SUCCESS = false ]; do
83-
RETRY_COUNT=$((RETRY_COUNT + 1))
84-
echo "Deployment attempt $RETRY_COUNT of $MAX_RETRIES..."
85-
86-
if npx wrangler pages deploy playwright-report --project-name=${{ steps.project-name.outputs.name }} --branch=${{ steps.project-name.outputs.branch }}; then
87-
SUCCESS=true
88-
echo "Deployment successful on attempt $RETRY_COUNT"
89-
else
90-
echo "Deployment failed on attempt $RETRY_COUNT"
91-
if [ $RETRY_COUNT -lt $MAX_RETRIES ]; then
92-
echo "Retrying in 10 seconds..."
93-
sleep 10
94-
fi
95-
fi
96-
done
97-
98-
if [ $SUCCESS = false ]; then
99-
echo "All deployment attempts failed"
100-
exit 1
101-
fi
77+
- name: Handle Test Completion
78+
if: steps.pr.outputs.result != 'null' && github.event.action == 'completed'
10279
env:
10380
CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }}
10481
CLOUDFLARE_ACCOUNT_ID: ${{ secrets.CLOUDFLARE_ACCOUNT_ID }}
105-
106-
comment-tests-starting:
107-
runs-on: ubuntu-latest
108-
if: github.repository == 'Comfy-Org/ComfyUI_frontend' && github.event.workflow_run.event == 'pull_request' && github.event.action == 'requested'
109-
permissions:
110-
pull-requests: write
111-
actions: read
112-
steps:
113-
- name: Get PR number
114-
id: pr
115-
uses: actions/github-script@v7
116-
with:
117-
script: |
118-
const { data: pullRequests } = await github.rest.pulls.list({
119-
owner: context.repo.owner,
120-
repo: context.repo.repo,
121-
state: 'open',
122-
head: `${context.repo.owner}:${context.payload.workflow_run.head_branch}`,
123-
});
124-
125-
if (pullRequests.length === 0) {
126-
console.log('No open PR found for this branch');
127-
return null;
128-
}
129-
130-
return pullRequests[0].number;
131-
132-
- name: Get completion time
133-
id: completion-time
134-
run: echo "time=$(date -u '${{ env.DATE_FORMAT }}')" >> $GITHUB_OUTPUT
135-
136-
- name: Generate comment body for start
137-
if: steps.pr.outputs.result != 'null'
138-
id: comment-body-start
82+
GITHUB_TOKEN: ${{ github.token }}
13983
run: |
140-
echo "<!-- PLAYWRIGHT_TEST_STATUS -->" > comment.md
141-
echo "## 🎭 Playwright Test Results" >> comment.md
142-
echo "" >> comment.md
143-
echo "<img alt='comfy-loading-gif' src='https://github.com/user-attachments/assets/755c86ee-e445-4ea8-bc2c-cca85df48686' width='14px' height='14px' style='vertical-align: middle; margin-right: 4px;' /> **Tests are starting...** " >> comment.md
144-
echo "" >> comment.md
145-
echo "⏰ Started at: ${{ steps.completion-time.outputs.time }} UTC" >> comment.md
146-
echo "" >> comment.md
147-
echo "### 🚀 Running Tests" >> comment.md
148-
echo "- 🧪 **chromium**: Running tests..." >> comment.md
149-
echo "- 🧪 **chromium-0.5x**: Running tests..." >> comment.md
150-
echo "- 🧪 **chromium-2x**: Running tests..." >> comment.md
151-
echo "- 🧪 **mobile-chrome**: Running tests..." >> comment.md
152-
echo "" >> comment.md
153-
echo "---" >> comment.md
154-
echo "⏱️ Please wait while tests are running across all browsers..." >> comment.md
155-
156-
- name: Comment PR - Tests Started
157-
if: steps.pr.outputs.result != 'null'
158-
uses: edumserrano/find-create-or-update-comment@82880b65c8a3a6e4c70aa05a204995b6c9696f53 # v3.0.0
159-
with:
160-
issue-number: ${{ steps.pr.outputs.result }}
161-
body-includes: '<!-- PLAYWRIGHT_TEST_STATUS -->'
162-
comment-author: 'github-actions[bot]'
163-
edit-mode: replace
164-
body-path: comment.md
165-
166-
comment-tests-completed:
167-
runs-on: ubuntu-latest
168-
needs: deploy-reports
169-
if: github.repository == 'Comfy-Org/ComfyUI_frontend' && github.event.workflow_run.event == 'pull_request' && github.event.action == 'completed' && always()
170-
permissions:
171-
pull-requests: write
172-
actions: read
173-
steps:
174-
- name: Get PR number
175-
id: pr
176-
uses: actions/github-script@v7
177-
with:
178-
script: |
179-
const { data: pullRequests } = await github.rest.pulls.list({
180-
owner: context.repo.owner,
181-
repo: context.repo.repo,
182-
state: 'open',
183-
head: `${context.repo.owner}:${context.payload.workflow_run.head_branch}`,
184-
});
185-
186-
if (pullRequests.length === 0) {
187-
console.log('No open PR found for this branch');
188-
return null;
189-
}
190-
191-
return pullRequests[0].number;
192-
193-
- name: Download all deployment info
194-
if: steps.pr.outputs.result != 'null'
195-
uses: actions/download-artifact@v4
196-
with:
197-
github-token: ${{ secrets.GITHUB_TOKEN }}
198-
run-id: ${{ github.event.workflow_run.id }}
199-
pattern: deployment-info-*
200-
merge-multiple: true
201-
path: deployment-info
202-
203-
- name: Get completion time
204-
id: completion-time
205-
run: echo "time=$(date -u '${{ env.DATE_FORMAT }}')" >> $GITHUB_OUTPUT
206-
207-
- name: Generate comment body for completion
208-
if: steps.pr.outputs.result != 'null'
209-
id: comment-body-completed
210-
run: |
211-
echo "<!-- PLAYWRIGHT_TEST_STATUS -->" > comment.md
212-
echo "## 🎭 Playwright Test Results" >> comment.md
213-
echo "" >> comment.md
214-
215-
# Check if all tests passed
216-
ALL_PASSED=true
217-
for file in deployment-info/*.txt; do
218-
if [ -f "$file" ]; then
219-
browser=$(basename "$file" .txt)
220-
info=$(cat "$file")
221-
exit_code=$(echo "$info" | cut -d'|' -f2)
222-
if [ "$exit_code" != "0" ]; then
223-
ALL_PASSED=false
224-
break
225-
fi
226-
fi
227-
done
228-
229-
if [ "$ALL_PASSED" = "true" ]; then
230-
echo "✅ **All tests passed across all browsers!**" >> comment.md
231-
else
232-
echo "❌ **Some tests failed!**" >> comment.md
233-
fi
234-
235-
echo "" >> comment.md
236-
echo "⏰ Completed at: ${{ steps.completion-time.outputs.time }} UTC" >> comment.md
237-
echo "" >> comment.md
238-
echo "### 📊 Test Reports by Browser" >> comment.md
239-
240-
for file in deployment-info/*.txt; do
241-
if [ -f "$file" ]; then
242-
browser=$(basename "$file" .txt)
243-
info=$(cat "$file")
244-
exit_code=$(echo "$info" | cut -d'|' -f2)
245-
url=$(echo "$info" | cut -d'|' -f3)
246-
247-
# Validate URLs before using them in comments
248-
sanitized_url=$(echo "$url" | grep -E '^https://[a-z0-9.-]+\.pages\.dev(/.*)?$' || echo "INVALID_URL")
249-
if [ "$sanitized_url" = "INVALID_URL" ]; then
250-
echo "Invalid deployment URL detected: $url"
251-
url="#" # Use safe fallback
252-
fi
253-
254-
if [ "$exit_code" = "0" ]; then
255-
status="✅"
256-
else
257-
status="❌"
258-
fi
259-
260-
echo "- $status **$browser**: [View Report]($url)" >> comment.md
261-
fi
262-
done
263-
264-
echo "" >> comment.md
265-
echo "---" >> comment.md
266-
if [ "$ALL_PASSED" = "true" ]; then
267-
echo "🎉 Your tests are passing across all browsers!" >> comment.md
268-
else
269-
echo "⚠️ Please check the test reports for details on failures." >> comment.md
270-
fi
271-
272-
- name: Comment PR - Tests Complete
273-
if: steps.pr.outputs.result != 'null'
274-
uses: edumserrano/find-create-or-update-comment@82880b65c8a3a6e4c70aa05a204995b6c9696f53 # v3.0.0
275-
with:
276-
issue-number: ${{ steps.pr.outputs.result }}
277-
body-includes: '<!-- PLAYWRIGHT_TEST_STATUS -->'
278-
comment-author: 'github-actions[bot]'
279-
edit-mode: replace
280-
body-path: comment.md
84+
# Rename merged report if exists
85+
[ -d "reports/playwright-report-chromium-merged" ] && \
86+
mv reports/playwright-report-chromium-merged reports/playwright-report-chromium
87+
88+
chmod +x scripts/cicd/pr-playwright-deploy-and-comment.sh
89+
./scripts/cicd/pr-playwright-deploy-and-comment.sh \
90+
"${{ steps.pr.outputs.result }}" \
91+
"${{ github.event.workflow_run.head_branch }}" \
92+
"completed"

0 commit comments

Comments
 (0)