test(mesh-store): direct unit tests for MeshStore's own orchestration logic - #126
Merged
Merged
Conversation
MeshStore's constructor takes no injectable deps (unlike its extracted collaborators), and its own orchestration-level logic -- requireTransport/ requireIdentity's guard errors, the connected getter, init()'s connect-vs- becomeCoordinator-vs-EADDRINUSE branching, the events getter's dispatch table, federation/listener/room-join-approval passthroughs, and shutdown() -- was previously exercised only indirectly through wireTestTransport-based integration tests, which don't isolate MeshStore's own branching from its collaborators' real behaviour. Uses a hand-built fake MeshTransport passed to the real setTransport() for deterministic control over init()'s branches, and reaches the private roomProtocol/connectionApproval/peerLifecycle/deliveryEngine/staleAgentChecker collaborators via a narrow cast to assert a delegating wrapper actually calls through -- TypeScript's private is compile-time only, and this is exactly the kind of whitebox check a one-line pass-through method needs that no public-API-only test can express. Covers every currently-open survivor/timeout this file's mutation score reports except the events getter's onMessage/onPeerConnected dispatch pair sharing a mutant with an already-covered branch, and getFederationFingerprint (no test infrastructure for a real TLS identity certificate exists yet).
…aced Verified against a real run rather than trusting the prior commit's own unverified claim about what remained (which named the wrong survivors entirely -- neither of the two it named was actually still open). The real gaps: constructor wiring for both discovery backends (observable via discovery.advertise's own "Available:" error message), the getOnPatch closure passed to DeliveryEngine (observable via a real broadcastPatch call), ConnectionApproval's queueDelivery closure and MeshStore.deliver()'s own delegation (both observable via drainDelivery picking up the queued event), and shutdown()'s pending-markRead-timer clearing loop. Two survivors remain genuinely open, left for follow-up rather than forced: FederationManager's own meshName string (needs a real two-way federation connection to observe, the kind of setup federation.integration.test.ts already has but doesn't currently assert this specific field against), and a handful of Timeout-classified mutants inside init()'s branches and the events getter that this file's own new tests do correctly fail fast against, but where the whole command-runner suite still doesn't exit within budget for an unrelated reason -- timeouts already count as caught for scoring purposes, so these don't block the threshold, only slow the run.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…and isShutDown Verified against a fresh mutation run: the EADDRINUSE-degrade onError message assertion only checked its first half, missing a mutant on the second half's own string literal. shutdown()'s isShutDown = true flip had no assertion at all -- fixed via its one observable effect (fireLocalDelivery's own isShutDown() guard on scheduling further markRead timers) rather than reaching into the private field directly. One survivor remains genuinely open: FederationManager's meshName string, which needs a real two-way federation connection to observe -- left for follow-up rather than forced with a contrived unit test.
…ranch Verified against a fresh mutation run the previous commit's own isShutDown test still survived -- fireLocalDelivery returns before ever reaching the isShutDown-guarded timer push unless onDelivery is set, so the assertion never actually exercised the line it claimed to cover. Fixed by setting store.onDelivery, then confirmed directly: with this.isShutDown = true manually flipped to false in the source, the test now genuinely fails (pending length 1 instead of 0), and passes against the real code -- proof the fix actually distinguishes the two, not just that it was plausible.
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.
Summary
Closes #103
Test plan
pnpm typecheckpnpm lintpnpm test(65 files, 524 tests, full suite green)