test: query-parameter drift check, table-driven endpoint tests, shared test kit #186

Grandiras · 2026-09-27T17:24:18Z

Grandiras commented

2026-09-27 17:24:18 +00:00

Part of #180 (pass E, part 1: Lexicon client tests).

What this does

Pass E's L2 (table-driven endpoint tests + query-parameter drift check) and L6
(test kit consolidation), scoped to tests/ATProtoNet.Tests/{Ozone,Chat/Bsky, Lexicon/App/Bsky,Lexicon/Com/AtProto,Lexicon/Site}. No src/ changes: the
drift check found no real bugs (see below). Test-only, no CHANGELOG entry.

L2(a): query-parameter drift check

Lexicon/Upstream/SdkSource.Parameters.cs walks the source the same way the
existing NSID/method scan does: it finds every var parameters = new XrpcParams()...; (or filter.ToParams(...)...;) statement, the nearest
following XRPC call that uses it, and the literal keys passed to .Add/
.AddAll in between — including keys added inside a conditional block or
chained after a ToParams() helper. LexiconDriftTests.ParamCalls_ KeysAreUpstreamParameters then checks every key against the upstream
Lexicon's parameters for that NSID, with a QueryParamAllowlist (currently
empty) for deliberate exceptions, checked for staleness like the other
allow-lists.

Coverage: 143 call sites, 477 keys, 0 failures. Verified the check actually
catches drift by temporarily renaming a key in GraphClient.GetFollowersAsync
(sort → sortXX) and confirming the test failed with app.bsky.graph. getFollowers:sortXX: not an upstream parameter, then reverting.

Findings: no real bugs. Every existing query-parameter key already
matches its upstream Lexicon; the allow-list is empty.

L2(b): table-driven theory for simple endpoints

Lexicon/EndpointRequestTests.cs is a [Theory] over 53 rows, each a
self-contained scenario: script a response, call the endpoint with every
argument, assert the exact HTTP method/NSID/query-or-body (via new HttpStub. AssertGet/AssertPost, proxy header included where the original test
checked one), and a light check that the response deserializes. It spans
chat.bsky.{actor,notification,group,convo,moderation}, app.bsky.{labeler, bookmark,notification,unspecced,embed,graph,draft,ageassurance},
com.atproto.{admin,temp}, tools.ozone.{queue,report,moderation, communication,team,set,server,signature,setting,safelink} and site. standard.*'s delete wrappers.

Removed test files (nothing left to keep once their tests moved to the table):

Chat/Bsky/Notification/ChatNotificationClientTests.cs (2 tests: getPreferences, putPreferences)
Lexicon/App/Bsky/Labeler/LabelerClientTests.cs (2 tests: getServices with/without detailed)

Removed test methods, by category (covered by the table row of the same NSID unless noted):

Ack-only calls (no interesting response, just a method/body/proxy check): chat.bsky.{actor.deleteAccount, group.withdrawJoinRequest, group.rejectJoinRequest, group.updateJoinRequestsRead, convo.updateAllRead, moderation.updateActorAccess}, app.bsky.{notification.updateSeen, graph.muteActor, draft.updateDraft, draft.deleteDraft, bookmark.createBookmark, bookmark.deleteBookmark, notification.unregisterPush},
com.atproto.temp.{requestPhoneVerification, revokeAccountCredentials}, tools.ozone.{queue.unassignModerator, report.unassignModerator, report.refreshStats, setting.removeOptions, safelink.removeRule},
site.standard.*'s 4 delete wrappers (publication/document/subscription/recommend).
Plain (non-union) lookups: chat.bsky.actor.getStatus, app.bsky.{labeler.getServices, ageassurance.begin, notification.listActivitySubscriptions, notification.putActivitySubscription, unspecced.getPostThreadV2's defaults-only case, embed.getEmbedExternalView's nothing-resolved case}, com.atproto.{admin.searchAccounts, admin.updateAccountSigningKey, temp.checkSignupQueue, temp.dereferenceScope}, tools.ozone.{queue.routeReports, queue.deleteQueue, moderation.getEvent, moderation.getRepo, moderation.queryEvents's plain form, moderation.getAccountTimeline, moderation.getReporterStats, communication.*, team.*, set.*, server.getConfig, signature.findRelatedAccounts}.
Merged near-duplicates: OzoneClientTests.FindRelatedAccounts_QueriesByDid and TypedOzoneClientTests. FindRelatedAccountsAsync_LimitThenCursor_SendsBoth covered the same endpoint from two files (did-only, then
limit+cursor); one row now covers did+limit+cursor.

What was kept, and why (per-row response checks would either lose real
coverage or duplicate a test that already exists for the right reason):

Union/polymorphic responses: OzoneModerationToolingTests (repo/record/subject/event variants),
OzoneClientTests.QueryStatusesAsync_ReadsSubjectStatuses (ConvoSubject/AccountHosting), ComAtProtoGapTests's
CheckHandleAvailabilityAsync_* (3 tests, the Available/Unavailable/Unknown result variants), TypedBskyClientTests,
TypedOzoneClientTests, TypedRepoClientTests (typed-identifier round-trips are these files' whole point),
BookmarkClientTests, UnspeccedClientTests, EmbedClientTests, GraphSurfaceTests (thread/embed/bookmark item
variants).
Pagination walks: every Enumerate*Async cursor-following test (Chat, Bsky, Ozone).
Error mapping: IdentityClientTests.ResolveIdentityAsync_HandleNotFound_*, TypedOzoneClientTests. GetRepoAsync_InvalidDidInTheResponse_*, TypedRepoClientTests.Response_WithAnInvalidIdentifier_*,
ComAtProtoGapTests.ImportRepoAsync_Rejected_*.
Special encoding / bespoke logic: ComAtProtoGapTests's 3 ImportRepoAsync_* stream tests
(content-length, stream position, chunked non-seekable), TypedBskyClientTests.PostAsync_ WritesACanonicalCreatedAt (timestamp canonicalization), VideoUploadTests.UploadVideoAsync_ BacksOffExponentially (its own XrpcClient — see below).
Regressions and cross-cutting checks, explicitly flagged in their own comments: OzoneClientTests. QueryStatusesAsync_Filter_* ("querySubjects never existed"), TypedBskyClientTests. ListNotificationsAsync_* ("seenAt is deliberately not a parameter"), ConvoClientTests. ProxyHeader_DoesNotAffectOtherXrpcCalls and ChatModerationClientTests.Calls_WithoutAClientWideProxy_*
(proxy-absence, not proxy-presence), FeedSurfaceTests.SearchPostsV2Async_SendsEveryFilterUnderItsLexiconName
(asserts the full upstream parameter set, bidirectionally — kept as-is rather than reduced to the table's
lighter check), WireFidelityTests (untouched, per its own docstring: regression tests for #123).

L6: test kit

Ozone/OzoneTestClient.cs (hand-rolled RecordingHandler + SentRequest) deleted; its 4 consumers
(OzoneQueueClientTests, OzoneReportClientTests, OzoneToolsClientTests, OzoneModerationToolingTests)
moved to XrpcTestClient.
The 8 hand-built HttpStub+HttpClient+XrpcClient trios named in the issue — Chat
Actor/Convo/Group/Moderation/Notification, Labeler, VideoUpload, StandardSite — moved onto XrpcTestClient,
reached through AtProtoClient's own Chat/Bsky/Site properties instead of constructing the Lexicon
client directly. Exception: VideoUploadTests.UploadVideoAsync_BacksOffExponentially keeps its inline
XrpcClient — AtProtoClient's internal test constructor accepts a TimeProvider but only wires it to
SessionManager, not to the XrpcClient's own retry delay, so routing this one test through it would silently
drop the injected clock. Flagged as a follow-up task (see below); out of scope for src/ here.
The 2 hand-built AtProtoClients named in the issue (TypedBskyClientTests, ComAtProtoGapTests) moved onto
XrpcTestClient. Found and fixed the same pattern in 2 more files in scope not named in the issue
(IdentityClientTests, TypedRepoClientTests) — same anti-pattern, same fix, no test-body changes since every
test in those two checks distinct behavior.
HttpStub gained Last, AssertCall/AssertGet/AssertPost (method/NSID/query-or-body/proxy in one call,
JSON body compared structurally) and a static Json(object) helper, replacing 12 per-file copies of the same
helpers across ChatModerationClientTests, GroupClientTests, OzoneClientTests and others. XrpcTestClient
forwards all of them, plus Fallback (both overloads) and ClearRequests.
TestSupport/TestIds.ModDid replaces the did:plc:ewvi7nxzyoun6zhxrhs64oiz literal 9 files in scope
redeclared as their own private const string ModDid/DidText (44 across the whole suite; BskyFixtures. AliceDid — used well beyond this PR's scope — now aliases the same constant instead of repeating the literal).
Each file's local const still exists (so none of their many interpolated fixture strings needed touching),
just pointed at the shared one.

Verification

python3 <scratchpad>/loc.py origin/main HEAD (bucket tests/ATProtoNet.Tests, my only touched area):
code 48007 → 47830 (−177), xmldoc 1476 → 1543 (+67, the new theory/kit doc comments), comment 1842 → 1899
(+57, the "covered by EndpointRequestTests" pointers left in place of removed tests), files 291 → 291
(3 deleted, 3 created).
Test totals: 5005 → 5005 (5000 passed / 5 skipped, both baseline and head; the 5 skips are
AspireManifestTests, unrelated). Net count is unchanged — 53 new theory rows against roughly that many
individual [Fact]s removed — while coverage strengthened: the drift check now checks query-parameter names
for 143 call sites SDK-wide, which the removed per-endpoint tests only checked one at a time.
Release build: dotnet build -c Release -p:EnableSourceControlManagerQueries=false → 0 warnings, 0 errors.
dotnet test tests/ATProtoNet.Tests/ --configuration Release -p:EnableSourceControlManagerQueries=false,
run twice: identical both times (0 failed, 5000 passed, 5 skipped).

Deferred

The remaining ~150-200 per-endpoint tests in this scope were left as dedicated tests, each for a stated
reason (union/typed-identifier round-trip, pagination, error mapping, bespoke encoding, or an explicit
regression comment) — see "What was kept" above. A further pass could still fold some of the "plain lookup
with an exhaustive multi-field response check" tests into lighter table rows, but I judged the marginal
line savings weren't worth the risk of quietly dropping a real assertion during transcription.
ConvoModelsTests, CachingLexiconResolverTests, LexiconResolverTests, LexiconClientTests (uses its own
ScriptedHandler, not the HttpStub family) and the remaining Lexicon/App/Bsky "Surface" test files
(Feed/Graph) were reviewed but left untouched — they're model/resolver machinery or already tight,
union-heavy files, not per-endpoint duplication.
T1/T3/T4 (shared HttpMessageHandler/IDidResolver/TimeProvider fakes, identifier-contract test
de-duplication, XrpcClientProxyTests theories) are explicitly a later part of Pass E per the brief; the
Identity, Http, Streaming, Server and Spaces test folders were not touched.
Flagged (not fixed, src/ out of scope here): AtProtoClient's internal 5-arg test constructor doesn't wire
its TimeProvider parameter to the XrpcClient's retry delay, only to session refresh — a background task
was spawned for this.

🤖 Generated with Claude Code

Part of #180 (pass E, part 1: Lexicon client tests). ## What this does Pass E's L2 (table-driven endpoint tests + query-parameter drift check) and L6 (test kit consolidation), scoped to `tests/ATProtoNet.Tests/{Ozone,Chat/Bsky, Lexicon/App/Bsky,Lexicon/Com/AtProto,Lexicon/Site}`. No `src/` changes: the drift check found no real bugs (see below). Test-only, no CHANGELOG entry. ### L2(a): query-parameter drift check `Lexicon/Upstream/SdkSource.Parameters.cs` walks the source the same way the existing NSID/method scan does: it finds every `var parameters = new XrpcParams()...;` (or `filter.ToParams(...)...;`) statement, the nearest following XRPC call that uses it, and the literal keys passed to `.Add`/ `.AddAll` in between — including keys added inside a conditional block or chained after a `ToParams()` helper. `LexiconDriftTests.ParamCalls_ KeysAreUpstreamParameters` then checks every key against the upstream Lexicon's `parameters` for that NSID, with a `QueryParamAllowlist` (currently empty) for deliberate exceptions, checked for staleness like the other allow-lists. Coverage: 143 call sites, 477 keys, 0 failures. Verified the check actually catches drift by temporarily renaming a key in `GraphClient.GetFollowersAsync` (`sort` → `sortXX`) and confirming the test failed with `app.bsky.graph. getFollowers:sortXX: not an upstream parameter`, then reverting. **Findings: no real bugs.** Every existing query-parameter key already matches its upstream Lexicon; the allow-list is empty. ### L2(b): table-driven theory for simple endpoints `Lexicon/EndpointRequestTests.cs` is a `[Theory]` over 53 rows, each a self-contained scenario: script a response, call the endpoint with every argument, assert the exact HTTP method/NSID/query-or-body (via new `HttpStub. AssertGet`/`AssertPost`, proxy header included where the original test checked one), and a light check that the response deserializes. It spans `chat.bsky.{actor,notification,group,convo,moderation}`, `app.bsky.{labeler, bookmark,notification,unspecced,embed,graph,draft,ageassurance}`, `com.atproto.{admin,temp}`, `tools.ozone.{queue,report,moderation, communication,team,set,server,signature,setting,safelink}` and `site. standard.*`'s delete wrappers. **Removed test files** (nothing left to keep once their tests moved to the table): - `Chat/Bsky/Notification/ChatNotificationClientTests.cs` (2 tests: getPreferences, putPreferences) - `Lexicon/App/Bsky/Labeler/LabelerClientTests.cs` (2 tests: getServices with/without `detailed`) **Removed test methods, by category** (covered by the table row of the same NSID unless noted): - **Ack-only calls** (no interesting response, just a method/body/proxy check): `chat.bsky.{actor.deleteAccount, group.withdrawJoinRequest, group.rejectJoinRequest, group.updateJoinRequestsRead, convo.updateAllRead, moderation.updateActorAccess}`, `app.bsky.{notification.updateSeen, graph.muteActor, draft.updateDraft, draft.deleteDraft, bookmark.createBookmark, bookmark.deleteBookmark, notification.unregisterPush}`, `com.atproto.temp.{requestPhoneVerification, revokeAccountCredentials}`, `tools.ozone.{queue.unassignModerator, report.unassignModerator, report.refreshStats, setting.removeOptions, safelink.removeRule}`, `site.standard.*`'s 4 delete wrappers (publication/document/subscription/recommend). - **Plain (non-union) lookups**: `chat.bsky.actor.getStatus`, `app.bsky.{labeler.getServices, ageassurance.begin, notification.listActivitySubscriptions, notification.putActivitySubscription, unspecced.getPostThreadV2's defaults-only case, embed.getEmbedExternalView's nothing-resolved case}`, `com.atproto.{admin.searchAccounts, admin.updateAccountSigningKey, temp.checkSignupQueue, temp.dereferenceScope}`, `tools.ozone.{queue.routeReports, queue.deleteQueue, moderation.getEvent, moderation.getRepo, moderation.queryEvents's plain form, moderation.getAccountTimeline, moderation.getReporterStats, communication.*, team.*, set.*, server.getConfig, signature.findRelatedAccounts}`. - **Merged near-duplicates**: `OzoneClientTests.FindRelatedAccounts_QueriesByDid` and `TypedOzoneClientTests. FindRelatedAccountsAsync_LimitThenCursor_SendsBoth` covered the same endpoint from two files (did-only, then limit+cursor); one row now covers did+limit+cursor. **What was kept, and why** (per-row response checks would either lose real coverage or duplicate a test that already exists for the right reason): - **Union/polymorphic responses**: `OzoneModerationToolingTests` (repo/record/subject/event variants), `OzoneClientTests.QueryStatusesAsync_ReadsSubjectStatuses` (ConvoSubject/AccountHosting), `ComAtProtoGapTests`'s `CheckHandleAvailabilityAsync_*` (3 tests, the Available/Unavailable/Unknown result variants), `TypedBskyClientTests`, `TypedOzoneClientTests`, `TypedRepoClientTests` (typed-identifier round-trips are these files' whole point), `BookmarkClientTests`, `UnspeccedClientTests`, `EmbedClientTests`, `GraphSurfaceTests` (thread/embed/bookmark item variants). - **Pagination walks**: every `Enumerate*Async` cursor-following test (Chat, Bsky, Ozone). - **Error mapping**: `IdentityClientTests.ResolveIdentityAsync_HandleNotFound_*`, `TypedOzoneClientTests. GetRepoAsync_InvalidDidInTheResponse_*`, `TypedRepoClientTests.Response_WithAnInvalidIdentifier_*`, `ComAtProtoGapTests.ImportRepoAsync_Rejected_*`. - **Special encoding / bespoke logic**: `ComAtProtoGapTests`'s 3 `ImportRepoAsync_*` stream tests (content-length, stream position, chunked non-seekable), `TypedBskyClientTests.PostAsync_ WritesACanonicalCreatedAt` (timestamp canonicalization), `VideoUploadTests.UploadVideoAsync_ BacksOffExponentially` (its own `XrpcClient` — see below). - **Regressions and cross-cutting checks, explicitly flagged in their own comments**: `OzoneClientTests. QueryStatusesAsync_Filter_*` ("querySubjects never existed"), `TypedBskyClientTests. ListNotificationsAsync_*` ("seenAt is deliberately not a parameter"), `ConvoClientTests. ProxyHeader_DoesNotAffectOtherXrpcCalls` and `ChatModerationClientTests.Calls_WithoutAClientWideProxy_*` (proxy-absence, not proxy-presence), `FeedSurfaceTests.SearchPostsV2Async_SendsEveryFilterUnderItsLexiconName` (asserts the full upstream parameter set, bidirectionally — kept as-is rather than reduced to the table's lighter check), `WireFidelityTests` (untouched, per its own docstring: regression tests for #123). ### L6: test kit - `Ozone/OzoneTestClient.cs` (hand-rolled `RecordingHandler` + `SentRequest`) deleted; its 4 consumers (`OzoneQueueClientTests`, `OzoneReportClientTests`, `OzoneToolsClientTests`, `OzoneModerationToolingTests`) moved to `XrpcTestClient`. - The 8 hand-built `HttpStub`+`HttpClient`+`XrpcClient` trios named in the issue — Chat Actor/Convo/Group/Moderation/Notification, Labeler, VideoUpload, StandardSite — moved onto `XrpcTestClient`, reached through `AtProtoClient`'s own `Chat`/`Bsky`/`Site` properties instead of constructing the Lexicon client directly. **Exception**: `VideoUploadTests.UploadVideoAsync_BacksOffExponentially` keeps its inline `XrpcClient` — `AtProtoClient`'s internal test constructor accepts a `TimeProvider` but only wires it to `SessionManager`, not to the `XrpcClient`'s own retry delay, so routing this one test through it would silently drop the injected clock. Flagged as a follow-up task (see below); out of scope for `src/` here. - The 2 hand-built `AtProtoClient`s named in the issue (`TypedBskyClientTests`, `ComAtProtoGapTests`) moved onto `XrpcTestClient`. Found and fixed the same pattern in 2 more files in scope not named in the issue (`IdentityClientTests`, `TypedRepoClientTests`) — same anti-pattern, same fix, no test-body changes since every test in those two checks distinct behavior. - `HttpStub` gained `Last`, `AssertCall`/`AssertGet`/`AssertPost` (method/NSID/query-or-body/proxy in one call, JSON body compared structurally) and a static `Json(object)` helper, replacing 12 per-file copies of the same helpers across `ChatModerationClientTests`, `GroupClientTests`, `OzoneClientTests` and others. `XrpcTestClient` forwards all of them, plus `Fallback` (both overloads) and `ClearRequests`. - `TestSupport/TestIds.ModDid` replaces the `did:plc:ewvi7nxzyoun6zhxrhs64oiz` literal 9 files in scope redeclared as their own `private const string ModDid`/`DidText` (44 across the whole suite; `BskyFixtures. AliceDid` — used well beyond this PR's scope — now aliases the same constant instead of repeating the literal). Each file's local const still exists (so none of their many interpolated fixture strings needed touching), just pointed at the shared one. ## Verification - `python3 <scratchpad>/loc.py origin/main HEAD` (bucket `tests/ATProtoNet.Tests`, my only touched area): code 48007 → 47830 (**−177**), xmldoc 1476 → 1543 (+67, the new theory/kit doc comments), comment 1842 → 1899 (+57, the "covered by EndpointRequestTests" pointers left in place of removed tests), files 291 → 291 (3 deleted, 3 created). - Test totals: **5005 → 5005** (5000 passed / 5 skipped, both baseline and head; the 5 skips are `AspireManifestTests`, unrelated). Net count is unchanged — 53 new theory rows against roughly that many individual `[Fact]`s removed — while coverage strengthened: the drift check now checks query-parameter names for 143 call sites SDK-wide, which the removed per-endpoint tests only checked one at a time. - Release build: `dotnet build -c Release -p:EnableSourceControlManagerQueries=false` → 0 warnings, 0 errors. - `dotnet test tests/ATProtoNet.Tests/ --configuration Release -p:EnableSourceControlManagerQueries=false`, run twice: identical both times (0 failed, 5000 passed, 5 skipped). ## Deferred - The remaining ~150-200 per-endpoint tests in this scope were left as dedicated tests, each for a stated reason (union/typed-identifier round-trip, pagination, error mapping, bespoke encoding, or an explicit regression comment) — see "What was kept" above. A further pass could still fold some of the "plain lookup with an exhaustive multi-field response check" tests into lighter table rows, but I judged the marginal line savings weren't worth the risk of quietly dropping a real assertion during transcription. - `ConvoModelsTests`, `CachingLexiconResolverTests`, `LexiconResolverTests`, `LexiconClientTests` (uses its own `ScriptedHandler`, not the `HttpStub` family) and the remaining Lexicon/App/Bsky "Surface" test files (Feed/Graph) were reviewed but left untouched — they're model/resolver machinery or already tight, union-heavy files, not per-endpoint duplication. - T1/T3/T4 (shared `HttpMessageHandler`/`IDidResolver`/`TimeProvider` fakes, identifier-contract test de-duplication, `XrpcClientProxyTests` theories) are explicitly a later part of Pass E per the brief; the `Identity`, `Http`, `Streaming`, `Server` and `Spaces` test folders were not touched. - Flagged (not fixed, `src/` out of scope here): `AtProtoClient`'s internal 5-arg test constructor doesn't wire its `TimeProvider` parameter to the `XrpcClient`'s retry delay, only to session refresh — a background task was spawned for this. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 16 commits

2026-09-27 17:24:18 +00:00

test(lexicon): drift-check query-parameter keys against upstream (#180 ) 16ca4a8593

Extends SdkSource and LexiconDriftTests to find every .Add/.AddAll
query-parameter key a client method sends and check it against the
upstream Lexicon's `parameters`, the same way NSIDs, GET/POST and JSON
names are already checked. Covers 143 call sites and 477 keys; no
drift found, verified by injecting a bad key and seeing it caught.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: grow the test kit and move 5 Chat clients + Labeler onto it (#180 ) 843340407d

Adds HttpStub.Last/AssertCall/AssertGet/AssertPost/Json, forwarded
through XrpcTestClient, and an optional TimeProvider on XrpcTestClient.
Moves the Chat Actor/Group/Moderation/Notification and Labeler client
tests off a hand-built HttpStub+HttpClient+XrpcClient trio onto
XrpcTestClient, reached through AtProtoClient's own Chat/Bsky
properties instead of constructing the Lexicon client directly.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: move ConvoClientTests onto the shared test kit (#180 ) 072c82a195

Replaces the hand-built HttpStub+HttpClient+XrpcClient trio with
XrpcTestClient, reached through AtProtoClient.Chat.Convo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: move StandardSiteClientTests onto the test kit; add Fallback (#180 ) 3211078533

Adds XrpcTestClient.Fallback (used throughout this file, which scripts
one fallback responder for its generic repo CRUD calls) and drops the
hand-built HttpStub+HttpClient+XrpcClient trio for XrpcTestClient,
reached through AtProtoClient.Site.

VideoUploadTests keeps its one inline XrpcClient: AtProtoClient's
internal test constructor doesn't wire a custom TimeProvider to the
XrpcClient's retry delay, only to session refresh, so routing this
test through it silently drops the injected clock (flagged separately,
src/ is out of scope for this change).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: move Ozone Queue/Tools tests off OzoneTestClient (#180 ) 475e7492f8

Replaces OzoneTestClient's Respond/Sent.Is(Query|Procedure) with
XrpcTestClient's On/AssertGet/AssertPost.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: retire OzoneTestClient for XrpcTestClient (#180 ) 5aba4cca1a

Finishes moving the Ozone test files (Report, ModerationTooling) off
OzoneTestClient's Respond/Sent.Is(Query|Procedure)/Requests onto
XrpcTestClient's On/AssertGet/AssertPost/To, and deletes the now-unused
OzoneTestClient.cs and its SentRequest helper record.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: move the last 2 hand-built AtProtoClients onto the test kit (#180 ) 82a35edd49

TypedBskyClientTests and ComAtProtoGapTests each hand-built an
AtProtoClient over a fallback-only HttpStub; both now use
XrpcTestClient, which gained Fallback (Func overload) and
ClearRequests forwarders for it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: table-driven theory for 20 simple per-endpoint requests (#180 ) f7c86397d2

Adds ATProtoNet.Tests.Lexicon.EndpointRequestTests: one row per XRPC
call whose interesting behavior is the request it sends (method, NSID,
query or body) and that a canned response deserializes. Migrates the
first batch of candidates - the ack-only and plain-field-response
endpoints across chat.bsky.actor/notification/group, app.bsky.labeler,
com.atproto.admin/temp and tools.ozone.queue/report - removing their
now-redundant per-file [Fact]s (two whole files, Chat Notification and
Labeler, had nothing left and are deleted). Response tests that check
a union, pagination, an error mapping, or bespoke logic (stream
uploads, canonical timestamps) stay in their own files.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: move IdentityClientTests and TypedRepoClientTests onto the kit (#180 ) 1742ec1a8e

Two more hand-built HttpStub+HttpClient+AtProtoClient constructions in
Lexicon/Com/AtProto, same pattern as the 8+2 named in the issue, moved
onto XrpcTestClient. No test bodies changed: every test here checks
distinct behavior (typed-identifier round-trips, URI splitting, error
mapping), so none is a table-driven-theory candidate.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: table-driven theory batch 2 - Ozone, bookmark, notification (#180 ) b7a2cb7475

9 more rows: tools.ozone.setting.removeOptions, .safelink.removeRule,
.moderation.getAccountTimeline/getReporterStats, app.bsky.bookmark
create/deleteBookmark, app.bsky.notification.unregisterPush /
listActivitySubscriptions / putActivitySubscription's plain case.
Removes the now-redundant per-file tests; each source file keeps its
union, pagination and partial-update tests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: table-driven theory batch 3 - StandardSite delete endpoints (#180 ) 20b1b40793

4 rows for StandardSiteClient's generic repo-CRUD delete wrappers
(publication, document, subscription, recommend), removing their
now-redundant per-file tests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: table-driven theory batch 4 - TypedBsky, Draft, AgeAssurance (#180 ) 3aaf00795b

5 more rows: app.bsky.notification.updateSeen, app.bsky.graph.muteActor,
app.bsky.draft.updateDraft/deleteDraft, app.bsky.ageassurance.begin.
Each source file keeps its typed-identifier round-trip and union
response tests untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: table-driven theory batch 5 - Convo/Moderation ack calls (#180 ) 81e5308376

2 more rows: chat.bsky.convo.updateAllRead, chat.bsky.moderation.updateActorAccess.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: table-driven theory batch 6 - OzoneClientTests bulk migration (#180 ) 5c6fa04a18

11 more rows covering tools.ozone.moderation (getEvent, getRepo, plain
queryEvents), .communication, .team, .set, .server and .signature.
OzoneClientTests keeps only its 3 special cases (raw-$type EmitEvent,
the querySubjects regression note, the union-typed subject-statuses
response); TypedOzoneClientTests' near-duplicate findRelatedAccounts
test folds into the same row as OzoneClientTests' version.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: table-driven theory batch 7 - Unspecced/Embed defaults (#180 ) 8d563eb343

2 more rows for the "nothing resolved / defaults only" cases of
getPostThreadV2 and getEmbedExternalView; the rest of each file's
union-heavy response tests stay in place.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

test: add TestIds shared constant for the repeated moderator DID (#180 )

CI / pds-integration (pull_request) Successful in 30s

Details

CI / build-and-test (pull_request) Successful in 1m5s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s

Details

eb8e74e314

did:plc:ewvi7nxzyoun6zhxrhs64oiz is redeclared as a private const in
9 files across Ozone/Chat/Bsky/Com.AtProto test classes (44 across the
whole suite) and duplicated again as BskyFixtures.AliceDid. Adds
TestSupport/TestIds.ModDid as the one place that names it; each file's
own local const/alias now points at it instead of repeating the
literal, so their many interpolated fixtures are untouched.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

opencode-bot commented

2026-09-27 17:24:20 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the review runs.

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-186/run-2026-09-27T17-24-19-570Z.md) — updating every ~5 minutes while the review runs.

opencode-bot commented

2026-09-27 17:24:22 +00:00

⏳ Held on the OpenCode Go window — the 5-hour window is at 0% (the bot holds at 95%).
This continues automatically after the window resets (2026-09-27T22:24:20.012Z) — nothing to re-trigger.

⏳ **Held on the OpenCode Go window** — the 5-hour window is at 0% (the bot holds at 95%). This continues automatically after the window resets (2026-09-27T22:24:20.012Z) — nothing to re-trigger.

Grandiras commented

2026-09-27 17:24:24 +00:00

@claude /review, please. This is part 1 of pass E of #180. It touches tests only. Please check three things:

The new query-parameter drift check (Lexicon/Upstream/SdkSource.Parameters.cs, LexiconDriftTests.ParamCalls_KeysAreUpstreamParameters). Does it really find every .Add/.AddAll key per call site? Could it pass vacuously, for example if a regex misses a call shape?
The 53-row EndpointRequestTests theory. Does each row assert at least what the removed tests asserted? The description lists them by category.
The HttpStub helpers and TestIds.

@claude /review, please. This is part 1 of pass E of #180. It touches tests only. Please check three things: 1. **The new query-parameter drift check** (`Lexicon/Upstream/SdkSource.Parameters.cs`, `LexiconDriftTests.ParamCalls_KeysAreUpstreamParameters`). Does it really find every `.Add`/`.AddAll` key per call site? Could it pass vacuously, for example if a regex misses a call shape? 2. **The 53-row `EndpointRequestTests` theory.** Does each row assert at least what the removed tests asserted? The description lists them by category. 3. **The `HttpStub` helpers and `TestIds`.**

claude-bot commented

2026-09-27 17:24:28 +00:00

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/642); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-186/run-642.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-27 17:26:24 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This is a large, mechanical test-only refactor (table-driven EndpointRequestTests, a new query-parameter drift check, and consolidation onto XrpcTestClient/HttpStub), and it's well-executed. I checked the parts of the diff that weren't shown in full (SdkSource.Parameters.cs, HttpStub.cs, XrpcTestClient.cs, TestIds.cs) directly against the PR branch, and they're consistent with the description: the new drift-check scanner is an honestly-documented heuristic (nearest-declaration-to-nearest-call regex walk, not a real parser), which is an acceptable trade-off for test infrastructure since a false negative there just means reduced coverage, not a false test failure. Spot-checked a sample of the moved rows in EndpointRequestTests.cs against the tests they replaced (argument order, expected query/body strings, proxy headers) and they match the original assertions faithfully — including the trickier ones like the merged FindRelatedAccountsAsync did+limit+cursor row and the ModDid/TestIds constant consolidation. No functional or security issues found.

One small piece of leftover dead code noted below; not blocking.

⏱ 1m 43s · 11 turns · tokens: 102.9k in / 10.5k out (+1223.3k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a large, mechanical test-only refactor (table-driven `EndpointRequestTests`, a new query-parameter drift check, and consolidation onto `XrpcTestClient`/`HttpStub`), and it's well-executed. I checked the parts of the diff that weren't shown in full (`SdkSource.Parameters.cs`, `HttpStub.cs`, `XrpcTestClient.cs`, `TestIds.cs`) directly against the PR branch, and they're consistent with the description: the new drift-check scanner is an honestly-documented heuristic (nearest-declaration-to-nearest-call regex walk, not a real parser), which is an acceptable trade-off for test infrastructure since a false negative there just means reduced coverage, not a false test failure. Spot-checked a sample of the moved rows in `EndpointRequestTests.cs` against the tests they replaced (argument order, expected query/body strings, proxy headers) and they match the original assertions faithfully — including the trickier ones like the merged `FindRelatedAccountsAsync` did+limit+cursor row and the `ModDid`/`TestIds` constant consolidation. No functional or security issues found. One small piece of leftover dead code noted below; not blocking. --- ⏱ 1m 43s · 11 turns · tokens: 102.9k in / 10.5k out (+1223.3k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-186/run-642.md)

tests/ATProtoNet.Tests/Chat/Bsky/Moderation/ChatModerationClientTests.cs

					
				@ -173,2 +147,3 @@

				    private HttpStub.RecordedRequest AssertGet(string nsid, string query) => _fixture.AssertGet(nsid, query, OzoneProxy);

				    public void Dispose() => _httpClient.Dispose();

				    private HttpStub.RecordedRequest AssertPost(string nsid) => _fixture.AssertPost(nsid, proxy: OzoneProxy);

private HttpStub.RecordedRequest AssertPost(string nsid) => _fixture.AssertPost(nsid, proxy: OzoneProxy); is now unused — its only caller, UpdateActorAccessAsync_PostsTheActorAccessAndRef, was removed and moved to EndpointRequestTests. Every remaining test in this file uses AssertGet only. Worth deleting; doesn't appear to trigger a build warning here so it's not blocking.

`private HttpStub.RecordedRequest AssertPost(string nsid) => _fixture.AssertPost(nsid, proxy: OzoneProxy);` is now unused — its only caller, `UpdateActorAccessAsync_PostsTheActorAccessAndRef`, was removed and moved to `EndpointRequestTests`. Every remaining test in this file uses `AssertGet` only. Worth deleting; doesn't appear to trigger a build warning here so it's not blocking.

claude-bot commented

2026-09-27 17:26:24 +00:00

🔧 Let Claude fix this — tick the box and claude-bot picks up the review above, on this PR branch. Nothing runs until you do.

@claude-bot Address the findings of the latest review on this pull request.

🔧 **Let Claude fix this** — tick the box and claude-bot picks up the review above, on this PR branch. Nothing runs until you do. - [ ] @claude-bot Address the findings of the latest review on this pull request.

Grandiras commented

2026-09-27 17:27:43 +00:00

Thanks. The unused AssertPost helper in ChatModerationClientTests.cs gets deleted in the next pass-E PR, which carries on consolidating these test files. I'm merging this one on the approval.

Thanks. The unused `AssertPost` helper in `ChatModerationClientTests.cs` gets deleted in the next pass-E PR, which carries on consolidating these test files. I'm merging this one on the approval.

Grandiras merged commit d8a7404ef4 into main

2026-09-27 17:27:44 +00:00

Grandiras deleted branch debloat-e1-endpoint-tests

2026-09-27 17:27:45 +00:00

Grandiras referenced this pull request from a commit

2026-09-27 17:27:45 +00:00

test: query-parameter drift check, table-driven endpoint tests, shared test kit (#186)

Grandiras referenced this pull request

2026-09-27 17:33:41 +00:00

fix: pass AtProtoClient's clock to its XRPC transport #188

Grandiras referenced this pull request

2026-09-27 17:52:57 +00:00