Skip to content

test(daemon): split the two replay test files over the size tripwire - #2455

Merged
thymikee merged 2 commits into
mainfrom
claude/2446-replay-test-split
Sep 10, 2026
Merged

thymikee merged 2 commits into
mainfrom
claude/2446-replay-test-split

Conversation

@thymikee

@thymikee thymikee commented Sep 10, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #2353. No *.test.ts under src/daemon/** should sit over the 1,000-line tripwire, so the size ratchet can fail on new size, not only on growth (#2417 left these two). Split into sibling <base>-<concern>.test.ts files:

  • session-replay-divergence.test.ts (1,100) -> suggestions, chrome-filter, overlay, capture-policy, observation — 13 cases.
  • session-replay-runtime-maestro.test.ts (1,963) -> dispatch, run-script, targeting, swipe, keyboard, run-flow, shell-artifacts — 43 cases.

Divergence siblings mock inline (Vitest hoists a file's own vi.mock above its own imports, so the SUT loads under them), matching the existing session-replay-divergence-android-occlusion.test.ts sibling. The freshness-retry sleep stub is declared only in the two siblings that exercise a retry branch, so it stays local. Maestro keeps session-replay-runtime-maestro.fixtures.ts (24-line resolveTargetDevice + the runReplayFixture harness): Vitest allows one vi.mock per module per file, so the Maestro family cannot reuse session-replay-runtime.test.ts's differently-resolved device, and copying that mock + harness per file is what #2446 asked to avoid. 17 files touched, test-only.

Isolation tradeoff vs #2446. #2446 asked for a shared divergence fixtures module so a mock can't leak into unrelated siblings. A file's own inline mock cannot leak at all, and the divergence shared setup was only the 6-line dispatch+interactor mock plus a resetLegacySnapshotCapture call that already exists — extracting a fixtures module there forced a frozen-object SUT hand-off and a load-bearing import-order invariant, costing more than the leak risk it prevented. Maestro genuinely needs fixtures for the reasons above, so the two families diverge on purpose.

Validation

Tested at ba061f5 (final revision).

  • 56 test cases preserved (13 + 43) under src/daemon/replay/internal/__tests__; test titles and assertion multisets diffed identical vs main.
  • pnpm check:affected --run green (vitest-related, layering, mutation-registry, size ratchet — largest new file 605 lines).
  • pnpm lint, pnpm typecheck green.
  • Test-only change: no runtime/coverage/device behavior touched, so provider/coverage lanes are unaffected.

No daemon test file should sit over the 1,000-line tripwire, so the size
ratchet can fail on new size, not only on growth. Split the two files PR
#2417 left over into sibling `<base>-<concern>.test.ts` files:

- session-replay-divergence.test.ts -> suggestions, chrome-filter, overlay,
  capture-policy, observation (13 cases).
- session-replay-runtime-maestro.test.ts -> dispatch, run-script, targeting,
  swipe, keyboard, run-flow, shell-artifacts (43 cases).

Shared setup moves to a dedicated fixtures module per family:
- session-replay-divergence.fixtures.ts holds the device-resolution and
  snapshot-interactor mocks and the shared reset, and re-exports the SUTs so
  siblings load them under the mocks. The freshness-retry `sleep` stub lives in
  a separate retry fixtures module the two retry siblings import first, so it
  cannot no-op the retry delay in siblings that never retry.
- session-replay-runtime-maestro.fixtures.ts declares the Maestro
  device-resolution vi.mock once, so the split does not copy it per file.

The diff is moves, import paths and fixture extraction. The case count under
src/daemon/replay/internal/__tests__ is unchanged (13 + 43 = 56); no test is
deleted or skipped, and assertions are identical.

Closes #2446
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.53 MB 4.53 MB -246 B
Package (unpacked) 4.53 MB 4.53 MB -246 B
Package (download) 1.34 MB 1.34 MB -136 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 27.6 ms 27.3 ms -0.3 ms
CLI --help 78.2 ms 77.8 ms -0.4 ms

… indirection

The divergence split did not need a shared fixtures module. Its only shared setup was the
dispatch-resolve + interactor mock pair and a reset that legacy-snapshot-capture-fixture
already provides, so routing the SUTs back through a frozen object and import-order-first
fixtures bought nothing and left a load-bearing import invariant that a reordering could
silently break. Each sibling now declares its own vi.mock (Vitest hoists a file's own mock
above its own imports) and imports the SUT directly, matching the existing
session-replay-divergence-android-occlusion.test.ts sibling. The freshness-retry sleep stub
is declared only in the two siblings that exercise a retry branch, so it stays local.

Maestro keeps its fixtures: the 24-line resolveTargetDevice mock cannot be copied per file
(one mock per module per file forbids merging with session-replay-runtime.test.ts) and the
runReplayFixture harness is genuinely shared. 56 tests unchanged.
@thymikee

Copy link
Copy Markdown
Member Author

No test-behavior findings at ba061f5: all 56 test bodies are preserved, and the split keeps retry mocks isolated. Please refresh the summary and validation revision: the final diff has 17 files and file-local divergence mocks, not the shared divergence fixtures currently described; explain that isolation tradeoff against #2446. Completed checks are green, with iOS still running.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 10, 2026
@thymikee
thymikee merged commit c99b450 into main Sep 10, 2026
18 checks passed
@thymikee
thymikee deleted the claude/2446-replay-test-split branch September 10, 2026 13:14
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-10 13:15 UTC

@thymikee

Copy link
Copy Markdown
Member Author

Refreshed the summary/validation to the final revision (ba061f5, 17 files): divergence siblings now carry file-local mocks (Vitest hoists a file's own vi.mock above its imports) with the retry sleep stub confined to the two retry siblings, and only Maestro keeps a fixtures module. Added an Isolation tradeoff vs #2446 paragraph: inline mocks can't leak, and divergence's shared setup was too small to justify the frozen-object hand-off + import-order invariant the fixtures module required — Maestro keeps fixtures only because the one-mock-per-module rule and shared harness size make them load-bearing.

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