Skip to content

ci: a test-only change under studio or player needs no captures - #4956

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/ci-captures-skip-tests
Oct 3, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/ci-captures-skip-tests

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What

The Studio and player captures check (scripts/check-pr-captures.mjs) asked for Before and After screenshots on PRs that only change tests under packages/studio or packages/player. It already skipped markdown there as "docs, not behaviour"; tests are the same kind of file. Paths under a tests/ or __tests__/ directory and files named *.test.* or *.spec.* are no longer watched, so a test-only diff passes with nothing to show, and a test file no longer spends the "No visible change" line budget.

Why

#4953 changes only the edit accuracy bench harness (packages/studio/tests/e2e/edit-accuracy/). The check counted its 44 lines as a Studio change and failed it, although nothing a user sees changes.

Related work

Refs #4953.

How

One test-path pattern next to DOC_FILE, applied in isWatched, the single filter every diffed path goes through (parseNumstat). The old TEST_FILE pattern only exempted *.test|spec.[jt]sx? from the visual-file rule (it missed .mjs and the tests/ tree); it goes, since a test path never reaches that rule now. The existing case that passed a .test.tsx straight to evaluate now goes through parseNumstat, as real runs do.

Test plan

  • New case "test files under a watched package need no capture": a bench .mjs under tests/, a .spec.tsx and a .test.ts are filtered out and an empty body passes. Fails on main (all four files kept), passes here.
  • node --test scripts/check-pr-captures.test.mjs: 75 pass, 0 fail, 3 runs in a row.
  • The script against test(studio): the edit accuracy bench uses the port its server bound #4953's real diff with an empty body: main asks for Before and After; this branch passes.

No visible change

CI script only.

Size

One pattern and one test case.

Tests are not behaviour a user sees, like the markdown the check already skips.
A bench-harness PR (#4953) was asked for Before and After screenshots.
@miguel-heygen
miguel-heygen marked this pull request as ready for review October 3, 2026 15:57
@github-actions

github-actions Bot commented Oct 3, 2026

Copy link
Copy Markdown

Edit accuracy: accurate 1216 (base branch 1216), smooth 1108 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (1)

@terencecho terencecho left a comment

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.

Approved at 67a14da3aaee189e3b663f9ef1c03a689106f68a on code merit. The captures gate now excludes nested test/spec files and Markdown from the watched Studio/player diff before applying the visual-change and media rules; a mixed test-plus-production diff still enforces those rules. The added tests cover nested bench/test paths, Markdown, and the mixed production case, while existing visual rejection remains tested. I checked the exact-head script and workflow wiring; all 11 required checks passed, including captures and test reachability. I reviewed source via the GitHub API and did not run local tests.

— Review by tai (pr-review)

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 3, 2026
Merged via the queue into main with commit 5561b8c Oct 3, 2026
81 checks passed
@miguel-heygen
miguel-heygen deleted the fix/ci-captures-skip-tests branch October 3, 2026 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants