testing: event-pump tests on stub HttpClient + TestClock (comms-e5vm.2) - #12
Merged
Merged
Conversation
Tier-2 proof: migrate the event-pump / long-poll LOGIC tests off the real
Bun.serve realm onto the owned-fake stub HttpClient driven by TestClock,
exercised through the full adapter stack. Deterministic, no socket.
- Extend the stub seam with a hang ({ hang: true }) response variant that
parks on Effect.never. An instant in-memory stub has no long-poll *hold*,
so an eager Stream.runDrain would burn through the whole response sequence
in a hot loop; the hang models the hold (and stops one iterator racing
ahead into another's BAD_EVENT_QUEUE_ID). The downstream Tier-2 migrations
(.3/.7/.8) drive the same stack and get it from the shared seam.
- New packages/zulip/adapter-events.test.ts on stub + TestClock via
effectTest: gap-replay watermark across iterator instances (comms-4au),
429 RATE_LIMIT_HIT recovery on /events and /register (comms-9wi),
BAD_EVENT_QUEUE_ID reconnect + inbox.replay() gap-backfill (comms-jnn),
cursor/heartbeat advancement, and the scope-close fiber-interrupt (spj3.8).
- Delete the migrated tests and the happy-path live-contract duplicates
(first .next, mention-received) from adapter.test.ts. The rest of
adapter.test.ts stays on the realm for comms-e5vm.3.
Dissolves the real-socket contention flake structurally: the hanging
long-poll tests leave the realm-bound file, so comms-hbm9 auto-resolves.
Tier-3 residue for comms-4lz5: the genuine AbortSignal -> fetch -> TCP
teardown of an in-flight FetchHttpClient long-poll on scope close stays one
real-socket test; the stub only proves the Effect fiber-interrupt logic.
This was referenced Jun 13, 2026
GraemeF
added a commit
that referenced
this pull request
Jun 13, 2026
#14) Tier-2 migration (epic comms-e5vm). Moves `packages/zulip/bot-dm-guard.test.ts` off the real-socket `Bun.serve` realm (`startTestRealm` + `FetchHttpClient`) onto the owned-fake **stub HttpClient** via `effectTest`, following the pattern proved in comms-e5vm.2 (#12). ## What changed The bot-dm-guard wrapper's job is a *decision*: intercept `POST /messages` with `type=private` and reject when every non-self recipient is a bot, otherwise forward. The old tests wired the inner `ZulipHttp` against an in-process `Bun.serve` realm purely to give the wrapper a forward target. They now build that inner `ZulipHttp` over `makeStubHttpClient().client` instead — no socket, deterministic. - `buildHttp(stub)` provides `stub.client` for `HttpClient.HttpClient`. - Reject cases need no canned response — the guard short-circuits before `inner.post`. - Allow / pass-through cases register `stub.respond(...)`; assertions count `stub.captured`. - No `TestClock`: this file has no timing/retry, so `effectTest` takes no `layer`. ## On deletion The epic says happy-path cases that *merely duplicate the live contract* get deleted, not migrated. **None here qualify** — all 12 are guard-wrapper logic (bot↔bot rejection, the allow-branch decisions, parse-failure rejects, GET/DELETE/non-`/messages` pass-through transparency). The live contract can't even express bot↔bot rejection, and the allow-cases assert the guard's *decision to forward*, not that the forward succeeds. So all 12 migrate; none are deleted (deleting any would reduce guard coverage). ## Out of scope Other Tier-2 files (`adapter.test.ts` comms-e5vm.3, `http.test.ts` comms-e5vm.8), the Tier-3 real-socket teardown test (comms-4lz5), and the stateful-fake deletion (comms-e5vm.6). `startTestRealm` / `registerRealmHooks` remain — still consumed by the not-yet-migrated files.
GraemeF
added a commit
that referenced
this pull request
Jun 13, 2026
The Tier-3 residue of the hexagonal test-architecture epic (comms-e5vm): the **one** assertion in the suite that genuinely cannot leave the socket — that interrupting an in-flight \`FetchHttpClient\` long-poll on scope close actually tears down the underlying TCP connection (\`AbortSignal → fetch → socket\`). ## Why this is the only surviving socket test comms-e5vm.2 (#12) moved every event-pump **LOGIC** test — gap-replay, 429 retry, BAD_EVENT_QUEUE_ID reconnect, and the Effect **fiber-interrupt** path of scope close — onto the owned-fake stub HttpClient + TestClock: deterministic, no socket. The stub proves the *fiber* unwinds. It cannot prove the *socket* unwinds, because there is no socket. This test closes exactly that gap and is the only surviving \`Bun.serve\` long-poll in the suite. ## The assertion is server-side A real \`Bun.serve\` whose long-poll handler parks forever but listens on the request's \`req.signal\`. When the client tears the connection down, Bun fires that abort — observing it is proof the teardown reached the peer, not merely that our Effect fiber unwound (which the stub already covers). The Effect platform client wires this end to end: \`httpClient.make\` aborts the request's \`AbortController\` \`onInterrupt\`, and \`FetchHttpClient\` passes that \`signal\` into \`fetch\`. Lifecycle is \`Effect.acquireRelease\`/\`Scope\` throughout: the server's release is \`server.stop(true)\`, and the long-poll is forked into an inner \`Effect.scoped\` whose close is the interruption under test — so release is guaranteed even on the interruption path. The proof is timeout-guarded, so a non-teardown fails loud (verified by a mutation: detaching the fiber from the scope makes the test time out). ## Broader-rework question (comms-4lz5's original scope): recommend **leave-as-is** The bead also asked whether to migrate the ~170 \`realm.\`/\`fixture.\` hook call sites to a per-test \`Effect.scoped\` wrapper. That rework was motivated by a contention-driven teardown-starvation flake in the infinite-long-poll tests — which .2 dissolved structurally by moving that logic off the socket. Every fixture still wired through \`registerRealmHooks\` is now a plain request/response realm that starts and stops in a few ms; there is no teardown-starvation mode left to fix, and the \`acquireRelease\`/\`Scope\` shape now lives exactly where it is load-bearing (this one socket test). The migration would be churn across ~170 sites for no behavioural payoff — YAGNI. This PR also refreshes the now-stale \`realm-hooks.ts\` doc comment that still described the dissolved failure mode. Closes comms-4lz5.
This was referenced Jun 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tier-2 proof for the hexagonal test-architecture epic: migrate the event-pump / long-poll LOGIC tests off the real
Bun.serverealm onto the owned-fake stub HttpClient driven by TestClock, exercised through the full adapter stack (adapter → ZulipHttp → HttpClient). Deterministic, no socket. This establishes the pattern the later per-file migrations (.3/.7/.8) follow.What changed
Stub seam gains a hang variant (
packages/testing/stub-http-client.ts): a{ hang: true }response parks onEffect.never. An instant in-memory stub has no long-poll hold, so an eagerStream.runDrainwould burn through the whole canned sequence in a hot loop — the hang models the hold (and stops one iterator racing ahead into another'sBAD_EVENT_QUEUE_ID). The downstream Tier-2 migrations drive the same stack and inherit it from the shared seam. Minimal, additive; the request is still captured before parking, and a parked fiber is interrupted (not errored) on scope close.New
packages/zulip/adapter-events.test.tson stub + TestClock viaeffectTest:events()iterator instances (comms-4au)RATE_LIMIT_HITrecovery on/eventsand/register(comms-9wi) — the rate-limit backoff now sleeps on the virtual clockBAD_EVENT_QUEUE_IDreconnect +inbox.replay()gap-backfill (comms-jnn)last_event_idcursor + heartbeat advancementDeleted from
adapter.test.ts: the migrated tests plus the happy-path live-contract duplicates (first .next(),mention-received). The rest ofadapter.test.tsstays on the realm — that migration is comms-e5vm.3.Flake dissolved
The hanging long-poll tests were the real-socket contention source: their
before/afterEachrealm-hook teardown raced an in-flight hung long-poll. Moving them off the socket dissolves it structurally — comms-hbm9 auto-resolves once this lands.Tier-3 residue (for comms-4lz5)
The stub proves the Effect fiber-interrupt logic. The genuine
AbortSignal → fetch → TCP teardownof an in-flightFetchHttpClientlong-poll on scope close cannot move off the socket — that stays exactly one real-socket Tier-3 test (comms-4lz5).Verification
bun run checkgreen inside the nix CI shell: typecheck + biome + 870 tests + hermes gate. The migrated suite was run 5× to confirm determinism and pristine output (no stray transient/error logs).