Sync integration/liveobjects with main - #2232
Open
maratal wants to merge 9 commits into
Open
Conversation
Restructure the UTS target to mirror ably-java's uts module (tier-then-module): Harness/ -> infra/unit/, Tests/<module>/unit/ -> unit/<module>/. Add the integration tier's infrastructure, mirroring ably-java's infra/integration: - SandboxApp: provisions/deletes throwaway sandbox apps from ably-common's test-app-setup.json - proxy/ProxyManager (macOS-only): syncs (downloads, SHA-256-verifies, caches at ~/.cache/uts-proxy — shared with ably-java) and launches the pinned uts-proxy release; UTS_PROXY_LOCAL_PATH override; atexit reaper - proxy/ProxySession: typed client for the proxy control REST API (rules, imperative actions, typed event log) + connectThroughProxy client wiring - infra/Utils.swift: shared async pollUntil/awaitState/awaitChannelState and the URLSession plumbing the integration infra is built on Bring the unit infra to helper parity with ably-java: .connectedMessage, respondWithSuccess(_:), respondWithDelay, PendingHTTPConnection.queryParams, pendingScheduledCount, dummy-key seeding in makeRealtime/makeRest, and a configurable MockTimeProvider clock origin. Add env-gated (UTS_INTEGRATION_SMOKE) acceptance tests for both integration tiers: IntegrationSmokeTest (direct sandbox, run per protocol variant: JSON + msgpack) and ProxyInfraSmokeTests (full proxy chain, JSON-only with token auth, as the proxy requires). Rewrite Test/UTS/README.md as a comprehensive human-readable guide mirroring ably-java's uts/README.md (tier walkthroughs, request-flow diagrams, per-file reference, java-to-cocoa helper map) and sync the uts-to-swift skill; update spec links for the helpers/proxy.md -> uts/docs/proxy.md move.
…doc roles Review fixes: - pollUntil / ProxyManager.waitForHealth / SandboxApp's retry backoff now handle task cancellation instead of hot-looping or burning retries; the backoff sleeps only between attempts and its comment matches the delays - UTS_PROXY_LOCAL_PATH installs land in a separate uts-proxy-local file, so a local build can never masquerade as the checksum-verified release cache - ProxySession's doc example shows token auth (the key-based example would throw at runtime: basic auth is TLS-only, RSA1); controlDelete now logs transport-level failures, not just non-2xx statuses - ProxyManager documents the one-test-process-at-a-time advisory (ably-java's single-fork equivalent) rather than a per-process-port redesign - dropped the unverifiable "sandbox apps auto-expire" claim DRY setup/teardown: Swift Testing has no setUp/tearDown hooks and deinit cannot await, so integration suites now subclass scoped-resource base cases: IntegrationTestCase (withSandboxApp / withRealtimeClient) and ProxyTestCase (withProxySession, proxyClientOptions), built on a shared runThenCleanUp(_:body:cleanup:) engine that always tears down and rethrows the body's error after cleanup. Both smoke tests are rewritten on top, every wait is guarded so a timeout can't cascade into secondary failures, and each carries a TODO to remove it once spec-derived tests cover its tier. Docs: split responsibilities — Test/UTS/README.md now only documents the existing setup (authoring templates and procedures moved to the uts-to-swift skill, which gains integration/proxy file templates); README gains the base test cases section, updated walkthroughs, and the java lifecycle mapping.
…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).
…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.
[AIT-847] UTS: align layout with ably-java and add integration/proxy test infrastructure
[AIT-1146] Add first spec-derived UTS integration and proxy tests via rewritten uts-to-swift skill
…ts-sync-with-main
WalkthroughThe PR expands UTS translation from single specs to mapped modules and tiers, adds deterministic resolution and audit scripts, introduces shared unit/integration/proxy infrastructure, documents the workflow, and adds REST, realtime sandbox, and proxy-derived tests. ChangesUTS workflow and validation
Swift UTS test infrastructure
UTS documentation and derived tests
Estimated code review effort: 4 (Complex) | ~60 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 187-199: Update the tag-processing loop in the audit function to
associate each tag with its following `@Test` method, scope scanning to that
method’s matching closing brace, and exclude bottom-of-file helpers. Reject or
otherwise report tags without an attached test method, while preserving
assertion and await collection only within the annotated test method.
In @.claude/skills/uts-to-swift/SKILL.md:
- Around line 575-576: Update the awaitTime helper to remain async throws and
bridge the REST completion using a throwing continuation: when the callback
receives a non-nil error, resume by throwing immediately; when the error is nil
but the date is missing, resume by throwing an appropriate failure; otherwise
resume with the date. Ensure every callback path resumes the continuation
exactly once and preserve the existing REST call flow.
- Line 700: Add the text language identifier to the unlabeled fenced
decision-tree block in the documentation, changing its opening fence to use
```text while preserving the block’s contents and closing fence.
🪄 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 Plus
Run ID: 6f8ffc21-f50a-45c5-9526-a7488fb2dc62
📒 Files selected for processing (25)
.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.jsonTest/UTS/README.mdTest/UTS/infra/Utils.swiftTest/UTS/infra/integration/IntegrationTestCase.swiftTest/UTS/infra/integration/SandboxApp.swiftTest/UTS/infra/integration/proxy/ProxyManager.swiftTest/UTS/infra/integration/proxy/ProxySession.swiftTest/UTS/infra/integration/proxy/ProxyTestCase.swiftTest/UTS/infra/unit/Captured.swiftTest/UTS/infra/unit/CapturingLog.swiftTest/UTS/infra/unit/MockHTTPClient.swiftTest/UTS/infra/unit/MockTimeProvider.swiftTest/UTS/infra/unit/MockWebSocket.swiftTest/UTS/infra/unit/NoOpReachability.swiftTest/UTS/infra/unit/ProtocolMessage.swiftTest/UTS/infra/unit/UTSTestCase.swiftTest/UTS/integration/proxy/realtime/AuthReauthTests.swiftTest/UTS/integration/standard/realtime/ChannelHistoryTests.swiftTest/UTS/integration/standard/realtime/TokenRequestTests.swiftTest/UTS/unit/realtime/ConnectionRecoveryTests.swiftTest/UTS/unit/rest/TimeTests.swift
Comment on lines
+187
to
+199
| for idx, (start, tag) in enumerate(tags): | ||
| end = tags[idx + 1][0] if idx + 1 < len(tags) else len(lines) | ||
| asserts, awaits = [], [] | ||
| for line in lines[start:end]: | ||
| if line.strip().startswith("//"): # comment-only line — not executable | ||
| continue | ||
| for m in SWIFT_ASSERT_RE.finditer(line): | ||
| asserts.append(m.group(1)) | ||
| for m in SWIFT_AWAIT_RE.finditer(line): | ||
| awaits.append(m.group(1)) | ||
| out[tag] = {"assertionCalls": asserts, "assertionCount": len(asserts), | ||
| "awaitCalls": awaits, "awaitCount": len(awaits)} | ||
| return out, duplicates |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Scope counts to the annotated test method, not the next UTS tag.
The final tagged test currently absorbs every bottom-of-file helper extension. Helper #require/wait calls can inflate its counts and mask missing test assertions. Associate each tag with its following @Test function and stop at that function’s matching closing brace; reject unattached tags.
The workflow guidance explicitly puts helpers at the bottom of the file and allows assertions inside them.
🤖 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/scripts/audit_translation.py around lines 187 -
199, Update the tag-processing loop in the audit function to associate each tag
with its following `@Test` method, scope scanning to that method’s matching
closing brace, and exclude bottom-of-file helpers. Reject or otherwise report
tags without an attached test method, while preserving assertion and await
collection only within the annotated test method.
Comment on lines
+575
to
+576
| REST calls will callback (e.g. `rest.time { ... }`) — make the test `async throws` and create a helper | ||
| method that bridges the completion handler with a continuation: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -U 'guard let date else \{\n\s*Issue\.record\([^)]*\)\n\s*\}\n\s*continuation\.resume' \
.claude/skills/uts-to-swift/SKILL.mdRepository: ably/ably-cocoa
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant skill documentation around the REST continuation guidance.
if [ -f .claude/skills/uts-to-swift/SKILL.md ]; then
wc -l .claude/skills/uts-to-swift/SKILL.md
sed -n '540,610p' .claude/skills/uts-to-swift/SKILL.md
else
fd -a 'SKILL.md|uts-to-swift' .
fiRepository: ably/ably-cocoa
Length of output: 3971
Resume after recording non-nil-date completion handler failures.
In awaitTime, the if let error branch also falls through to the date == nil check instead of returning or calling continuation.resume. That leaves the continuation unresumed and changes the function signature from async throws to plain async for no good reason. Handle both error and missing-date results with a throwing continuation/result continuation so every callback path resumes 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 575 - 576, Update the
awaitTime helper to remain async throws and bridge the REST completion using a
throwing continuation: when the callback receives a non-nil error, resume by
throwing immediately; when the error is nil but the date is missing, resume by
throwing an appropriate failure; otherwise resume with the date. Ensure every
callback path resumes the continuation exactly once and preserve the existing
REST call flow.
| Handle failures using this decision tree (see [reference doc](https://github.com/ably/specification/blob/main/uts/docs/writing-derived-tests.md)): | ||
| Handle test failures using this decision tree (the **Required reading** doc you fetched up front has the full detail): | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language to this fenced block.
markdownlint MD040 rejects the unlabeled decision-tree fence.
Proposed fix
-```
+```text📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 700-700: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 at line 700, Add the text language
identifier to the unlabeled fenced decision-tree block in the documentation,
changing its opening fence to use ```text while preserving the block’s contents
and closing fence.
Source: Linters/SAST tools
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
2 participants
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
Merges the latest
mainintointegration/liveobjectsto keep the integration branch up to date and avoid future conflicts.Before this merge,
integration/liveobjectswas 8 commits behindmain. The merge applied cleanly with no conflicts.Notable changes pulled in from
main:Test/UTS/Harness/→Test/UTS/infra/unit/, plus newTest/UTS/infra/integration/(sandbox + proxy) infrastructure.AuthReauthTests,ChannelHistoryTests,TokenRequestTests).uts-to-swiftskill updates (objects mapping, audit/resolve scripts, package mapping).Verification
swift build— succeeds.swift build --build-tests— succeeds (only a pre-existing warning inRealtimeClientConnectionTests.swift).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation