test(postgrest): migrate PostgRESTTests to Swift Testing, keep Mocker (Phase 4)#1095
Merged
grdsdev merged 3 commits intoJul 21, 2026
Conversation
Coverage Report for CI Build 29828798480Coverage increased (+0.4%) to 83.943%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions6 previously-covered lines in 2 files lost coverage.
Coverage Stats
💛 - Coveralls |
6 tasks
grdsdev
added a commit
that referenced
this pull request
Jul 9, 2026
Converts the largest AuthTests file (87 tests, 81 Mocker Mock() calls, 53 curl-snapshot assertions) to Swift Testing + Replay, per the Phase 0 decision (SDK-1247) and the Phase 4/Phase 3 conversion patterns (PostgRESTTests PR #1095, StorageTests PR #1096). - Curl-snapshot request-shape assertions dropped per the Phase 0 "no shim" decision, replaced by Replay's matcher-based enforcement. - Query strings preserved via exact-URL matching (.method, .url) rather than path-only matching, so query-string regressions are still caught (the gap PR #1095's follow-up commit found and fixed). - Request bodies verified via a custom .custom matcher that decodes both sides as AnyJSON for structural comparison (key order/whitespace insensitive), following the pattern from StorageTests PR #1096's jsonBody(of:) helper -- important here since most of this file's Mocker bodies carry auth-relevant payloads (passwords, tokens, PKCE code verifiers, MFA/passkey credentials). - withMainSerialExecutor wrapping moved from XCTestCase.invokeTest into the handful of @test bodies that actually spawn concurrent authStateChanges-driven Tasks. - HTTPResponse.stub(...) helper (used cross-file by SessionManagerTests) preserved at the bottom of the file. Linear: SDK-1252
grdsdev
added a commit
that referenced
this pull request
Jul 9, 2026
… bugs - RequestsTests.swift: converts the last remaining XCTest/Mocker file. These tests only verify request routing (method+path); response bodies are minimal but decodable fixtures where AuthClient actually decodes a response (Session/User), reusing AuthClientTests' MockData. - Fixes two real bugs surfaced by actually running the suite (not just compiling): AuthClientTests/SessionManagerTests used `.method, .url` exact-URL matching, which broke on percent-encoding differences between how the stub URL string parses vs. how the live request is constructed (e.g. `:`/`/` in a redirect_to query value). Switched to `.method, .path, .query` throughout, matching the Matcher type's own documented guidance to prefer composed matchers over whole-URL comparison. - SessionManagerTests/AuthClientTests: withMainSerialExecutor mutates a process-global flag; Swift Testing runs same-suite tests concurrently by default, so mark both suites .serialized to avoid cross-test interference, mirroring the _clock precedent in PostgrestBuilderTests (PR #1095). - setSessionWithAFutureExpirationDate/updateUser: fixed stubs that decoded the wrong response type (Session vs User) — caught by running tests, not visible from a clean compile. swift build --target AuthTests: 0 errors. swift test --filter AuthTests: 206/206 passed. Linear: SDK-1252
Same Phase 4 migration as #1095, but stays on Mocker for HTTP mocking instead of Replay -- Replay isn't proven yet for all our cases, so we stick with Mocker for now. Restores .snapshotRequest curl-body assertions everywhere instead of Replay's exact-URL/.query/.custom matcher scheme (which existed only to claw back query/body coverage that path-only Replay stubs had silently dropped in #1095's first pass) -- Mocker gives that coverage for free. PostgresQueryTests.swift's XCTestCase-inheritance base class becomes a composed PostgrestQueryFixture struct (renamed file), since Swift Testing suites don't support shared state through subclassing. The 5 suites built on that fixture (Builder/Filter/Query/Rpc/Transform) share a process-global Mocker registry, so they're nested under a shared PostgrestMockerTests(.serialized) namespace to keep them from running concurrently with each other -- Swift Testing applies .serialized recursively to nested suites. PostgrestBuilderTests is a plain struct now, not a class with deinit: main already refactored PostgrestBuilder off the process-global _clock test seam (see the clock-DI commit) in favor of explicit per-test clock injection via makeSUTWithCustomFetch, so the deinit-based global-clock reset #1095 needed no longer applies. Ported forward the tests/fixes that landed on main after #1095's branch point: notIn/dryRun/maybeSingle coverage and the Prefer-header fix. Linear: SDK-1251
Mocker's mock registry is process-global across the whole test binary, not just within one test target. StorageTests' StorageMockerTests and PostgRESTTests' PostgrestMockerTests namespaces each already serialize their own suites internally (.serialized), but nothing stopped the two *targets* from running concurrently with each other -- Swift Testing schedules suites from every compiled test target against one shared executor. That let one target's Mock.register()/lookup interleave with the other's, causing intermittent "no matching stub" failures (and, in one case, silently wrong response data) that only reproduced when both targets' tests ran together, never in isolation. Add a small process-wide async lock (Sources/TestHelpers/MockerSerialization.swift) exposed as a `.mockerSerialized` Swift Testing trait, and apply it directly to every concrete Mocker-backed suite in both targets (StorageBucketAPITests, StorageFileAPITests, and the five PostgrestQueryFixture-based suites). The trait's `isRecursive` must stay false and be applied suite-by-suite, not recursively via an enclosing namespace: marking it recursive on the otherwise test-less StorageMockerTests/PostgrestMockerTests enums (whose nested suites are declared via cross-file `extension`) crashes Swift Testing's trait-application graph with a stack overflow. Verified: `swift test --filter "StorageTests|PostgRESTTests"` and the full suite pass cleanly across many repeated runs; before this fix both failed close to every time.
grdsdev
force-pushed
the
guilherme/sdk-1251-swift-testing-migration-phase-4-postgresttests
branch
from
July 21, 2026 10:38
6072467 to
4bf9f25
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe pull request migrates PostgREST tests from XCTest to Swift Testing, adds shared query fixtures and deterministic request mocking, and introduces process-wide Mocker serialization through a Swift Testing trait. Query, filter, RPC, builder, transform, and Storage suites receive updated annotations and assertions while retaining existing request snapshots and behavioral coverage. Package configuration switches snapshot dependencies and enables Swift 6 checking for PostgREST tests. Possibly related PRs
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
grdsdev
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates the PostgREST test target from XCTest to Swift Testing while preserving existing Mocker-based request verification, and adds a cross-target serialization trait to prevent Mocker registry races when multiple Swift Testing targets run together.
Changes:
- Converted
Tests/PostgRESTTestssuites to Swift Testing (@Suite/@Test+#expect) and replaced sharedXCTestCaseinheritance with a composedPostgrestQueryFixture. - Added a process-wide
.mockerSerializedtrait inTestHelpersand applied it to Storage/PostgREST Mocker-backed suites to avoid cross-target flakiness. - Updated
Package.swiftto includePostgRESTTestsin full Swift 6 checking and removed an unused test dependency.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/StorageTests/StorageFileAPITests.swift | Applies .mockerSerialized to serialize Mocker-backed suite across targets. |
| Tests/StorageTests/StorageBucketAPITests.swift | Documents cross-target Mocker race and applies .mockerSerialized. |
| Tests/PostgRESTTests/PostgrestTransformBuilderTests.swift | Migrates suite to Swift Testing and composes PostgrestQueryFixture. |
| Tests/PostgRESTTests/PostgrestRpcBuilderTests.swift | Migrates RPC builder tests to Swift Testing under serialized Mocker boundary. |
| Tests/PostgRESTTests/PostgrestResponseTests.swift | Converts response unit tests to Swift Testing assertions. |
| Tests/PostgRESTTests/PostgrestQueryFixture.swift | Introduces composed fixture + serialized namespace for PostgREST Mocker suites. |
| Tests/PostgRESTTests/PostgrestQueryBuilderTests.swift | Converts query builder request/behavior tests to Swift Testing. |
| Tests/PostgRESTTests/PostgrestFilterValueTests.swift | Converts filter value unit tests to Swift Testing. |
| Tests/PostgRESTTests/PostgrestFilterBuilderTests.swift | Converts filter builder request-shape tests to Swift Testing. |
| Tests/PostgRESTTests/PostgrestBuilderTests.swift | Converts builder tests (incl. retry tests) to Swift Testing and uses fixture. |
| Tests/PostgRESTTests/PostgresQueryTests.swift | Removes the old XCTest base class in favor of composition. |
| Tests/PostgRESTTests/JSONTests.swift | Converts JSON decoding tests to Swift Testing. |
| Tests/PostgRESTTests/BuildURLRequestTests.swift | Keeps SnapshotTesting curl snapshots; updates to Swift Testing + #filePath. |
| Sources/TestHelpers/MockerSerialization.swift | Adds process-wide async lock + .mockerSerialized Swift Testing trait. |
| Package.swift | Drops unused InlineSnapshotTesting dep for PostgRESTTests and enables Swift 6 mode. |
| dictionary.txt | Adds “subclassing” to cspell dictionary. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Tests/PostgRESTTests/PostgrestBuilderTests.swift`:
- Around line 61-68: Add Issue.record("Expected error to be thrown") immediately
after the awaited execute() call in executeWithNonSuccessStatusCode at
Tests/PostgRESTTests/PostgrestBuilderTests.swift#L61-L68 and
executeWithNonJSONError at
Tests/PostgRESTTests/PostgrestBuilderTests.swift#L83-L91, while preserving the
existing PostgrestError assertions in each catch block.
In `@Tests/PostgRESTTests/PostgrestRpcBuilderTests.swift`:
- Around line 88-110: Add an Issue.record("Expected error to be thrown")
immediately after execute() in both
rpcWithGetMethodAndNonJSONObjectShouldThrowError and
rpcWithHeadMethodAndNonJSONObjectShouldThrowError, so each test fails when no
error is thrown while preserving the existing PostgrestError assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc7cdf57-328e-48f2-ba0b-5276c5dd244a
📒 Files selected for processing (16)
Package.swiftSources/TestHelpers/MockerSerialization.swiftTests/PostgRESTTests/BuildURLRequestTests.swiftTests/PostgRESTTests/JSONTests.swiftTests/PostgRESTTests/PostgresQueryTests.swiftTests/PostgRESTTests/PostgrestBuilderTests.swiftTests/PostgRESTTests/PostgrestFilterBuilderTests.swiftTests/PostgRESTTests/PostgrestFilterValueTests.swiftTests/PostgRESTTests/PostgrestQueryBuilderTests.swiftTests/PostgRESTTests/PostgrestQueryFixture.swiftTests/PostgRESTTests/PostgrestResponseTests.swiftTests/PostgRESTTests/PostgrestRpcBuilderTests.swiftTests/PostgRESTTests/PostgrestTransformBuilderTests.swiftTests/StorageTests/StorageBucketAPITests.swiftTests/StorageTests/StorageFileAPITests.swiftdictionary.txt
💤 Files with no reviewable changes (1)
- Tests/PostgRESTTests/PostgresQueryTests.swift
Contributor
|
The following capabilities are marked
The following capabilities are marked
These may have been renamed, removed, or never registered. Please update the capability matrix. |
grdsdev
added a commit
that referenced
this pull request
Jul 21, 2026
Same cross-target Mocker race fixed for StorageTests/PostgRESTTests in #1095: Mocker's mock registry is process-global across the whole test binary, so FunctionsClientTests' own .serialized trait only protects it from itself, not from StorageTests/PostgRESTTests' Mocker-backed suites running concurrently in the same swift-testing execution. Adds the same Sources/TestHelpers/MockerSerialization.swift helper (.mockerSerialized trait) and applies it alongside .serialized on FunctionsClientTests. Verified in isolation (30/30, run 3x) and, by temporarily pairing with #1095's fixed StorageTests files, cross-target against Storage (run 5x, clean) -- the full local suite still shows the race today only because main's merged StorageTests doesn't have the fix yet; that lands with #1095.
4 tasks
spydon
approved these changes
Jul 21, 2026
Four error-path tests (executeWithNonSuccessStatusCode,
executeWithNonJSONError, rpcWithGetMethodAndNonJSONObjectShouldThrowError,
rpcWithHeadMethodAndNonJSONObjectShouldThrowError) asserted only inside
their catch block -- if execute() stopped throwing (a regression), the
do block would complete and the test would pass vacuously. Add
Issue.record after execute() in each, mirroring the pattern already
used elsewhere in this file (e.g. the stripNulls/csv combination tests).
Rename cSV/cSVWithStripNullsThrowsError to csv/csvWithStripNullsThrowsError
for standard lowerCamelCase test naming.
(Copilot also flagged `import HTTPTypes` in PostgrestBuilderTests.swift
as unused -- verified false: removing it breaks the build, since
`.init("apikey")!` implicitly constructs an HTTPField.Name and needs
the module in scope. Left it in place.)
Contributor
Author
|
Addressed in 334b571:
Verified: 105/105 tests pass (3 runs), full suite green, format/spell-check clean. |
grdsdev
deleted the
guilherme/sdk-1251-swift-testing-migration-phase-4-postgresttests
branch
July 21, 2026 12:49
grdsdev
added a commit
that referenced
this pull request
Jul 21, 2026
Same cross-target Mocker race fixed for StorageTests/PostgRESTTests in binary, so FunctionsClientTests' own .serialized trait only protects it from itself, not from StorageTests/PostgRESTTests' Mocker-backed suites running concurrently in the same swift-testing execution. Adds the same Sources/TestHelpers/MockerSerialization.swift helper (.mockerSerialized trait) and applies it alongside .serialized on FunctionsClientTests. Verified in isolation (30/30, run 3x) and, by temporarily pairing with #1095's fixed StorageTests files, cross-target against Storage (run 5x, clean) -- the full local suite still shows the race today only because main's merged StorageTests doesn't have the fix yet; that lands with #1095.
3 tasks
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.
Summary
Migrates all 10 files in
Tests/PostgRESTTests(~2900 lines) from XCTest to Swift Testing (#expect/#require/@Test/@Suite), per the Phase 0 decision (SDK-1247). Keeps Mocker for HTTP mocking — Replay isn't proven yet for all our cases, so we're sticking with Mocker for now (may revisit later). This is Phase 4 of the migration tracked in SDK-435.Changes
import XCTestremains in the target..snapshotRequest { curl... }curl-body assertion is kept as-is — Mocker gives full query-string/body/header verification for free, so none of the extra matcher machinery an equivalent Replay port would need is necessary here.PostgresQueryTests.swift, anXCTestCasebase class shared via inheritance by 5 other test suites, is replaced by a composedPostgrestQueryFixturestruct (renamed toPostgrestQueryFixture.swift), since Swift Testing suites don't support shared state through subclassing.PostgrestBuilderTestsis a plainstruct(nodeinit/class needed): the process-global_clocktest seam it used to reset was already removed by a separate refactor that injects the clock explicitly per test viamakeSUTWithCustomFetch.BuildURLRequestTestskeeps itsassertSnapshot(as: .curl)request-shape snapshots, switching#file→#filePathsince Swift Testing resolves#fileto a module-relative path (breaks snapshot directory resolution under bothswift testandxcodebuild).mainafter this branch's original base:notIn/dryRun/maybeSinglemodifier tests and theselect(count:)Prefer-header fix test.PostgRESTTestsmoves off the blanket.swiftLanguageMode(.v5)pin onto full Swift 6 language mode, matching production targets. Drops the unusedInlineSnapshotTestingdependency. No new package dependency.Cross-target Mocker race (found while verifying this PR)
The 5 suites built on
PostgrestQueryFixtureshare Mocker's mock registry, so — same asStorageBucketAPITests/StorageFileAPITests— they're nested under aPostgrestMockerTests(.serialized)namespace so they don't run concurrently with each other.That alone wasn't enough: Mocker's registry is process-global across the entire test binary, not per target. Running
PostgRESTTestsand the already-mergedStorageTeststogether reliably failed (swift test --filter "StorageTests|PostgRESTTests"), even though each passed cleanly alone — Swift Testing schedules suites from every compiled test target against one shared executor, soStorageMockerTestsandPostgrestMockerTestscould still interleave and race on Mocker's shared registry.Added
Sources/TestHelpers/MockerSerialization.swift: a small process-wide async lock exposed as a.mockerSerializedSwift Testing trait, applied directly to every concrete Mocker-backed suite in bothStorageTestsandPostgRESTTests. It has to be applied suite-by-suite (isRecursive = false), not recursively via the enclosing namespace enums — marking it recursive there crashes Swift Testing's trait-application graph (stack overflow) when the namespace's only nested suites are declared via cross-fileextension.This touches
StorageTests(already merged, outside this PR's nominal scope) because leaving it out would mean merging this PR immediately reintroduces full-suite flakiness onmain.Note: the pending Functions PR (#1094), once merged, will need the same
.mockerSerializedtreatment onFunctionsClientTests— it has its own Mocker-backed Swift Testing suite that will race against these two the same way.Test plan
swift test --filter PostgRESTTests: 105/105 tests, 10 suites, run 6+ times consecutively — no flakiness (up from the original 97 due to tests added tomainsince).swift test --filter "StorageTests|PostgRESTTests": run 5+ times — reliably failed before the cross-target fix, clean after.swift test --skip IntegrationTests: run 3+ times, no regressions../scripts/spell-check.shand./scripts/format.sh: clean.Linear
Closes SDK-1251