[AIT-1146] Add first spec-derived UTS integration and proxy tests via rewritten uts-to-swift skill#2224
Conversation
…ts-to-kotlin Rebuild the skill to match ably-java's uts-to-kotlin step for step: it now takes a whole UTS module directory (not a single spec file) and runs in two phases — scripted selection (resolve module -> confirm mapping -> pick tier -> pick specs -> translate-only vs translate-and-evaluate), then per-spec translation with a deterministic faithfulness audit. New alongside SKILL.md (v2.0.0): - scripts/resolve_uts.py — validates a module dir, reads uts-package-mapping.json, and emits per-tier target directories plus spec files with derived Swift class names (minimal-diff port of kotlin's; Tests suffix, no package concept). - scripts/audit_translation.py — Step 7 review aid: Test-ID coverage (missing/orphan // UTS: tags) and a per-test spec-line ledger with an assertion-shortfall tripwire. Improves on the kotlin original by skipping comment-only lines when counting assertions, so a commented-out #expect can't mask a dropped one. - uts-package-mapping.json — spec module -> Test/UTS tier-directory mapping (realtime, rest, objects). - references/objects-mapping.md — intentionally-empty placeholder for the future ably-js -> ably-cocoa LiveObjects type map. SKILL.md keeps all cocoa-specific translation content (Ably.Private access ladder, Captured<T>/Sendable rules, mock tables, per-tier file templates) at kotlin's structure, adds the three-outcome evaluation model incl. the spec-error fail-fast pattern, and a dedicated integration/proxy section built on the Test/UTS scoped-resource base cases. Verified: resolver and audit exercised against the full spec corpus (146 files, zero extractor failures) and both existing UTS suite/spec pairs; swift test --filter UTS passes (13/13).
WalkthroughThe UTS-to-Swift workflow now resolves module directories and tiers before translation, provides expanded mocking and integration guidance, adds deterministic audit tooling, and includes new direct-sandbox and proxy reference tests. ChangesUTS-to-Swift translation workflow
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant resolve_uts.py
participant UTS module
participant Swift test target
resolve_uts.py->>UTS module: discover tiers and spec files
resolve_uts.py->>Swift test target: resolve mapped output directories
Swift test target-->>UTS module: translate selected specs
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/skills/uts-to-swift/SKILL.md (2)
304-315: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix Swift 6 concurrency violation in mock setup example.
The variable
attemptCountis mutated inside a mock handler closure, which is marked@Sendablein Swift 6. This will result in a compiler error (Mutation of captured var 'attemptCount' in concurrently-executing code).As recommended later in the document, use the thread-safe
Captured<T>helper to branch logic on the attempt count instead.🐛 Proposed fix
-var attemptCount = 0 -let wsProvider = MockWebSocketProvider(onConnectionAttempt: { connection in - attemptCount += 1 - if attemptCount == 1 { +let capturedAttempts = Captured<MockWebSocket>() +let wsProvider = MockWebSocketProvider(onConnectionAttempt: { connection in + capturedAttempts.append(connection) + if capturedAttempts.count == 1 { connection.respondWithSuccess() connection.sendToClient(.connected(connectionId: "conn-s", connectionKey: "key-s", connectionStateTtl: 2)) } else { connection.respondWithRefused() } })🤖 Prompt for 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. In @.claude/skills/uts-to-swift/SKILL.md around lines 304 - 315, Update the mock setup example around MockWebSocketProvider to replace the captured mutable attemptCount variable with the thread-safe Captured<Int> helper, and use its value/update operations inside the `@Sendable` connection handler to preserve first-attempt success and subsequent-attempt refusal behavior.
576-586: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix Swift syntax error and leaked continuation.
This snippet contains two issues:
- The
guard let date else { ... }block lacks a scope exit (e.g.,returnorthrow), which will cause aguard body must not fall throughcompiler error.- If a
returnis simply added, the continuation will leak, causing the test task to hang indefinitely.Since the documentation text above instructs to "make the test
async throws", update this helper to useasync throwsandwithCheckedThrowingContinuationto resolve both issues idiomatically.🐛 Proposed fix
-private func awaitTime(_ rest: ARTRest, sourceLocation: SourceLocation = `#_sourceLocation`) async -> Date { - await withCheckedContinuation { (continuation: CheckedContinuation<Date, Never>) in +private func awaitTime(_ rest: ARTRest, sourceLocation: SourceLocation = `#_sourceLocation`) async throws -> Date { + try await withCheckedThrowingContinuation { continuation in rest.time { date, error in - if let error { Issue.record("time() failed: \(error)", sourceLocation: sourceLocation) } - guard let date else { Issue.record("time() failed: date should not be nil", sourceLocation: sourceLocation) } + if let error { + Issue.record("time() failed: \(error)", sourceLocation: sourceLocation) + continuation.resume(throwing: error) + return + } + guard let date else { + Issue.record("time() failed: date should not be nil", sourceLocation: sourceLocation) + continuation.resume(throwing: CancellationError()) + return + } continuation.resume(returning: date) // resume exactly once } } }🤖 Prompt for 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. In @.claude/skills/uts-to-swift/SKILL.md around lines 576 - 586, Update awaitTime to be async throws and use withCheckedThrowingContinuation with a CheckedContinuation<Date, Error>. In the rest.time callback, throw or resume the continuation with the received error, throw a descriptive error when date is nil, and resume with the date on success, ensuring every path completes exactly once.
🤖 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 @.claude/skills/uts-to-swift/scripts/audit_translation.py:
- Around line 128-139: Update the fence-tracking logic around FENCE_RE and
in_fence so only triple-backtick fences explicitly labeled pseudo open an
extraction block; ignore Swift, JSON, shell, and unlabeled fences entirely.
Preserve the existing heading, preamble, and line collection behavior for
content inside pseudo fences.
- Around line 167-172: Update the assertion extraction loop in
audit_translation.py to lexically remove Swift block comments, trailing
comments, and string literals before applying SWIFT_ASSERT_RE, rather than only
skipping comment-only lines. Count assertion and poll-helper calls exclusively
from executable Swift text while preserving the existing assertionCalls and
assertionCount output.
- Around line 59-63: Separate assertion and await detection in
audit_translation.py: update SWIFT_ASSERT_RE and the related counting logic to
track `#expect/`#require independently from awaitConnectionState,
awaitChannelState, awaitState, pollUntil, and poll helpers. Compare the
resulting Swift assertion count with specAssertCount and the Swift await count
with specAwaitCount, including the corresponding logic around the additional
reported location.
- Line 13: Remove the unnecessary backslash escapes before backticks in the
documentation text within audit_translation.py, including the Test-ID coverage
description. Keep the backticks unescaped so the file passes Ruff W605 without
changing the documented meaning.
---
Outside diff comments:
In @.claude/skills/uts-to-swift/SKILL.md:
- Around line 304-315: Update the mock setup example around
MockWebSocketProvider to replace the captured mutable attemptCount variable with
the thread-safe Captured<Int> helper, and use its value/update operations inside
the `@Sendable` connection handler to preserve first-attempt success and
subsequent-attempt refusal behavior.
- Around line 576-586: Update awaitTime to be async throws and use
withCheckedThrowingContinuation with a CheckedContinuation<Date, Error>. In the
rest.time callback, throw or resume the continuation with the received error,
throw a descriptive error when date is nil, and resume with the date on success,
ensuring every path completes exactly once.
🪄 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: d55f5b3a-3f92-4430-8f4a-35e626f9625c
📒 Files selected for processing (5)
.claude/skills/uts-to-swift/SKILL.md.claude/skills/uts-to-swift/references/objects-mapping.md.claude/skills/uts-to-swift/scripts/audit_translation.py.claude/skills/uts-to-swift/scripts/resolve_uts.py.claude/skills/uts-to-swift/uts-package-mapping.json
There was a problem hiding this comment.
Pull request overview
This PR rewrites the .claude/skills/uts-to-swift skill to operate on an entire UTS module directory (mirroring the uts-to-kotlin workflow) rather than a single spec file, and adds deterministic resolver/audit scripts to drive selection and translation review.
Changes:
- Add
resolve_uts.pyto validate a UTS module directory, resolve tier targets viauts-package-mapping.json, and list candidate specs with derived Swift class names. - Add
audit_translation.pyto mechanically compare spec Test IDs / pseudocode ledger vs generated Swift (// UTS:tags + assertion/wait calls). - Update
SKILL.mdto document the new two-phase module-directory flow and introduce module mapping + audit steps (plus a placeholder objects mapping reference).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .claude/skills/uts-to-swift/uts-package-mapping.json | Defines module→tier target directory mapping under Test/UTS, plus optional per-module notes reference. |
| .claude/skills/uts-to-swift/SKILL.md | Replaces single-spec flow with module-directory selection + per-spec translation/evaluation + audit-driven review guidance. |
| .claude/skills/uts-to-swift/scripts/resolve_uts.py | New deterministic resolver for module validation, tier discovery, target-dir resolution, and spec/class-name enumeration. |
| .claude/skills/uts-to-swift/scripts/audit_translation.py | New deterministic audit tool for Test-ID coverage and pseudocode-vs-Swift completeness tripwires. |
| .claude/skills/uts-to-swift/references/objects-mapping.md | Placeholder notes file for future objects (LiveObjects) mapping work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nly fences Apply four fixes from the PR #2224 review: - audit_translation.py: count Swift assertions (#expect/#require) and infra wait/poll calls SEPARATELY, mirroring the spec side's specAssertCount/ specAwaitCount — under the previous combined count, a surplus of waits could mask dropped assertions (demonstrably: RTN16f has 2 spec asserts and 3 wait calls, so deleting both assertions kept the shortfall negative). assertionShortfall stays the primary tripwire; awaitShortfall is a softer secondary signal since continuation-bridged helpers legitimately replace infra wait calls. Adds swiftAwaitCalls/Count, awaitShortfall, swiftAwaitTotal and testsWithAwaitShortfall to the report. - audit_translation.py: only ```pseudo fences open ledger extraction — other labelled and unlabelled fences (json fixtures, examples) are skipped, making the code match its own docstring. Output-neutral on the current corpus (146 spec files, zero ledger diffs): every non-pseudo fence today sits in a file preamble, outside the test blocks. - audit_translation.py: drop invalid backslash-backtick escapes from the module docstring (Ruff W605; SyntaxWarning on Python 3.12+). - resolve_uts.py: expanduser() on the module path so ~-prefixed paths resolve instead of failing with a misleading DIR_NOT_FOUND. SKILL.md Step 7 updated to describe the split counts and the new awaitShortfall secondary-signal checklist item. Verified: both real spec/test pairs clean under the split with no false positives; mutation test still trips the assert tripwire; whole-corpus audit sweep unchanged (146 files, 0 crashes, 0 spec-ledger diffs vs the previous revision).
…e tags, neutral paths Apply three fixes from the PR #2224 re-review: - resolve_uts.py: fail fast with a new BAD_MAPPING error when the mapping's testRoot is blank/missing/non-string — previously a corrupted mapping silently emitted absolute-looking nonsense like "/unit/realtime". Validated before --create so a corrupted file is never written back; trailing slash normalised. - audit_translation.py: detect duplicated // UTS: tags — the per-tag dict keeps only the last method's block, so a copy-paste-duplicated tag made a method invisible to the audit while coverage read clean. Reported as idCoverage.duplicateInSwift and included in the exit-2 gate alongside missing/orphan IDs (a duplicate is a finding, not an inability to run, so no hard failure; blocks are deliberately not merged). - SKILL.md: replace the personal example paths with <cloned-ably-specification-repo-path>/uts/objects — explicit that the spec repo must be cloned first (usage guard now links to ably/specification), and uts/ sits at the spec repo root. Verified: all three BAD_MAPPING variants fire and --create leaves a broken mapping untouched; a duplicate-tag mutation exits 2 flagging both the shared tag and the displaced missing id; corpus-wide Test IDs confirm no legitimate duplicates exist within any spec file (same spec *point* across many tests is common and unaffected — tags compare full structured IDs); both real spec/test pairs regression-clean.
…rop env gating
The first spec-derived integration tests now exist for both real-backend
tiers, translated from the UTS specs via the uts-to-swift skill:
- integration/standard/realtime/ChannelHistoryTests.swift — RTL10d
(channel_history_test.md): cross-client history, JSON + msgpack variants.
- integration/standard/realtime/TokenRequestTests.swift — RSA9/RSA9a/RSA9g
(token_request_test.md): locally signed TokenRequest accepted by the server.
- integration/proxy/realtime/AuthReauthTests.swift — RTN22/RTC8a
(auth_reauth.md): injected server-initiated AUTH triggers re-auth without
disrupting the connection; verified via the proxy event log. macOS-only.
Per their TODOs, the interim infra acceptance ("smoke") tests are removed —
the spec-derived tests cover their ground end-to-end.
All env-var gating is dropped from the integration suites so every tier runs
directly from swift test and the Xcode test runner UI; tier selection is by
--filter, mirroring ably-java's per-tier Gradle tasks. UTS_PROXY_LOCAL_PATH
(config override) and RUN_DEVIATIONS (deviation mechanism) remain.
Test/UTS/README.md is brought to block-level parity with ably-java's
uts/README.md: §2 tier table now carries "Example in this repo" file paths;
§3 lists all five derived specs and uses the current fail-fast decision-tree
wording; §7 unit walkthrough restructured to numbered sub-sections; §10
rewritten around ungated per-suite runs (+ the Notes block); §11.4/§11.5
walkthroughs now walk the real ChannelHistoryTests/AuthReauthTests with code
excerpts at the same points as ably-java's §10/§11; §11.3 gains the control
API endpoints and common-actions list; §12 cheat-sheet gains the build-client
snippets, proxy-log inspect, Test ID format and decision-tree entries.
SKILL.md: reference examples now point at the spec-derived suites (matching
uts-to-kotlin), generated tests are instructed to be ungated, and the Step 7
checklist restores kotlin's setup_synced_channel awaited-operation example.
Verified: all three suites pass ungated against the real sandbox
(TokenRequest 2/2, ChannelHistory 2/2 JSON+msgpack, AuthReauth 1/1); unit
suites 11/11; swift build --build-tests clean; make lint clean; zero
smoke/env-gate references remain in code or docs.
🎯 What this PR does
Rewrites the
uts-to-swiftskill (v2.0.0) to mirror ably-java'suts-to-kotlinstep for step. The skill previously translated one spec file at a time with hand-maintained path tables and an eyeball-only review; it now drives the whole module-directory workflow the kotlin skill established, backed by two deterministic scripts:Built on top of the
Test/UTSintegration/proxy infrastructure introduced by #2223 (hence the base branch).The PR also ships the skill's first real output: three spec-derived integration tests that replace the interim smoke tests, with all env-var gating dropped and the UTS docs brought to block-level parity with ably-java — see the spec-derived tests section below.
🧭 The new two-phase flow
Phase 1 — Selection (scripted, byte-for-byte deterministic):
scripts/resolve_uts.pyvalidates the module directory, readsuts-package-mapping.json, and emits per-tier target directories plus every candidate spec with its derived Swift class name (connection_recovery_test.md→ConnectionRecoveryTests).Phase 2 — Per-spec translation (Steps 1–7): read spec (+ per-module translation notes) → resolver-provided output path → read the tier's infra → generate → compile → run (evaluate mode only) → audit-driven review.
📦 What's new alongside SKILL.md
scripts/resolve_uts.py--createmaintains the mapping. Minimal-diff port of kotlin's (only deltas:Testssuffix, no Kotlin package concept, cocoa wording).scripts/audit_translation.pymissingInSwift/orphanInSwiftfrom// UTS:tags) + a per-test spec-line ledger with an assertion-shortfall tripwire.uts-package-mapping.jsonTest/UTStier-directory mapping (realtime,rest,objects).references/objects-mapping.mdobjectstranslation starts).✍️ SKILL.md highlights
@BeforeAll/finallyteardown, and JSON forced explicitly on the proxy tier.import Ably.Privateinternals-access ladder,Captured<T>/Swift-6-Sendable rules, paired pseudocode→Swift mock examples, and per-tier file templates (unit / direct-sandbox / proxy).Issue.record) so the spec gets fixed at source. The accommodate-both pattern stays banned.writing-derived-tests.mdup front, anduts/docs/proxy.mdbefore any proxy-tier translation (with a local-checkout fallback while the upstreamdocs/move is unmerged).respondWithTimeout/respondWithDNSError(extend-or-record instruction), and fixture-driven specs (e.g.msgpack_interop.md) legitimately yieldspecCount: 0+ an expected orphan tag in the audit.🔍 One deliberate improvement over the kotlin original
audit_translation.pyskips comment-only lines when counting assertions — a commented-out// #expect(…)is not an assertion, and counting it (as kotlin's script does with a commentedassertEquals) would silently mask exactly the dropped-assertion case the tripwire exists to catch. Worth upstreaming touts-to-kotlin.✅ Verification (initial skill commit)
NOT_A_UTS_MODULE_PATH,DIR_NOT_FOUND,NO_TIER_DIRS,MAPPING_NOT_FOUND,BAD_TARGET_NAME) verified;--createround-trip preservesnotesand LF endings.ConnectionRecoveryTests,TimeTests— the one shortfall it flags isTimeTests' documented annotated omission); mutation tests confirm missing-test, orphan-tag, and shortfall detection; swept over the entire spec corpus (146 files) with zero extractor failures.uts-to-kotlinwith only the documented deltas; every snippet API verified against the actualTest/UTS/infrasignatures.swift test --filter UTSpasses;make lintclean; the initial commit touches nothing underSource/orTest/(later commits in this PR do add tests underTest/UTS/— see below).🔁 Review follow-ups (
7e383be3,455f4ca7)All ten review comments across two rounds are addressed in the threads; the code changes:
swiftAssertionCountvsswiftAwaitCount,assertionShortfallprimary /awaitShortfalladvisory) — a wait surplus could previously mask deleted assertions (proven onRTN16f).```pseudo-labelled blocks feed the spec ledger (corpus-verified output-neutral today).duplicateInSwiftinidCoverage+ the exit-2 gate — a duplicated// UTS:tag previously made a method invisible to the audit (reported, not merged; not a hard failure).BAD_MAPPINGfail-fast for a blank/invalidtestRoot(validated before--createcan persist corruption) andexpanduser()for~-paths in the resolver.<cloned-ably-specification-repo-path>/uts/objects).🧪 First spec-derived integration tests (
58fa0f5e)The rewritten skill was then used for real (translate-and-evaluate mode) — this PR includes its first output, replacing the interim infra "smoke" tests per their TODOs:
integration/standard/realtime/ChannelHistoryTests.swiftchannel_history_test.md(RTL10d)integration/standard/realtime/TokenRequestTests.swifttoken_request_test.md(RSA9/RSA9a/RSA9g)TokenRequestaccepted by the real server, with/withoutclientIdintegration/proxy/realtime/AuthReauthTests.swiftauth_reauth.md(RTN22/RTC8a)authCallback+ client→server AUTH frame in the proxy log, connection undisrupted. macOS-onlyIntegrationSmokeTest/ProxyInfraSmokeTestsremoved — the spec-derived tests cover their ground end-to-end (sandbox provisioning, real clients, proxy chain, log assertions).UTS_INTEGRATION_*): every tier now runs directly fromswift testand the Xcode test runner UI; tier selection is by--filter, mirroring ably-java'srunUtsUnitTests/runUtsIntegrationTestssplit.UTS_PROXY_LOCAL_PATH(config override) andRUN_DEVIATIONS(deviation mechanism) remain.Test/UTS/README.mdbrought to block-level parity with ably-java'suts/README.md: §2 tier table with "Example in this repo" paths, §7 walkthrough restructured to numbered sub-sections, §10 rewritten around ungated per-suite runs, §11.4/§11.5 now walk the realChannelHistoryTests/AuthReauthTestswith code excerpts at the same points as ably-java's §10/§11, §11.3 control-API endpoints + common-actions list, §12 cheat-sheet build-client snippets / Test-ID format / decision-tree entries.ChannelHistoryTestreference), and generated tests are instructed to stay ungated.Verification: all three suites pass ungated against the real sandbox (TokenRequest 2/2, ChannelHistory 2/2 JSON+msgpack, AuthReauth 1/1 incl. proxy binary sync); unit suites 11/11;
swift build --build-testsandmake lintclean; zero smoke/env-gate references remain in code or docs.Summary by CodeRabbit
New Features
Documentation