Skip to content

Commit bdc0345

Browse files
claude[bot]claude
andauthored
feat: attach video walkthrough of newly-added tests to PR comment (#14560)
&lt;!-- ccr-slack-attribution --&gt; _Requested by **Christian Byrne** · [Slack thread](https://comfy-organization.slack.com/archives/C090H3X4VBL/p1785633691296179?thread_ts=1785619438.875159&amp;cid=C090H3X4VBL)_ ## Summary When a PR adds a **new** Playwright spec file, CI now records a video walkthrough of just that new test and posts a link to it in the PR comment, next to the existing trace-viewer link. The existing suite keeps running without video, so CI cost/time doesn't grow. ## Changes - **What**: - `.github/workflows/ci-tests-e2e.yaml`: new `playwright-video-new-tests` job diffs the PR against its base branch (`git diff --diff-filter=A`) to find spec files added under `browser_tests/tests/`. If (and only if) there are any, it re-runs just those files with Playwright video recording turned on, then a new `deploy-and-comment-video` job deploys the video(s) to Cloudflare Pages (same mechanism as the existing trace/report deploy — `wrangler pages deploy`) and extends the existing unified PR comment with a collapsed `video` section, using the same `upsert-comment-section` action the trace section already uses. - `.github/workflows/ci-tests-e2e-forks.yaml`: mirrors the same deploy+comment steps for forked PRs, following this repo's existing fork-safe two-workflow split (`workflow_run` downloads the artifact and posts the comment with write permissions, since fork PRs only get a read-only token). - `playwright.config.ts`: video is opt-in via a `RECORD_VIDEO=true` env var (only set by the new CI job), slowed down via `SLOW_MO=1000` so the recording is legible — no change to default behavior for local runs or the rest of the suite. - `scripts/cicd/extract-playwright-videos.ts` / `scripts/cicd/pr-video-deploy-and-comment.sh`: new scripts mirroring the existing `extract-playwright-counts.ts` / `pr-playwright-deploy-and-comment.sh` pattern, but for video attachments instead of trace/pass-fail counts. - `browser_tests/fixtures/utils/litegraphUtils.ts`: fixes `connectOutput`/`connectWidget` to route slot positions through `toAbsolute()` before dragging (they were canvas-relative pixels being fed to page-absolute drag coordinates). - `browser_tests/README.md`: documents the new-test video workflow. - **Breaking**: none — additive only; existing trace/report comment behavior is untouched, and the new jobs are not part of the required `e2e-status` gate check. - **Dependencies**: none new (reuses `wrangler`, already used by the existing trace deploy step). ## Review Focus - The "which files are new" detection (`git diff --diff-filter=A ...HEAD -- browser_tests/tests`) relies on `fetch-depth: 0` in the new job's checkout step. - The bash `pr-*-deploy-and-comment.sh` scripts each duplicate their own Cloudflare deploy-retry logic rather than sharing a helper — matching the existing convention between `pr-playwright-deploy-and-comment.sh` and `pr-storybook-deploy-and-comment.sh`, so this follows suit rather than introducing a new shared-module pattern. ## Screenshots (if applicable) N/A — this PR changes CI plumbing; the effect is a collapsible "🎬 New-test video walkthrough" section in a PR's bot comment whenever that PR adds a new spec file. --- _Generated by [Claude Code](https://claude.ai/code/session_019rbmF3UTtRDFYkWEDnhzVL)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 84d94e1 commit bdc0345

10 files changed

Lines changed: 180 additions & 36 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
name: Read Section File and Upsert Comment Section
2+
description: >
3+
Reads a generated markdown section file, if present and non-empty, and
4+
upserts it into the unified PR report comment via upsert-comment-section.
5+
No-ops when the section file is missing or empty, so callers don't need
6+
their own hashFiles guard around the read+upsert step pair.
7+
8+
inputs:
9+
pr-number:
10+
description: PR number to comment on
11+
required: true
12+
section-name:
13+
description: 'Section identifier (e.g. "playwright", "video")'
14+
required: true
15+
section-file:
16+
description: Path to the markdown file containing the section content
17+
required: true
18+
token:
19+
description: GitHub token with pull-requests write permission
20+
required: true
21+
comment-marker:
22+
description: Top-level HTML comment marker shared by all sections in this comment
23+
required: false
24+
default: '<!-- COMFYUI_FRONTEND_PR_REPORT -->'
25+
26+
runs:
27+
using: composite
28+
steps:
29+
- name: Read section file
30+
id: section
31+
if: ${{ hashFiles(inputs.section-file) != '' }}
32+
uses: juliangruber/read-file-action@b549046febe0fe86f8cb4f93c24e284433f9ab58 # v1.1.7
33+
with:
34+
path: ${{ inputs.section-file }}
35+
36+
- name: Upsert section into unified report
37+
if: ${{ steps.section.outputs.content != '' }}
38+
uses: ./.github/actions/upsert-comment-section
39+
with:
40+
pr-number: ${{ inputs.pr-number }}
41+
section-name: ${{ inputs.section-name }}
42+
section-content: ${{ steps.section.outputs.content }}
43+
comment-marker: ${{ inputs.comment-marker }}
44+
token: ${{ inputs.token }}

.github/actions/upsert-comment-section/action.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ inputs:
1717
required: true
1818
comment-marker:
1919
description: Top-level HTML comment marker shared by all sections in this comment
20-
required: true
20+
required: false
21+
default: '<!-- COMFYUI_FRONTEND_PR_REPORT -->'
2122
token:
2223
description: GitHub token with pull-requests write permission
2324
required: true

.github/workflows/ci-tests-e2e-forks.yaml

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ jobs:
5858
pr-number: ${{ steps.pr.outputs.number }}
5959
section-name: playwright
6060
section-content: '## 🎭 Playwright: ⏳ Running...'
61-
comment-marker: '<!-- COMFYUI_FRONTEND_PR_REPORT -->'
6261
token: ${{ github.token }}
6362

6463
- name: Download and Deploy Reports
@@ -91,19 +90,11 @@ jobs:
9190
"$BRANCH_NAME" \
9291
"completed"
9392
94-
- name: Read playwright section
95-
id: section
96-
if: steps.route.outputs.handle == 'true' && github.event.action == 'completed' && hashFiles('playwright-section.md') != ''
97-
uses: juliangruber/read-file-action@b549046febe0fe86f8cb4f93c24e284433f9ab58 # v1.1.7
98-
with:
99-
path: playwright-section.md
100-
101-
- name: Upsert playwright section into unified report
102-
if: steps.route.outputs.handle == 'true' && github.event.action == 'completed' && steps.section.outputs.content != ''
103-
uses: ./.github/actions/upsert-comment-section
93+
- name: Read and upsert playwright section
94+
if: steps.route.outputs.handle == 'true' && github.event.action == 'completed'
95+
uses: ./.github/actions/read-and-upsert-comment-section
10496
with:
10597
pr-number: ${{ steps.pr.outputs.number }}
10698
section-name: playwright
107-
section-content: ${{ steps.section.outputs.content }}
108-
comment-marker: '<!-- COMFYUI_FRONTEND_PR_REPORT -->'
99+
section-file: playwright-section.md
109100
token: ${{ github.token }}

.github/workflows/ci-tests-e2e.yaml

Lines changed: 90 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,84 @@ jobs:
214214
[[ "$SHARDED" != "success" || "$BROWSERS" != "success" ]] && echo "E2E failed" && exit 1
215215
echo "E2E passed"
216216
217+
# Records video only for spec files newly added in this PR, to keep cost bounded.
218+
playwright-video-new-tests:
219+
needs: setup
220+
if: ${{ github.event_name == 'pull_request' }}
221+
runs-on: ubuntu-latest
222+
timeout-minutes: 15
223+
container:
224+
image: ghcr.io/comfy-org/comfyui-ci-container:0.0.21
225+
credentials:
226+
username: ${{ github.actor }}
227+
password: ${{ secrets.GITHUB_TOKEN }}
228+
permissions:
229+
contents: read
230+
packages: read
231+
outputs:
232+
has-new-tests: ${{ steps.detect.outputs.has-new-tests }}
233+
steps:
234+
- name: Checkout repository
235+
uses: actions/checkout@v7
236+
with:
237+
fetch-depth: 0
238+
239+
- name: Detect newly-added test spec files
240+
id: detect
241+
run: |
242+
git config --global --add safe.directory "$GITHUB_WORKSPACE"
243+
BASE_SHA="${{ github.event.pull_request.base.sha }}"
244+
NEW_FILES=$(git diff --name-only --diff-filter=A "$BASE_SHA"...HEAD -- browser_tests/tests | grep '\.spec\.ts$' || true)
245+
246+
if [ -z "$NEW_FILES" ]; then
247+
echo "has-new-tests=false" >> "$GITHUB_OUTPUT"
248+
else
249+
echo "has-new-tests=true" >> "$GITHUB_OUTPUT"
250+
fi
251+
252+
{
253+
echo "files<<EOF_NEW_SPEC_FILES"
254+
echo "$NEW_FILES"
255+
echo "EOF_NEW_SPEC_FILES"
256+
} >> "$GITHUB_OUTPUT"
257+
258+
echo "Newly-added spec files:"
259+
echo "${NEW_FILES:-<none>}"
260+
261+
- name: Download built frontend
262+
if: steps.detect.outputs.has-new-tests == 'true'
263+
uses: actions/download-artifact@v7
264+
with:
265+
name: frontend-dist
266+
path: dist/
267+
268+
- name: Start ComfyUI server
269+
if: steps.detect.outputs.has-new-tests == 'true'
270+
uses: ./.github/actions/start-comfyui-server
271+
272+
- name: Install frontend deps
273+
if: steps.detect.outputs.has-new-tests == 'true'
274+
run: pnpm install --frozen-lockfile
275+
276+
- name: Run new test(s) with video recording
277+
if: steps.detect.outputs.has-new-tests == 'true'
278+
env:
279+
RECORD_VIDEO: 'true'
280+
SLOW_MO: '1000'
281+
NEW_SPEC_FILES: ${{ steps.detect.outputs.files }}
282+
run: |
283+
FILES=$(echo "$NEW_SPEC_FILES" | tr '\n' ' ')
284+
pnpm exec playwright test --project=chromium $FILES
285+
286+
- name: Upload new-test report (with embedded video)
287+
if: ${{ !cancelled() && steps.detect.outputs.has-new-tests == 'true' }}
288+
uses: actions/upload-artifact@v6
289+
with:
290+
name: playwright-report-new-tests
291+
path: ./playwright-report/
292+
retention-days: 30
293+
if-no-files-found: warn
294+
217295
#### BEGIN Deployment and commenting (PRs with direct secret access)
218296
# when using pull_request event, we have permission to comment directly
219297
# otherwise, we use workflow_run in ci-tests-e2e-forks.yaml
@@ -222,6 +300,9 @@ jobs:
222300
comment-on-pr-start:
223301
needs: changes
224302
runs-on: ubuntu-latest
303+
concurrency:
304+
group: pr-comment-${{ github.event.pull_request.number }}
305+
cancel-in-progress: false
225306
if: >-
226307
${{
227308
needs.changes.outputs.should-run == 'true' &&
@@ -241,13 +322,16 @@ jobs:
241322
pr-number: ${{ github.event.pull_request.number }}
242323
section-name: playwright
243324
section-content: '## 🎭 Playwright: ⏳ Running...'
244-
comment-marker: '<!-- COMFYUI_FRONTEND_PR_REPORT -->'
245325
token: ${{ github.token }}
246326

247327
# Deploy and upsert final playwright section
248328
deploy-and-comment:
249-
needs: [changes, playwright-tests, merge-reports]
329+
needs:
330+
[changes, playwright-tests, merge-reports, playwright-video-new-tests]
250331
runs-on: ubuntu-latest
332+
concurrency:
333+
group: pr-comment-${{ github.event.pull_request.number }}
334+
cancel-in-progress: false
251335
if: >-
252336
${{
253337
always() &&
@@ -283,19 +367,11 @@ jobs:
283367
"$BRANCH_NAME" \
284368
"completed"
285369
286-
- name: Read playwright section
287-
id: section
288-
if: ${{ !cancelled() && hashFiles('playwright-section.md') != '' }}
289-
uses: juliangruber/read-file-action@b549046febe0fe86f8cb4f93c24e284433f9ab58 # v1.1.7
290-
with:
291-
path: playwright-section.md
292-
293-
- name: Upsert playwright section into unified report
294-
if: ${{ !cancelled() && steps.section.outputs.content != '' }}
295-
uses: ./.github/actions/upsert-comment-section
370+
- name: Read and upsert playwright section
371+
if: ${{ !cancelled() }}
372+
uses: ./.github/actions/read-and-upsert-comment-section
296373
with:
297374
pr-number: ${{ github.event.pull_request.number }}
298375
section-name: playwright
299-
section-content: ${{ steps.section.outputs.content }}
300-
comment-marker: '<!-- COMFYUI_FRONTEND_PR_REPORT -->'
376+
section-file: playwright-section.md
301377
token: ${{ github.token }}

apps/website/src/components/common/BreadcrumbBar.vue

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const {
1818
class="max-w-9xl mx-auto flex items-center justify-between gap-4 px-6 py-3 lg:px-20"
1919
>
2020
<ol
21-
class="text-primary-warm-gray flex flex-wrap items-center gap-2 text-xs tracking-wide uppercase"
21+
class="flex flex-wrap items-center gap-2 text-xs tracking-wide text-primary-warm-gray uppercase"
2222
>
2323
<li
2424
v-for="(crumb, i) in crumbs"
@@ -43,7 +43,7 @@ const {
4343

4444
<p
4545
v-if="updated"
46-
class="text-primary-warm-gray shrink-0 text-xs tracking-wide uppercase"
46+
class="shrink-0 text-xs tracking-wide text-primary-warm-gray uppercase"
4747
>
4848
{{ updated }}
4949
</p>

apps/website/src/components/product/local/MobileDownloadEmailForm.vue

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ async function onSubmit() {
149149
tabindex="-1"
150150
:class="
151151
status === 'success'
152-
? 'bg-transparency-white-t4 text-primary-warm-gray flex h-16 items-center rounded-3xl px-4 text-[13px] font-semibold wrap-break-word focus:outline-none'
152+
? 'bg-transparency-white-t4 flex h-16 items-center rounded-3xl px-4 text-[13px] font-semibold wrap-break-word text-primary-warm-gray focus:outline-none'
153153
: undefined
154154
"
155155
>

apps/website/src/components/ui/icon-button/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export const iconButtonVariants = cva(
99
variants: {
1010
variant: {
1111
ghost:
12-
'text-primary-warm-white hover:text-primary-comfy-yellow bg-transparent',
12+
'hover:text-primary-comfy-yellow bg-transparent text-primary-warm-white',
1313
outline:
1414
'text-primary-comfy-yellow hover:bg-primary-comfy-yellow border-primary-comfy-yellow border-2 bg-primary-comfy-ink hover:text-primary-comfy-ink',
1515
solid: 'bg-primary-comfy-yellow text-primary-comfy-ink hover:opacity-90'

browser_tests/README.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,19 @@ Cloudflare Pages project per browser configuration
738738
(`https://[branch].comfyui-playwright-[browser].pages.dev`). PR comments carry
739739
/status and direct report links per browser.
740740

741+
## New-Test Video Walkthrough
742+
743+
CI re-runs any Playwright spec file newly added in a PR (detected via
744+
`git diff --diff-filter=A` against the base branch) with video recording
745+
enabled and deploys the resulting HTML reportPlaywright's report embeds
746+
the video inline per testto the same `comfyui-playwright-chromium`
747+
Cloudflare Pages project, on its own branch. The link is posted alongside
748+
the other browser report links in the PR comment. It's scoped to added
749+
files onlythe existing suite always runs without videoto keep CI cost
750+
and time bounded. Recording is opt-in via `RECORD_VIDEO=true` (set only by
751+
that CI job; see `playwright.config.ts`) and slowed down with `SLOW_MO` so
752+
the result is legible for reviewers.
753+
741754
## After Making Changes
742755

743756
- `pnpm typecheck:browser` after modifying TypeScript in this directory.

playwright.config.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ const maybeLocalOptions: PlaywrightTestConfig = process.env.PLAYWRIGHT_LOCAL
1818
retries: process.env.CI ? 3 : 0,
1919
workers: process.env.CI ? 2 : undefined,
2020
use: {
21-
trace: 'on-first-retry'
21+
trace: 'on-first-retry',
22+
video: process.env.RECORD_VIDEO === 'true' ? 'on' : undefined,
23+
launchOptions: {
24+
slowMo: Number(process.env.SLOW_MO) || 0
25+
}
2226
}
2327
}
2428

scripts/cicd/pr-playwright-deploy-and-comment.sh

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,17 @@ else
249249

250250
# Clean up temp directory
251251
rm -rf "$temp_dir"
252-
252+
253+
# Deploy the focused new-tests report (with embedded video), if the PR
254+
# added spec files (see playwright-video-new-tests in ci-tests-e2e.yaml).
255+
# Reuses the chromium project on its own branch so it doesn't clobber the
256+
# main chromium report deployed above.
257+
new_tests_url=""
258+
if [ -d "reports/playwright-report-new-tests" ]; then
259+
echo "Found new-tests report, deploying..."
260+
new_tests_url=$(deploy_report "reports/playwright-report-new-tests" "chromium" "${cloudflare_branch}-new-tests")
261+
fi
262+
253263
# Calculate total test counts across all browsers
254264
total_passed=0
255265
total_failed=0
@@ -399,10 +409,15 @@ $test_line"
399409
i=$((i + 1))
400410
done
401411
unset IFS
402-
412+
413+
if [ -n "$new_tests_url" ] && [ "$new_tests_url" != "failed" ]; then
414+
comment="$comment
415+
- **New-test walkthrough** (chromium, recorded video): [View Report](${new_tests_url})"
416+
fi
417+
403418
comment="$comment
404419
405420
</details>"
406-
421+
407422
post_comment "$comment"
408423
fi

0 commit comments

Comments
 (0)