Skip to content

test(remote-connection): extract shared fixtures from the connection suite - #2243

Merged
thymikee merged 1 commit into
mainfrom
claude/test-fixtures-remote-connection
Sep 2, 2026
Merged

test(remote-connection): extract shared fixtures from the connection suite#2243
thymikee merged 1 commit into
mainfrom
claude/test-fixtures-remote-connection

Conversation

@thymikee

@thymikee thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary

Extracts the repeated remote-connection test setup into a sibling fixture module while preserving every scenario, input, override, and assertion.

The suite remains at 50 tests and 213 assertions. The extraction reduces the test file from 2,973 to 2,696 lines and removes the obsolete manual size-pin adjustment now superseded by main's history-backed ratchet.

Validation

At 91fffd5f37b1017a7df94500d75f82df79739780:

  • pnpm vitest run --project unit-core src/__tests__/remote-connection.test.ts — 50/50 passed.
  • pnpm check:affected --run — formatting, lint, typecheck, fallow, and related tests all passed.
  • Fresh-worktree pnpm install --frozen-lockfile and pnpm build completed successfully.
  • Rebased onto current main; the obsolete size-pin-only commit was dropped.

Touched scope: two test files; no production, runtime, package, documentation, or device-facing behavior.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 2.58 MB 2.58 MB 0 B
JS gzip 868.4 kB 868.4 kB 0 B
npm bundled raw 2.58 MB 2.58 MB 0 B
npm bundled gzip 868.4 kB 868.4 kB 0 B
npm tarball 1.02 MB 1.02 MB +1 B
npm unpacked 3.46 MB 3.46 MB 0 B
npm clean-installed 3.46 MB 3.46 MB 0 B

npm unpacked components

Component Base Current Diff
JS / dist source 2.74 MB 2.74 MB 0 B
Apple runner source/project 544.4 kB 544.4 kB 0 B
Apple snapshot presentation source 33.8 kB 33.8 kB 0 B
macOS helper source 54.8 kB 54.8 kB 0 B
Android helper artifacts 43.3 kB 43.3 kB 0 B
Other package files 46.3 kB 46.3 kB 0 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.2 ms 29.8 ms +0.6 ms
CLI --help 77.1 ms 78.5 ms +1.4 ms

Top changed chunks: no changes in the largest emitted chunks.

Top changed packed files

No changed packed files.

@thymikee

thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

CODE REVIEW CLEAN, CI NOT READY at this exact head. The fixture extraction preserves client/Metro/state defaults and per-scenario overrides, keeps all 50 tests and 213 assertions, materially reduces duplication, and the test family shrinks overall. No code finding. Linux Smoke failed only because apt dependency installation hit the six-minute action timeout before checkout/build/product tests ran; rerun or otherwise clear that infrastructure failure before readiness labeling. The PR body’s full affected gate is also still pending and its local evidence predates the final head.

@thymikee

thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Updated exact-head status: code review remains clean at b5d4097, but the branch is not ready. Current main replaced the manual test-file pin map with the history-backed ratchet, so rebase onto current main, keep main's ratchet, and drop this PR's now-obsolete pin-lowering change. Then rerun exact-head CI. The Linux Smoke failure is infrastructure-only: apt dependency installation hit the exact six-minute timeout before checkout, build, or product tests ran.

…suite

The connection suite repeated its setup inline: the fake AgentDeviceClient,
the Metro prepare reply, the temp state directory, and the persisted
connection state literal with its version, profile hash, and timestamps.
Those move to a sibling fixtures module as named exports; every scenario
keeps its title, its inputs, and its own assertions.

jscpd (--min-tokens 80 --min-lines 8): 535 -> 211 duplicated lines,
27 -> 10 clones. Test count 50 -> 50, assert calls 213 -> 213.
@thymikee
thymikee force-pushed the claude/test-fixtures-remote-connection branch from b5d4097 to 91fffd5 Compare September 2, 2026 19:16
@thymikee

thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Branch blocker resolved at 91fffd5f37b1017a7df94500d75f82df79739780:

  • Rebased onto current main.
  • Kept main's history-backed test-file size ratchet.
  • Dropped the obsolete size-pin-only commit; the fixture extraction is the sole PR commit.
  • Remote-connection suite: 50/50 passed.
  • pnpm check:affected --run: all runnable checks passed.

Exact-head GitHub CI is now authoritative; the earlier Linux setup timeout belongs to the superseded head.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 2, 2026
@thymikee

thymikee commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

READY at 91fffd5f37b1017a7df94500d75f82df79739780. The branch is rebased cleanly onto current main; the obsolete size-ratchet pin commit was dropped while preserving the scoped fixture extraction. Local affected validation passed and every exact-head GitHub check is green. ready-for-human is applied.

@thymikee
thymikee merged commit d04e213 into main Sep 2, 2026
17 checks passed
@thymikee
thymikee deleted the claude/test-fixtures-remote-connection branch September 2, 2026 19:32
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-02 19:32 UTC

thymikee added a commit that referenced this pull request Sep 3, 2026
…suite (#2252)

Second pass after #2243: the connect --force family, the proxy open/install
pair, and the provider-allocation, persistence-failure, and proxy-disconnect
scenarios build their setup from writeReplacedProfiles, seedPreviousConnection,
forceConnectFlags, recordedLeaseRelease, and recordedLeaseAllocate. Titles and
assertion lines are unchanged; 50 tests before and after.
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