Skip to content

fix(e2e): make visual-regression editor captures deterministic #2192

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

Merged
ascorbic merged 1 commit into from
Jul 22, 2026
+35 −14
Merged

fix(e2e): make visual-regression editor captures deterministic #2192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 24 additions & 14 deletions .github/workflows/visual.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,19 @@ jobs:

# Always record PR metadata so the Report workflow can upsert *or* clear
# the sticky comment. Candidate baselines + diff images are added only
# when the suite actually diffed (exit != 0 AND regeneration produced
# snapshots) -- an infra failure exits non-zero but yields no candidates,
# so we don't mislabel it as a visual change.
# when the regenerated baselines actually differ from the committed ones.
# A non-zero exit alone is not drift: a flaky render can fail the diff
# check on the first pass yet regenerate byte-identical baselines under
# --update-snapshots, and an infra failure exits non-zero without
# regenerating anything. Both leave the snapshots dir git-clean, so we
# decide drift from `git status`, not from the exit code.
- name: Collect result
id: collect
env:
PR_NUMBER: ${{ github.event.pull_request.number }}
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
RUN_EXIT: ${{ steps.run.outputs.exit }}
SNAP_DIR: e2e/tests/visual-regression.spec.ts-snapshots
run: |
mkdir -p visual-out
echo "$PR_NUMBER" > visual-out/pr-number
Expand All @@ -99,21 +103,27 @@ jobs:
# Regenerate the accepted-state baselines (the candidates the Apply
# workflow will commit verbatim on 👍). These are Linux/chromium PNGs.
pnpm exec playwright test visual-regression --update-snapshots --reporter=line || true
mkdir -p visual-out/candidates
if [ -d e2e/tests/visual-regression.spec.ts-snapshots ]; then
cp -r e2e/tests/visual-regression.spec.ts-snapshots/. visual-out/candidates/
fi

if [ -z "$(ls -A visual-out/candidates 2>/dev/null)" ]; then
# Non-zero exit but nothing regenerated -> infra failure, not drift.
# Real drift == the regeneration changed tracked baselines or added
# new ones. Untracked new files count (bootstrap: a PR that adds a
# page has no committed baseline yet). A clean tree means the failure
# did not correspond to a stable pixel difference -- treat it as flake
# or infra and keep the check green with a warning.
if [ -z "$(git status --porcelain -- "$SNAP_DIR")" ]; then
echo "false" > visual-out/has-drift
echo "has_drift=false" >> "$GITHUB_OUTPUT"
echo "::warning::Visual suite failed without producing candidate baselines (likely an infra failure, not a UI diff)."
else
echo "true" > visual-out/has-drift
echo "has_drift=true" >> "$GITHUB_OUTPUT"
echo "Visual drift detected — candidates and diff images staged for the Report workflow."
echo "::warning::Visual suite exited non-zero but regenerated baselines are identical to the committed ones (flaky render or infra failure, not a UI diff)."
Comment on lines +111 to +115

@emdashbot emdashbot Bot Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] The git-clean branch now treats every non-zero exit without baseline changes as flake/infra and exits 0. That correctly handles the documented flaky-render case, but it also makes the measure job green for genuine failures that prevent screenshots from being generated in the first place (e.g., a JS error or navigation timeout on a screen, which also leaves the snapshots tree unchanged). The ::warning:: is visible in the job log, but the report workflow will delete or skip the sticky comment, so there's no durable signal for maintainers.

Consider preserving test-results/ (logs, traces, or at least the Playwright output) in the artifact on this path, so a failure that isn't drift can still be investigated without re-running the suite.

Suggested change
# or infra and keep the check green with a warning.
if [ -z "$(git status --porcelain -- "$SNAP_DIR")" ]; then
echo "false" > visual-out/has-drift
echo "has_drift=false" >> "$GITHUB_OUTPUT"
echo "::warning::Visual suite failed without producing candidate baselines (likely an infra failure, not a UI diff)."
else
echo "true" > visual-out/has-drift
echo "has_drift=true" >> "$GITHUB_OUTPUT"
echo "Visual drift detected — candidates and diff images staged for the Report workflow."
echo "::warning::Visual suite exited non-zero but regenerated baselines are identical to the committed ones (flaky render or infra failure, not a UI diff)."
if [ -z "$(git status --porcelain -- "$SNAP_DIR")" ]; then
echo "false" > visual-out/has-drift
echo "has_drift=false" >> "$GITHUB_OUTPUT"
echo "::warning::Visual suite exited non-zero but regenerated baselines are identical to the committed ones (flaky render or infra failure, not a UI diff)."
# Keep failure artifacts around even though we aren't reporting drift,
# so setup/screenshot-prevention failures remain inspectable.
cp -r test-results visual-out/test-results 2>/dev/null || true
exit 0
fi

exit 0
fi

# Stage the candidate baselines for the Report/Apply workflows.
mkdir -p visual-out/candidates
if [ -d "$SNAP_DIR" ]; then
cp -r "$SNAP_DIR/." visual-out/candidates/
fi
echo "true" > visual-out/has-drift
echo "has_drift=true" >> "$GITHUB_OUTPUT"
echo "Visual drift detected — candidates and diff images staged for the Report workflow."

- name: Upload artifact
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
Expand Down
11 changes: 11 additions & 0 deletions e2e/tests/visual-regression.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,17 @@ async function openAdmin(admin: AdminPage, path: string, dir: string): Promise<v
/** Settle fonts and freeze animation before capturing. */
async function stabilize(admin: AdminPage): Promise<void> {
await admin.page.addStyleTag({ content: FREEZE_CSS });
// Drop focus before capturing. The editor pages mount a TipTap toolbar
// whose buttons reflect the editor's focus/active state; whether the editor
// grabs focus during hydration is a race, so a run can capture a focused
// (highlighted) toolbar button or an unfocused one. Blurring makes the
// capture depend on the rendered page, not on who won the focus race.
await admin.page.evaluate(() => {
const active = document.activeElement;
if (active instanceof HTMLElement && active !== document.body) {
active.blur();
}
});
// Await font loading without returning the FontFaceSet that
// document.fonts.ready fulfils with -- Playwright cannot serialize it.
await admin.page.evaluate(async () => {
Expand Down
Loading