Skip to content

refactor(capture-kit): colocate fifteen tests with the modules they test - #3654

Merged
thymikee merged 2 commits into
mainfrom
refactor/colocate-capture-kit-tests
Oct 15, 2026
Merged

thymikee merged 2 commits into
mainfrom
refactor/colocate-capture-kit-tests

Conversation

@thymikee

@thymikee thymikee commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Summary

Colocation renames for capture-kit: 15 test files that were not beside a module they are named for. Two fixtures follow the tests that use them. No fixture export is dropped here (the remaining capture-kit fixture exports need test-kit and come later); the only package manifest edit is the new target path of the snapshot-presentation-ios-transitions-fixtures export.

The rule thread's colocation baseline and R83 list are not on main yet. This PR only shrinks what the seed will contain: capture-kit 15 → 0 colocation offenders.

Commits

  1. refactor(tests): name capture-kit tests for the module they test — pure git mv. Content changes are import specifiers only (self-package and barrel imports become the defining module; ../snapshot-freshness/ and ../../../ios-snapshot-engine/ prefixes drop), plus references to the old paths (macOS workflow, ADR 0026, a comment in projection.ts). oxfmt also collapsed one import in recovery.test.ts onto a single line.
  2. refactor(capture-kit): split the host-support checks out of the recording scripts test — the only non-move.

Renames

Old New Module under test
capture-admission/__tests__/screen-recording-boundary-faults durable-capture-resource-boundary-faults durable-capture-resource.ts (createDurableCaptureResource)
ios-snapshot-engine/conformance engine-conformance engine.ts
ios-snapshot-engine/eligibility-parity engine-eligibility-parity engine.ts
ios-snapshot-engine/properties engine-properties engine.ts
ios-snapshot-engine/mapping semantic-index-mapping semantic-index.ts
ios-snapshot-engine/presentation semantic-index-presentation semantic-index.ts
snapshot/__tests__/ax-digest snapshot-evidence-ax-digest snapshot-evidence.ts (computeAxDigest)
snapshot/__tests__/snapshot-actions-identity snapshot-diff-actions-identity snapshot-diff.ts
snapshot/snapshot-presentation/warnings quality-warnings quality-warnings.ts
snapshot-sibling-sized-chrome snapshot-occlusion-sibling-sized-chrome snapshot-occlusion.ts (its header already calls it a family of snapshot-occlusion.test.ts)
snapshot/__tests__/snapshot-freshness snapshot/snapshot-freshness/android.test.ts snapshot-freshness/android.ts
snapshot/__tests__/snapshot-freshness-recovery snapshot/snapshot-freshness/recovery.test.ts snapshot-freshness/recovery.ts
snapshot/snapshot-presentation/ios/action-shelf ios-snapshot-engine/action-shelf.test.ts ios-snapshot-engine/action-shelf.ts (behavior reached through buildIosInteractiveSnapshotPresentation in semantic-index.ts)
snapshot/snapshot-presentation/ios/rows ios-snapshot-engine/rows.test.ts ios-snapshot-engine/rows.ts (same)
recording/__tests__/recording-scripts split: swift-cache-recording-scripts, overlay-host-support, contact-sheet-frames-host-support swift-cache.ts (it locates the Swift scripts), overlay.ts, contact-sheet-frames.ts

The engine-* tests previously imported through ios-snapshot-engine/index.ts; they now import engine.ts / types.ts directly. rows.fixtures.ts and transitions.fixtures.ts moved with their tests (the old snapshot-presentation/ios/ directory is gone); the transitions fixture is still exported for one root test and is a later test-kit candidate.

Judgment call to check: swift-cache-recording-scripts type-checks and exercises the Swift sources under apple/runner/.../RecordingScripts; swift-cache.ts is the TS module that owns that directory, so I named it for that. If you'd rather count it as a non-module test, say so and I'll leave it out of the baseline instead.

Test-count parity (vitest --reporter=json, passed/total)

Before After
screen-recording-boundary-faults 4/4 durable-capture-resource-boundary-faults 4/4
conformance 8/8 engine-conformance 8/8
eligibility-parity 4/4 engine-eligibility-parity 4/4
properties 5/5 engine-properties 5/5
mapping 1/1 semantic-index-mapping 1/1
presentation 21/21 semantic-index-presentation 21/21
ax-digest 10/10 snapshot-evidence-ax-digest 10/10
snapshot-actions-identity 4/4 snapshot-diff-actions-identity 4/4
warnings 17/17 quality-warnings 17/17
snapshot-sibling-sized-chrome 25/25 snapshot-occlusion-sibling-sized-chrome 25/25
snapshot-freshness 10/10 snapshot-freshness/android 10/10
snapshot-freshness-recovery 5/5 snapshot-freshness/recovery 5/5
action-shelf 3/3 action-shelf 3/3
rows 2/2 rows 2/2
recording-scripts 8/8 swift-cache-recording-scripts 6/6 + overlay-host-support 1/1 + contact-sheet-frames-host-support 1/1
127/127 in 15 files 127/127 in 17 files

Selection: all paths are under packages/*/src/**, which the unit-core include covers. Because packages/capture-kit/package.json changed, check:affected --run fell back to the full test set, so every moved file ran in context (15,485 tests passed).

vi.mock effectiveness

None of the moved files use vi.mock (the only vi.mock in the touched directories is in overlay.test.ts, which did not move).

Surface

  • exports counts before → after: capture-kit 83 → 83. The snapshot-presentation-ios-transitions-fixtures export keeps its key and points at the moved file; no subpath added or removed, no barrels.
  • Colocation offenders: capture-kit 15 → 0.

Gates

pnpm check:quick, check:layering, check:fallow, check:gate-manifest pass; pnpm format leaves a clean tree. pnpm check:affected --run ran the full set: 15,485 passed, 1 failed — scripts/__tests__/eager-closure-budgets.test.ts > no APPROVED_OVER_CEILING row is stale (packages/daemon/src/index.ts). That failure is not from this change: it fails identically on a checkout of origin/main plus only the PR #3648 renames (no daemon file touched), so it comes from the daemon single-entry work already on main (#3633). The daemon thread owns the fix.

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

@cubic-dev-ai cubic-dev-ai Bot 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.

No issues found across 23 files

View guided diff | Turn on auto-fix | Re-trigger cubic

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Size Report

Metric Base Current Diff
Installed (including dependencies) 5.25 MB 5.25 MB 0 B
Package (unpacked) 5.25 MB 5.25 MB 0 B
Package (download) 1.58 MB 1.58 MB +8 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 26.5 ms 26.8 ms +0.3 ms
CLI --help 75.8 ms 75.9 ms +0.1 ms

@thymikee

Copy link
Copy Markdown
Member Author

This PR is ready at 90ce86d. Not blocking: the rename to engine-eligibility-parity.test.ts left the old bare name in two lines of docs/adr/0026-scroll-clip-authority.md (59 and 135) and in the Swift comment at SnapshotPresentationProjection.swift:5, so update those if you like, or leave them. The diff touches no production code, and I read it against the merge-base diff without running vitest, check:package or check:layering, so the test-count parity is from the PR body. CI is still queued or running with no failure log, and you report that eager-closure-budgets also fails on main; I did not reproduce that. No conflicts are known. Before merge, CI needs to finish and the eager-closure-budgets failure needs to be fixed on main or confirmed as pre-existing.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 15, 2026
thymikee and others added 2 commits October 15, 2026 14:42
Pure moves and renames. Import changes are specifier-only: self-package and
barrel imports become the defining module, and the moved freshness and action-shelf
tests drop their cross-directory prefixes. The transitions and rows fixtures move
with the tests that use them; the transitions fixture export follows its file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ding scripts test

The overlay and contact-sheet unsupported-host checks cover overlay.ts and
contact-sheet-frames.ts; they now sit beside those modules' tests. The Swift
typecheck and overlay export cases stay in swift-cache-recording-scripts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thymikee
thymikee force-pushed the refactor/colocate-capture-kit-tests branch from 90ce86d to 991ab1a Compare October 15, 2026 12:42
@thymikee
thymikee merged commit 0605d8c into main Oct 15, 2026
22 checks passed
@thymikee
thymikee deleted the refactor/colocate-capture-kit-tests branch October 15, 2026 13:37
@thymikee

Copy link
Copy Markdown
Member Author

The earlier review of 90ce86d was clean, and the code at 991ab1a is still clean. The diff only moves tests, edits one fixture export path and updates one workflow test path. The export names are unchanged and the new paths resolve.

The Smoke Tests, Mutants, Compatibility & Provenance and Coverage jobs fail. I could not read their logs, so I can't say why they fail. Nothing in this diff points at them, but Coverage and Compatibility could touch moved tests or the manifest. I did not run vitest, check:package or check:layering, and I did not read the one-line projection.ts hunk. Please confirm the four failing jobs are unrelated, or share the log lines if they are not. No other change is needed before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant