Grandiras/ATProto.NET

Fork 0

test: store contract tests, EF tests on SQLite, self-provisioning integration CI #176

Merged

Grandiras merged 6 commits from issue-137-tests-2 into main

2026-09-26 23:02:08 +00:00

Grandiras commented

2026-09-26 22:58:25 +00:00

Owner

Closes #137.

Second half of #137 (the first half, test infrastructure and low-value test removal, is #175 — already merged into main). This half covers what #175 deferred until after #136 merged: contract base classes for every store, moving EF tests to SQLite and dropping the InMemory provider, and the integration-CI changes, plus a further pass on the handler/host migration and duplicate-fact consolidation #175 also left open.

Contract base classes

Six abstract xUnit classes in tests/ATProtoNet.Tests/TestSupport/, one per store interface, each run against every implementation by a thin derived class:

SessionStoreContractTests → InMemoryAtProtoSessionStore, FileAtProtoSessionStore, EfCoreAtProtoSessionStore<T>
JtiReplayStoreContractTests → InMemoryJtiReplayStore, EfCoreJtiReplayStore<T>
RepoSyncStateStoreContractTests → InMemoryRepoSyncStateStore, EfCoreRepoSyncStateStore<T>
SpaceAuthorityStoreContractTests → InMemorySpaceAuthorityStore, EfCoreSpaceAuthorityStore<T>
SimpleSpaceStoreContractTests → InMemorySimpleSpaceStore, EfCoreSimpleSpaceStore<T>
OAuthStateStoreContractTests → InMemoryOAuthStateStore, DistributedCacheOAuthStateStore (over MemoryDistributedCache)

InMemorySimpleSpaceStore and InMemoryRepoSyncStateStore had no direct unit tests before this — only indirect coverage through other components — so the contract is their first real coverage.

Implementation-specific behaviour stayed where it was, or moved into a slimmer sibling test class: file permissions and legacy-format migration (FileAtProtoSessionStoreTests), EF schema/table names and the 0.6-row migration (EfCoreAtProtoSessionStoreTests), SQL collation and cross-restart persistence (EfCoreSpaceStoreTests, trimmed to just that plus one composition smoke test for the replay store table SpaceDbContext shares with JtiReplayDbContext), the in-memory sweep's internal timing hooks (InMemoryJtiReplayStoreTests), and per-requester/total capacity and key hashing for the OAuth state stores. Nothing lost an assertion — where a check depended on wall-clock precision that a SQLite round-trip doesn't preserve exactly (RegisterNotifyAsync renewal), the contract compares with a 1-second tolerance and the EF-specific test keeps the original fake-clock version.

EfCoreSpaceStoreTests split into three classes in the same file: the trimmed original plus new EfCoreSpaceAuthorityStoreContractTests and EfCoreSimpleSpaceStoreContractTests, since a single class can't derive from two contract base classes.

EF tests on SQLite; `Microsoft.EntityFrameworkCore.InMemory` dropped

Most EF store tests were already on SQLite from earlier issues (#133). The two remaining UseInMemoryDatabase call sites — EfCoreAtProtoSessionStoreTests and the two DI-registration test fixtures (JtiReplayStoreRegistrationTests, SpaceStoreRegistrationTests, which only check which store type resolves, never touch data) — now use a SqliteConnection with Mode=Memory;Cache=Shared, held open for the test's lifetime. Microsoft.EntityFrameworkCore.InMemory is removed from tests/ATProtoNet.Tests.csproj; Microsoft.EntityFrameworkCore.Sqlite stays pinned to 10.0.10, matching Microsoft.EntityFrameworkCore.Relational in src/ATProtoNet.Server.EntityFrameworkCore. This removes the version-pin trap the InMemory package needed (two EF Core Abstractions assemblies loading at once, MissingMethodException) — there's nothing left to pin.

Nothing was silently skipped: the ExecuteUpdate-based space store paths (#133) already ran on SQLite before this PR and still do.

Integration CI

AuthenticatedClientFixture provisions its own throwaway account through the admin API by default (like SpaceNetworkFixture), instead of requiring ATPROTO_TEST_HANDLE / ATPROTO_TEST_PASSWORD. Setting both env vars still overrides it. AuthenticationTests (the one suite that logged in directly against TestConfig.Handle/Password rather than through the fixture) now takes its credentials from a per-class AuthenticatedClientFixture (IClassFixture, not the shared [Collection("Authenticated")], so it stays independent of the other four suites sharing that collection).
The pds-integration CI job's filter widens from PdsAdminTests alone to also run AuthenticationTests, RepositoryTests, CustomRecordTests and IdentityResolutionTests against the reference PDS container — verified locally against the same ghcr.io/bluesky-social/pds:latest image via podman, with only ATPROTO_PDS_ADMIN_PASSWORD set: 30/30 passed. BlueskyFeatureTests' IntegrationRequirement.Bluesky cases stay out of the filter: that container has no app.bsky.* services, and with ATPROTO_REQUIRE_INTEGRATION=1 already set in that job, running them would fail rather than skip (confirmed locally — 3/3 failed with No service configured before I excluded them).
RequiresTheoryAttribute removed: it had no callers, and none of the integration suites' facts share enough shape to be worth merging into a Theory (each covers a materially different scenario against the live PDS, unlike the pure-function cases in the unit-test merges below).
CONTRIBUTING.md's manual integration-test setup no longer tells contributors to curl a test account into existence by hand.

Handler/host migration (continuing from #175)

Migrated onto HttpStub/XrpcTestClient where it was a real simplification:

Ozone/OzoneClientTests.cs, Ozone/TypedOzoneClientTests.cs — each had its own single-response or queue-based HttpMessageHandler.
Sync/SyncClientTests.cs — HttpStub's On(nsid, respond) lambda overload already supports the binary CAR responses this file needs (controllable Content-Type, declared length, chunked transfer), so no TestSupport changes were needed.
Spaces/SpaceClientTests.cs — built its own XrpcClient + SpaceClient/SimpleSpaceClient directly, instead of going through AtProtoClient.Space / .SimpleSpace like every other Lexicon-domain test file.

Left as-is, with reasons (this covers every file the issue named for this area):

Ozone/OzoneModerationToolingTests.cs, OzoneQueueClientTests.cs, OzoneReportClientTests.cs, OzoneToolsClientTests.cs were already migrated in #175 (they share OzoneTestClient's IsQuery/IsProcedure DSL) — not part of this pass's scope.
Server/Authentication/AtProtoOAuthServiceHttpClientTests.cs — its two handler-using tests check HttpClient ownership (a caller-supplied client survives the service; an SDK-created one gets the configured timeout), not XRPC scripting. HttpStub/XrpcTestClient are built around AtProtoClient; this file tests AtProtoOAuthService's raw HttpClient sourcing, and its existing 12-line handler is already about as small as the concern needs.
Spaces/SpaceCredentialMintTests.cs — its FakeSpaceNetwork is a genuinely stateful fixture: gated/synchronized minting for concurrency tests, per-space mint counts, live DPoP-bound credential signing. A HttpStub queue can't express "block this request until the test releases it."
Spaces/SpaceCredentialProviderTests.cs — its StaticHandler fakes a PLC directory response (DID document resolution), not an XRPC endpoint; already a 3-line handler, and HttpSpaceClientMetadataResolverTests alongside it fakes client-metadata/JWKS fetches the same way — neither goes through AtProtoClient.
Spaces/SpaceSyncerTests.cs — its StubHost mutates shared state (Ops, OpsStatus, CarStatus, Car, UndeclaredLength) mid-test across two different endpoint shapes (JSON oplog, binary CAR); migrating it safely needs more verification time than this pass had.
Server/Spaces/HttpSpaceClientMetadataResolverTests.cs — fakes third-party client-metadata.json/JWKS responses (including undeclared-Content-Length bodies) for HttpSpaceClientMetadataResolver, constructed directly over a raw HttpClient; not AtProtoClient/XRPC.
Server/Spaces/SimpleSpaceManagingAppClientTests.cs, SpaceServiceAuthKeyTests.cs, SpaceWriteNotifierTests.cs — each has its own small single-response RecordingHandler for a managing-app HTTP check, a named IHttpClientFactory client, and a write-notification POST respectively; none go through AtProtoClient, and SpaceWriteNotifierTests mixes in NSubstitute mocks of IDidResolver/ISpaceAuthorityStore that are orthogonal to the HTTP layer. Each handler is small and self-contained; the marginal win from swapping in a differently-shaped HttpStub wasn't worth the remaining review budget in this pass.
The four TestServer hosts (Server/Authentication/{AtProtoOAuthEndpointTests,ServiceAuthWithSpacesTests}, Server/Spaces/{SimpleSpaceEndpointTests,SpaceServerEndpointTests}) — #175 deferred these pending #136's DI-registration redesign, which has since merged, so the original blocker is gone. Left for a follow-up: SpaceServerEndpointTests.cs alone is 800 lines of DI-registration-heavy ASP.NET Core hosting, and consolidating four such hosts safely is a larger, separately-reviewable change than this PR's remaining scope allowed.
Spaces/{SpaceCommitTests,SpaceRepoCarTests,SpaceScopeTests,SpaceTokensTests,SpaceAuthorityTests,SpaceUriTests,SpaceUnionTests,Blake3Tests,LtHashTests}.cs — pure unit tests with no HTTP handler at all; not in the issue's list.
Streaming/*, Tap/TapClientTests.cs — WebSocket-based (Jetstream, Tap's event channel) or bespoke non-XRPC HTTP (Tap's admin endpoints), with their own fake connectors/sockets. Genuinely specialized; HttpStub has no WebSocket story.

Duplicate facts merged into theories

Streaming/JetstreamEventParserTests.cs: the six standalone Parse_*_ReturnsNull facts plus the existing 4-case Parse_MalformedFrame_ReturnsNull theory collapsed into one Parse_InvalidOrUnrecognizedFrame_ReturnsNull theory (9 cases).
Auth/OAuth/AtProtoScopesTests.cs: the pure "build a scope, assert the string" facts for Repo, Rpc, Blob, Account and the transitional scope constants collapsed into one Theory/MemberData group each; the three "no actions is rejected" exception facts collapsed into one. Exception-shape and reflection-based facts (Repo_EmptyCollection_Throws, Rpc_BothWildcards_Throws, IdentityAction_IsObsolete, the Include/Combine/Rpc-audience groups) stayed as they were — they check more than a single string-in/string-out relationship.

The remaining ~18 groups #175's audit identified were left for a further pass; each is a smaller win than these two and none block anything else in this cycle.

Breaking changes

None. Test-project-internal only. RequiresTheoryAttribute (unused, tests/ATProtoNet.IntegrationTests is not packed) is removed.

Verified

dotnet build -p:EnableSourceControlManagerQueries=false (Debug and Release, full solution): 0 warnings, 0 errors.
dotnet test tests/ATProtoNet.Tests/ -p:EnableSourceControlManagerQueries=false: 5058 passed / 5 skipped / 5063 total, 0 failures, twice in a row (also in --configuration Release).
dotnet test tests/ATProtoNet.IntegrationTests/: 85 skipped locally (no PDS configured), 0 failures.
Live reference PDS via podman (ghcr.io/bluesky-social/pds:latest, the image the CI job uses): with only ATPROTO_PDS_URL / ATPROTO_PDS_ADMIN_PASSWORD set, PdsAdminTests + AuthenticationTests + RepositoryTests + CustomRecordTests + IdentityResolutionTests — 30/30 passed, confirming AuthenticatedClientFixture's self-provisioning and the widened CI filter both work end to end.

Deferred / flagged for the docs agent (#138)

CLAUDE.md still mentions a [RequiresTheory(...)] counterpart to RequiresFactAttribute in the integration-test build/test section; that attribute is removed in this PR. CLAUDE.md is out of this PR's scope (owned by #138) — flagging the stale line for that pass.
/home/grandiras/.claude/projects/.../memory/efcore-version-pinning.md (a Claude memory file, not part of the repo) documents the InMemory/Relational version-pin trap this PR removes entirely by dropping the InMemory package; worth archiving or updating, but it's outside the repo and outside this PR's scope.

🤖 Generated with Claude Code

Closes #137. Second half of #137 (the first half, test infrastructure and low-value test removal, is #175 — already merged into `main`). This half covers what #175 deferred until after #136 merged: contract base classes for every store, moving EF tests to SQLite and dropping the InMemory provider, and the integration-CI changes, plus a further pass on the handler/host migration and duplicate-fact consolidation #175 also left open. ## Contract base classes Six abstract xUnit classes in `tests/ATProtoNet.Tests/TestSupport/`, one per store interface, each run against every implementation by a thin derived class: - `SessionStoreContractTests` → `InMemoryAtProtoSessionStore`, `FileAtProtoSessionStore`, `EfCoreAtProtoSessionStore<T>` - `JtiReplayStoreContractTests` → `InMemoryJtiReplayStore`, `EfCoreJtiReplayStore<T>` - `RepoSyncStateStoreContractTests` → `InMemoryRepoSyncStateStore`, `EfCoreRepoSyncStateStore<T>` - `SpaceAuthorityStoreContractTests` → `InMemorySpaceAuthorityStore`, `EfCoreSpaceAuthorityStore<T>` - `SimpleSpaceStoreContractTests` → `InMemorySimpleSpaceStore`, `EfCoreSimpleSpaceStore<T>` - `OAuthStateStoreContractTests` → `InMemoryOAuthStateStore`, `DistributedCacheOAuthStateStore` (over `MemoryDistributedCache`) `InMemorySimpleSpaceStore` and `InMemoryRepoSyncStateStore` had no direct unit tests before this — only indirect coverage through other components — so the contract is their first real coverage. Implementation-specific behaviour stayed where it was, or moved into a slimmer sibling test class: file permissions and legacy-format migration (`FileAtProtoSessionStoreTests`), EF schema/table names and the 0.6-row migration (`EfCoreAtProtoSessionStoreTests`), SQL collation and cross-restart persistence (`EfCoreSpaceStoreTests`, trimmed to just that plus one composition smoke test for the replay store table `SpaceDbContext` shares with `JtiReplayDbContext`), the in-memory sweep's internal timing hooks (`InMemoryJtiReplayStoreTests`), and per-requester/total capacity and key hashing for the OAuth state stores. Nothing lost an assertion — where a check depended on wall-clock precision that a SQLite round-trip doesn't preserve exactly (`RegisterNotifyAsync` renewal), the contract compares with a 1-second tolerance and the EF-specific test keeps the original fake-clock version. `EfCoreSpaceStoreTests` split into three classes in the same file: the trimmed original plus new `EfCoreSpaceAuthorityStoreContractTests` and `EfCoreSimpleSpaceStoreContractTests`, since a single class can't derive from two contract base classes. ## EF tests on SQLite; `Microsoft.EntityFrameworkCore.InMemory` dropped Most EF store tests were already on SQLite from earlier issues (#133). The two remaining `UseInMemoryDatabase` call sites — `EfCoreAtProtoSessionStoreTests` and the two DI-registration test fixtures (`JtiReplayStoreRegistrationTests`, `SpaceStoreRegistrationTests`, which only check which store type resolves, never touch data) — now use a `SqliteConnection` with `Mode=Memory;Cache=Shared`, held open for the test's lifetime. `Microsoft.EntityFrameworkCore.InMemory` is removed from `tests/ATProtoNet.Tests.csproj`; `Microsoft.EntityFrameworkCore.Sqlite` stays pinned to `10.0.10`, matching `Microsoft.EntityFrameworkCore.Relational` in `src/ATProtoNet.Server.EntityFrameworkCore`. This removes the version-pin trap the InMemory package needed (two EF Core Abstractions assemblies loading at once, `MissingMethodException`) — there's nothing left to pin. Nothing was silently skipped: the `ExecuteUpdate`-based space store paths (#133) already ran on SQLite before this PR and still do. ## Integration CI - **`AuthenticatedClientFixture`** provisions its own throwaway account through the admin API by default (like `SpaceNetworkFixture`), instead of requiring `ATPROTO_TEST_HANDLE` / `ATPROTO_TEST_PASSWORD`. Setting both env vars still overrides it. `AuthenticationTests` (the one suite that logged in directly against `TestConfig.Handle`/`Password` rather than through the fixture) now takes its credentials from a per-class `AuthenticatedClientFixture` (`IClassFixture`, not the shared `[Collection("Authenticated")]`, so it stays independent of the other four suites sharing that collection). - The `pds-integration` CI job's filter widens from `PdsAdminTests` alone to also run `AuthenticationTests`, `RepositoryTests`, `CustomRecordTests` and `IdentityResolutionTests` against the reference PDS container — **verified locally** against the same `ghcr.io/bluesky-social/pds:latest` image via podman, with only `ATPROTO_PDS_ADMIN_PASSWORD` set: 30/30 passed. `BlueskyFeatureTests`' `IntegrationRequirement.Bluesky` cases stay out of the filter: that container has no `app.bsky.*` services, and with `ATPROTO_REQUIRE_INTEGRATION=1` already set in that job, running them would fail rather than skip (confirmed locally — 3/3 failed with `No service configured` before I excluded them). - `RequiresTheoryAttribute` removed: it had no callers, and none of the integration suites' facts share enough shape to be worth merging into a `Theory` (each covers a materially different scenario against the live PDS, unlike the pure-function cases in the unit-test merges below). - `CONTRIBUTING.md`'s manual integration-test setup no longer tells contributors to `curl` a test account into existence by hand. ## Handler/host migration (continuing from #175) Migrated onto `HttpStub`/`XrpcTestClient` where it was a real simplification: - `Ozone/OzoneClientTests.cs`, `Ozone/TypedOzoneClientTests.cs` — each had its own single-response or queue-based `HttpMessageHandler`. - `Sync/SyncClientTests.cs` — `HttpStub`'s `On(nsid, respond)` lambda overload already supports the binary CAR responses this file needs (controllable `Content-Type`, declared length, chunked transfer), so no `TestSupport` changes were needed. - `Spaces/SpaceClientTests.cs` — built its own `XrpcClient` + `SpaceClient`/`SimpleSpaceClient` directly, instead of going through `AtProtoClient.Space` / `.SimpleSpace` like every other Lexicon-domain test file. Left as-is, with reasons (this covers every file the issue named for this area): - **`Ozone/OzoneModerationToolingTests.cs`, `OzoneQueueClientTests.cs`, `OzoneReportClientTests.cs`, `OzoneToolsClientTests.cs`** were already migrated in #175 (they share `OzoneTestClient`'s `IsQuery`/`IsProcedure` DSL) — not part of this pass's scope. - **`Server/Authentication/AtProtoOAuthServiceHttpClientTests.cs`** — its two handler-using tests check `HttpClient` *ownership* (a caller-supplied client survives the service; an SDK-created one gets the configured timeout), not XRPC scripting. `HttpStub`/`XrpcTestClient` are built around `AtProtoClient`; this file tests `AtProtoOAuthService`'s raw `HttpClient` sourcing, and its existing 12-line handler is already about as small as the concern needs. - **`Spaces/SpaceCredentialMintTests.cs`** — its `FakeSpaceNetwork` is a genuinely stateful fixture: gated/synchronized minting for concurrency tests, per-space mint counts, live DPoP-bound credential signing. A `HttpStub` queue can't express "block this request until the test releases it." - **`Spaces/SpaceCredentialProviderTests.cs`** — its `StaticHandler` fakes a PLC directory response (DID document resolution), not an XRPC endpoint; already a 3-line handler, and `HttpSpaceClientMetadataResolverTests` alongside it fakes client-metadata/JWKS fetches the same way — neither goes through `AtProtoClient`. - **`Spaces/SpaceSyncerTests.cs`** — its `StubHost` mutates shared state (`Ops`, `OpsStatus`, `CarStatus`, `Car`, `UndeclaredLength`) mid-test across two different endpoint shapes (JSON oplog, binary CAR); migrating it safely needs more verification time than this pass had. - **`Server/Spaces/HttpSpaceClientMetadataResolverTests.cs`** — fakes third-party client-metadata.json/JWKS responses (including undeclared-`Content-Length` bodies) for `HttpSpaceClientMetadataResolver`, constructed directly over a raw `HttpClient`; not `AtProtoClient`/XRPC. - **`Server/Spaces/SimpleSpaceManagingAppClientTests.cs`, `SpaceServiceAuthKeyTests.cs`, `SpaceWriteNotifierTests.cs`** — each has its own small single-response `RecordingHandler` for a managing-app HTTP check, a named `IHttpClientFactory` client, and a write-notification POST respectively; none go through `AtProtoClient`, and `SpaceWriteNotifierTests` mixes in NSubstitute mocks of `IDidResolver`/`ISpaceAuthorityStore` that are orthogonal to the HTTP layer. Each handler is small and self-contained; the marginal win from swapping in a differently-shaped `HttpStub` wasn't worth the remaining review budget in this pass. - **The four TestServer hosts** (`Server/Authentication/{AtProtoOAuthEndpointTests,ServiceAuthWithSpacesTests}`, `Server/Spaces/{SimpleSpaceEndpointTests,SpaceServerEndpointTests}`) — #175 deferred these pending #136's DI-registration redesign, which has since merged, so the original blocker is gone. Left for a follow-up: `SpaceServerEndpointTests.cs` alone is 800 lines of DI-registration-heavy ASP.NET Core hosting, and consolidating four such hosts safely is a larger, separately-reviewable change than this PR's remaining scope allowed. - **`Spaces/{SpaceCommitTests,SpaceRepoCarTests,SpaceScopeTests,SpaceTokensTests,SpaceAuthorityTests,SpaceUriTests,SpaceUnionTests,Blake3Tests,LtHashTests}.cs`** — pure unit tests with no HTTP handler at all; not in the issue's list. - **`Streaming/*`, `Tap/TapClientTests.cs`** — WebSocket-based (Jetstream, Tap's event channel) or bespoke non-XRPC HTTP (Tap's admin endpoints), with their own fake connectors/sockets. Genuinely specialized; `HttpStub` has no WebSocket story. ## Duplicate facts merged into theories - `Streaming/JetstreamEventParserTests.cs`: the six standalone `Parse_*_ReturnsNull` facts plus the existing 4-case `Parse_MalformedFrame_ReturnsNull` theory collapsed into one `Parse_InvalidOrUnrecognizedFrame_ReturnsNull` theory (9 cases). - `Auth/OAuth/AtProtoScopesTests.cs`: the pure "build a scope, assert the string" facts for `Repo`, `Rpc`, `Blob`, `Account` and the transitional scope constants collapsed into one `Theory`/`MemberData` group each; the three "no actions is rejected" exception facts collapsed into one. Exception-shape and reflection-based facts (`Repo_EmptyCollection_Throws`, `Rpc_BothWildcards_Throws`, `IdentityAction_IsObsolete`, the `Include`/`Combine`/`Rpc`-audience groups) stayed as they were — they check more than a single string-in/string-out relationship. The remaining ~18 groups #175's audit identified were left for a further pass; each is a smaller win than these two and none block anything else in this cycle. ## Breaking changes None. Test-project-internal only. `RequiresTheoryAttribute` (unused, `tests/ATProtoNet.IntegrationTests` is not packed) is removed. ## Verified - `dotnet build -p:EnableSourceControlManagerQueries=false` (Debug and Release, full solution): 0 warnings, 0 errors. - `dotnet test tests/ATProtoNet.Tests/ -p:EnableSourceControlManagerQueries=false`: **5058 passed / 5 skipped / 5063 total, 0 failures**, twice in a row (also in `--configuration Release`). - `dotnet test tests/ATProtoNet.IntegrationTests/`: 85 skipped locally (no PDS configured), 0 failures. - Live reference PDS via podman (`ghcr.io/bluesky-social/pds:latest`, the image the CI job uses): with only `ATPROTO_PDS_URL` / `ATPROTO_PDS_ADMIN_PASSWORD` set, `PdsAdminTests` + `AuthenticationTests` + `RepositoryTests` + `CustomRecordTests` + `IdentityResolutionTests` — **30/30 passed**, confirming `AuthenticatedClientFixture`'s self-provisioning and the widened CI filter both work end to end. ## Deferred / flagged for the docs agent (#138) - `CLAUDE.md` still mentions a `[RequiresTheory(...)]` counterpart to `RequiresFactAttribute` in the integration-test build/test section; that attribute is removed in this PR. `CLAUDE.md` is out of this PR's scope (owned by #138) — flagging the stale line for that pass. - `/home/grandiras/.claude/projects/.../memory/efcore-version-pinning.md` (a Claude memory file, not part of the repo) documents the InMemory/Relational version-pin trap this PR removes entirely by dropping the InMemory package; worth archiving or updating, but it's outside the repo and outside this PR's scope. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 6 commits

2026-09-26 22:58:25 +00:00

test: contract base classes for session, replay, sync-state, space and OAuth stores 4f1f48d213

Add abstract xUnit contract classes (TestSupport/*ContractTests.cs) covering
IAtProtoSessionStore, IJtiReplayStore, IRepoSyncStateStore, ISpaceAuthorityStore,
ISimpleSpaceStore and IOAuthStateStore, each run against every implementation via a
thin derived class. Implementation-specific tests (file permissions, EF schema/table
names, SQL collation, sweep-timing hooks, capacity/eviction) stay where they were.

Adds first direct unit coverage for InMemorySimpleSpaceStore and
InMemoryRepoSyncStateStore, which previously had none of their own. Converts the two
DI-registration test fixtures that used UseInMemoryDatabase (JtiReplayStoreRegistrationTests,
SpaceStoreRegistrationTests) to SQLite ahead of dropping the InMemory provider.

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

test: drop Microsoft.EntityFrameworkCore.InMemory from the test project db4c18aae5

All EF Core store tests now run over Sqlite (an in-memory, shared-cache database with
a kept-open connection), which the space stores already needed for composite keys and
key-violation-based replay detection. This removes the version-pin trap: the InMemory
package no longer has to be bumped in lockstep with EFCore.Relational in src.

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

test: self-provisioning AuthenticatedClientFixture, widen the PDS-integration CI job d1ec4a7750

AuthenticatedClientFixture now provisions its own throwaway account through the admin
API, like SpaceNetworkFixture, instead of requiring a pre-existing ATPROTO_TEST_HANDLE /
ATPROTO_TEST_PASSWORD account. Setting both env vars still overrides it. AuthenticationTests
(the one suite that logged in directly rather than through the fixture) now gets its
credentials from a per-class AuthenticatedClientFixture instead of TestConfig, keeping it
independent of the other suites sharing the "Authenticated" collection.

The pds-integration CI job's filter widens from PdsAdminTests alone to also run
AuthenticationTests, RepositoryTests, CustomRecordTests and IdentityResolutionTests against
the reference PDS container on every PR — verified locally against the same
ghcr.io/bluesky-social/pds:latest image the job uses, with only ATPROTO_PDS_ADMIN_PASSWORD
set (30/30 passed). BlueskyFeatureTests' IntegrationRequirement.Bluesky cases stay out: that
container has no app.bsky.* services, so ATPROTO_REQUIRE_INTEGRATION would fail rather than
skip them.

Removed RequiresTheoryAttribute (unused — no integration Facts share enough shape to be
worth merging into one Theory) and updated the CHANGELOG bullet and CONTRIBUTING.md's manual
setup instructions that described it.

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

test: migrate Ozone and Sync client tests onto HttpStub/XrpcTestClient 0a2155cc3b

OzoneClientTests, TypedOzoneClientTests and SyncClientTests each had their own
hand-rolled single-response or queue-based HttpMessageHandler. All three fit
XrpcTestClient/HttpStub directly: HttpStub's On(nsid, respond) lambda overload already
supports the binary CAR responses (controllable Content-Type, declared length, chunked
transfer) SyncClientTests needed, so no changes to TestSupport were required.

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

test: migrate SpaceClientTests onto XrpcTestClient 2c2769c87f

SpaceClientTests built its own XrpcClient + SpaceClient/SimpleSpaceClient directly over
a hand-rolled MockHttpMessageHandler, instead of going through AtProtoClient.Space /
AtProtoClient.SimpleSpace like the rest of the Lexicon-domain test files. Switched it to
XrpcTestClient, matching the established pattern; behaviour is unchanged (18/18 pass).

Left the remaining Spaces/** and Server/Spaces/** handler files and the four TestServer
hosts in the Space/OAuth area as they are — see the final report for why each one is
either a genuinely stateful fixture (concurrent-mint coordination in
SpaceCredentialMintTests, a mutable-property CAR/ops stub in SpaceSyncerTests) or
DI-registration-heavy ASP.NET Core hosting that is a larger, separately-risked migration.

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

test: merge duplicate facts into theories (JetstreamEventParserTests, AtProtoScopesTests)

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

Details

CI / build-and-test (pull_request) Successful in 56s

Details

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

Details

e22f0265ed

JetstreamEventParserTests: the six standalone Parse_*_ReturnsNull facts plus the existing
Parse_MalformedFrame_ReturnsNull theory collapse into one Parse_InvalidOrUnrecognizedFrame_ReturnsNull
theory (9 cases) — every one asserts the same thing (Parse returns null) over a different
malformed or unrecognized frame.

AtProtoScopesTests: the pure "build a scope, assert the string" facts for Repo, Rpc, Blob,
Account and the transitional scope constants collapse into one theory per group
(Func<string>-producing cases plus the expected string); the three "no actions" exception
facts collapse similarly. Kept every exception-shape and reflection-based fact
(Repo_EmptyCollection_Throws, Rpc_BothWildcards_Throws, IdentityAction_IsObsolete, the
Include/Combine/Rpc-audience groups) separate, since they check more than one thing or
assert a different shape than "expected string in, actual string out".

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

opencode-bot commented

2026-09-26 22:58:27 +00:00

Collaborator

🤖 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-176/run-2026-09-26T22-58-26-918Z.md) — updating every ~5 minutes while the review runs.

opencode-bot commented

2026-09-26 22:58:29 +00:00

Collaborator

⏳ 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-27T03:58:27.323Z) — 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-27T03:58:27.323Z) — nothing to re-trigger.

Grandiras commented

2026-09-26 22:58:32 +00:00

Author

Owner

@claude /review, second half of #137 (#175 was the first). This is test-only apart from ci.yml, CONTRIBUTING and one CHANGELOG bullet. Please check:

The TestSupport/*ContractTests.cs classes. Does every behaviour the old implementation-specific tests asserted survive, either in the contract or in the trimmed implementation tests? That includes EfCoreSpaceStoreTests, which was split into three. The one tolerance added (1 s on the RegisterNotifyAsync renewal, because SQLite loses sub-millisecond precision): is it justified, and is the exact fake-clock check kept elsewhere?
The EF tests now run on SQLite Mode=Memory;Cache=Shared. Can shared-cache names collide between parallel tests?
The ci.yml change. Only the filter widens, with no untrusted interpolation. AuthenticatedClientFixture now provisions its own account through the admin API.
The migrated Ozone, Sync and Space client tests. Are any assertions weakened?

@claude /review, second half of #137 (#175 was the first). This is test-only apart from ci.yml, CONTRIBUTING and one CHANGELOG bullet. Please check: 1. The `TestSupport/*ContractTests.cs` classes. Does every behaviour the old implementation-specific tests asserted survive, either in the contract or in the trimmed implementation tests? That includes `EfCoreSpaceStoreTests`, which was split into three. The one tolerance added (1 s on the `RegisterNotifyAsync` renewal, because SQLite loses sub-millisecond precision): is it justified, and is the exact fake-clock check kept elsewhere? 2. The EF tests now run on SQLite `Mode=Memory;Cache=Shared`. Can shared-cache names collide between parallel tests? 3. The ci.yml change. Only the filter widens, with no untrusted interpolation. `AuthenticatedClientFixture` now provisions its own account through the admin API. 4. The migrated Ozone, Sync and Space client tests. Are any assertions weakened?

claude-bot commented

2026-09-26 22:58:34 +00:00

Collaborator

🤖 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/628); 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-176/run-628.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-09-26 23:01:01 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

This is a large, well-executed test-only PR. I checked the parts most likely to hide bugs:

AuthenticatedClientFixture self-provisioning: constructor args, AllowInsecureHttp, DescribeServerAsync/CreateAccountAsync/DeleteAccountAsync signatures all match PdsAdminClient's actual API, and the handle-generation pattern ($"itest-{Guid.NewGuid():N}"[..16].TrimEnd('-') + domain) is copied verbatim from the already-merged SpaceNetworkFixture.CreateActorAsync, so it's consistent with existing convention (the TrimEnd('-') is a no-op given the hex alphabet, but that's pre-existing, not new).
AuthenticationTests switching to IClassFixture<AuthenticatedClientFixture> and using fixture.Handle/fixture.Password instead of TestConfig.Handle/Password is correct and matches the stated intent of keeping this suite independent of the shared "Authenticated" collection.
CanAuthenticate in TestInfrastructure.cs correctly widens the gate to HasAccount || AdminPassword set, and the skip-reason strings are updated to match.
CI filter change (ci.yml) uses valid vstest FullyQualifiedName~A|FullyQualifiedName~B OR syntax, consistent with the pre-existing single-term filter it replaces.
CHANGELOG: confirmed via line numbers that both the amended [RequiresFact] bullet and the new AuthenticatedClientFixture bullet land under ### Changed (not ### Breaking changes), consistent with the PR description's "Breaking changes: None."
Contract base classes: the diff shown to me was truncated before these files, so I diffed main against origin/issue-137-tests-2 directly to confirm all six (SessionStoreContractTests, JtiReplayStoreContractTests, RepoSyncStateStoreContractTests, SpaceAuthorityStoreContractTests, SimpleSpaceStoreContractTests, OAuthStateStoreContractTests) are actually added, and spot-checked two of them (SessionStoreContractTests, SpaceAuthorityStoreContractTests) — both are well-scoped, cross-checked against implementation (ArgumentNullException.ThrowIfNull calls in FileAtProtoSessionStore) and free of the specific-implementation leakage the PR description says was avoided.
The EF-to-SQLite migration uses Mode=Memory;Cache=Shared with a GUID-unique data source per test/class and a held-open connection, matching the pattern already used elsewhere in the suite (EfCoreJtiReplayStoreTests, SpaceNetworkFixture-adjacent code).

No correctness bugs found. The scope and reasoning in the PR description are unusually well-substantiated (line-cited verification, explicit reasons for every file left unmigrated), and the code matches what it claims.

⏱ 2m 18s · 18 turns · tokens: 113.0k in / 9.4k out (+2162.0k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. This is a large, well-executed test-only PR. I checked the parts most likely to hide bugs: - **`AuthenticatedClientFixture`** self-provisioning: constructor args, `AllowInsecureHttp`, `DescribeServerAsync`/`CreateAccountAsync`/`DeleteAccountAsync` signatures all match `PdsAdminClient`'s actual API, and the handle-generation pattern (`$"itest-{Guid.NewGuid():N}"[..16].TrimEnd('-') + domain`) is copied verbatim from the already-merged `SpaceNetworkFixture.CreateActorAsync`, so it's consistent with existing convention (the `TrimEnd('-')` is a no-op given the hex alphabet, but that's pre-existing, not new). - **`AuthenticationTests`** switching to `IClassFixture<AuthenticatedClientFixture>` and using `fixture.Handle`/`fixture.Password` instead of `TestConfig.Handle`/`Password` is correct and matches the stated intent of keeping this suite independent of the shared `"Authenticated"` collection. - **`CanAuthenticate`** in `TestInfrastructure.cs` correctly widens the gate to `HasAccount || AdminPassword set`, and the skip-reason strings are updated to match. - **CI filter change** (`ci.yml`) uses valid vstest `FullyQualifiedName~A|FullyQualifiedName~B` OR syntax, consistent with the pre-existing single-term filter it replaces. - **CHANGELOG**: confirmed via line numbers that both the amended `[RequiresFact]` bullet and the new `AuthenticatedClientFixture` bullet land under `### Changed` (not `### Breaking changes`), consistent with the PR description's "Breaking changes: None." - **Contract base classes**: the diff shown to me was truncated before these files, so I diffed `main` against `origin/issue-137-tests-2` directly to confirm all six (`SessionStoreContractTests`, `JtiReplayStoreContractTests`, `RepoSyncStateStoreContractTests`, `SpaceAuthorityStoreContractTests`, `SimpleSpaceStoreContractTests`, `OAuthStateStoreContractTests`) are actually added, and spot-checked two of them (`SessionStoreContractTests`, `SpaceAuthorityStoreContractTests`) — both are well-scoped, cross-checked against implementation (`ArgumentNullException.ThrowIfNull` calls in `FileAtProtoSessionStore`) and free of the specific-implementation leakage the PR description says was avoided. - The EF-to-SQLite migration uses `Mode=Memory;Cache=Shared` with a GUID-unique data source per test/class and a held-open connection, matching the pattern already used elsewhere in the suite (`EfCoreJtiReplayStoreTests`, `SpaceNetworkFixture`-adjacent code). No correctness bugs found. The scope and reasoning in the PR description are unusually well-substantiated (line-cited verification, explicit reasons for every file left unmigrated), and the code matches what it claims. --- ⏱ 2m 18s · 18 turns · tokens: 113.0k in / 9.4k out (+2162.0k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-176/run-628.md)

Grandiras merged commit 6981f3f3e8 into main

2026-09-26 23:02:08 +00:00

Grandiras deleted branch issue-137-tests-2

2026-09-26 23:02:09 +00:00

Grandiras referenced this pull request from a commit

2026-09-26 23:02:10 +00:00

test: store contract tests, EF tests on SQLite, self-provisioning integration CI (#176)

Grandiras referenced this pull request from a commit

2026-09-27 10:52:36 +00:00

docs(changelog): drop a #137 bullet duplicated by the rebase (#139)

Grandiras referenced this pull request from a commit

2026-09-27 10:52:37 +00:00