Make MockHTTPFetcher registration atomic and fix drained-queue reporting - #37
Merged
Merged
Conversation
|
| Name | Type |
|---|---|
| @germ-network/germ-convenience | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
germ-mark
added a commit
to germ-network/oauth4swift
that referenced
this pull request
Jul 28, 2026
GermConvenience closes the invariant hole by making request private(set) and body let, so the three sites that assigned header fields in place move to the new API. Behaviour is unchanged - each one only ever added a header. Companion to germ-network/GermConvenience#37; needs that released first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
on() parked the url and method on the actor for enqueue() to read, so the pair was not atomic: two chains configuring one fetcher concurrently could enqueue a response against each other's url. Measured 24% misattribution with two tasks under load and ~97% with eight. on() now returns a Registration carrying the url and method, so the race is unrepresentable rather than unlikely, and Errors.missingOnUrl is gone with it. A drained exact-method queue with no .any handler to fall back to reported unmockedRequest, which reads as "never mocked" - the exact-to-any fallback made tooManyRequests structurally unreachable for exact keys. Registered-but-drained now reports tooManyRequests for either key. The intentional fallback to .any is unchanged. Requests that matched no handler are now logged too, matching the tooManyRequests case, so requests(for:) is a complete record of what the code under test sent. Registration urls go through the same round trip lookup uses, so on(origin) matches rather than keying a url no request can produce. Also picks up the two implicit returns from review, and corrects the changeset, which still named the target GermConvenienceTesting after the rename to GermConvenienceMocks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
germ-mark
force-pushed
the
llm/mock-fetcher-fixes
branch
from
July 29, 2026 06:17
9a7f9bf to
6672ac8
Compare
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
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.
Suggestions into #32 — targets
feat/testing-mocks, so the diff here is one commit. Merge or cherry-pick as you like.Scoped to
GermConvenienceMocksonly. The core-library changes that were previously here are split out into #38, which targetsmainindependently.on()/enqueue()was not atomicon()parkedcurrentUrl/currentMethodon the actor andenqueue()read them back. Those are two separate actor messages, so a second chain could overwrite the cursor in between — responses land under the wrong URL, or a URL ends up with an empty queue and throwsunmockedRequestlater, far from the cause.It is not theoretical. Measured against the current implementation:
Load-dependent flakiness inside a testing library is the worst kind, because it gets blamed on the code under test.
on()now returns aRegistrationvalue carrying the url and method, andenqueue()hangs off that. The cursor is gone, so the race is unrepresentable rather than merely unlikely — andErrors.missingOnUrlbecomes impossible and is removed. Call sites are unchanged apart from droppingtry, sinceenqueueno longer throws:A drained exact queue reported "never mocked"
Because the exact-to-any fallback switches keys as soon as an exact queue empties,
requestCount > handlerCountwas structurally unreachable for an exact key. So over-requesting an exact-only mock threwunmockedRequest— indistinguishable from a URL you never registered — andtooManyRequestsonly ever fired for.anyhandlers. The more specific registration produced the vaguer error.Registered-but-drained now reports
tooManyRequestsfor either key. The exact-to-any fallback itself is unchanged —exhaustedExactHandlerFallsBackToAnyshows it is deliberate, so this only covers the case where there is no.anyhandler to fall back to.Smaller
on(origin)never matched. Lookup keys offrequest.request.url, whichHTTPRequesthas round-tripped through its pseudo-header fields — that rewrites a bare origin's empty path to/. Registration now goes through the same round trip, soon(URL("https://as.example"))matches a request to that origin instead of keying a url no request can produce.tooManyRequestsrejections already were, sorequests(for:)is a complete record of what the code under test sent. Shout if you would rather the log only contain matched requests — the asymmetry was the actual problem, either direction fixes it.GermConvenienceTestingafter the target was renamed toGermConvenienceMocks; that would have shipped into the CHANGELOG.requests(for:)includes rejected requests whileallRequestedcounts only those that consumed a response.Test notes
19 tests, clean build, no warnings. All 8 of your existing tests pass unmodified except for dropping the redundant
try. Added: the drained-exact case, unmocked-request logging, bare-origin registration, over-request-then-enqueue, and a 16-task concurrent configuration test that ran 20/20 clean.One coordination note: #38 moves
extension BundledHTTPRequest: Equatableonto the type itself. Whichever lands second, this PR's copy inMockHTTPFetcher.swiftshould be deleted — it is marked with a comment.🤖 Generated with Claude Code