Skip to content

[AIT-1081] Update liveobjects uts-to-kotlin skill to generate tests#1221

Open
sacOO7 wants to merge 9 commits into
feature/path-based-liveobjects-implementationfrom
feature/liveobjects-uts-tests
Open

[AIT-1081] Update liveobjects uts-to-kotlin skill to generate tests#1221
sacOO7 wants to merge 9 commits into
feature/path-based-liveobjects-implementationfrom
feature/liveobjects-uts-tests

Conversation

@sacOO7

@sacOO7 sacOO7 commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator
  • Fixes https://ably.atlassian.net/browse/AIT-1081
  • Generated UTS markdown to have better understanding of existing UTS infra.
  • This is also helpful for a relevant uts-to-kotlin skill to better generate ably-java tests
  • Based on the doc, we have missing sandbox test, need to update the same

📚 Human-readable UTS guide + website

To make the UTS setup approachable, this PR adds a comprehensive guide and a self-contained website, kept in sync:

  • uts/README.md — renders on the module's GitHub page.
  • uts/index.html — a standalone, offline-friendly website (inline SVG flow diagrams, scroll-spy nav, light/dark theme; no external assets).

Both walk through: what UTS is → the three test tiers → the spec docs (linked to GitHub) → the uts/ module layout → the SDK hook points → the unit & proxy infrastructure → the two example tests → deviations → how to run → appendices (flow diagrams + per-file API reference + source map).

🗂️ Test package restructure

Test sources are reorganised under a domain-rooted io.ably.lib.uts package that cleanly separates infrastructure from tests, and unit from integration:

io/ably/lib/uts/
├── infra/                        # test infrastructure (no @Test methods)
│   ├── Utils.kt                  #   shared awaits (awaitState / pollUntil …)
│   ├── unit/                     #   mock transports + ConnectionDetails builder
│   └── integration/              #   SandboxApp
│       └── proxy/                #     ProxyManager, ProxySession
├── unit/realtime/                # UNIT tests          → ConnectionRecoveryTest
└── integration/proxy/realtime/   # INTEGRATION tests   → AuthReauthTest

The unit/infra/unit/ and integration/infra/integration/ pairing is what the new Gradle tasks key off.

Note: the ConnectionDetails test builder no longer lives in io.ably.lib.types, so it now obtains the package-private constructor reflectively (same approach as liveobjects/.../TestUtils.kt).

⚙️ Per-tier Gradle tasks + CI wiring

Added two package-filtered test tasks in uts/build.gradle.kts, mirroring runLiveObjectsUnitTests / runLiveObjectsIntegrationTests:

Task Runs CI
:uts:runUtsUnitTests io.ably.lib.uts.unit.* (mocked, fast) check.yml (PR gate)
:uts:runUtsIntegrationTests io.ably.lib.uts.integration.* (real sandbox + proxy) integration-test.yml → new check-uts job

✅ Validation

  • ./gradlew :uts:runUtsUnitTests → 6/6 pass · :uts:runUtsIntegrationTests → 1/1 pass.
  • Full CI gate checkWithCodenarc checkstyleMain checkstyleTest runUnitTests runLiveObjectsUnitTests :uts:runUtsUnitTestsBUILD SUCCESSFUL.

Summary by CodeRabbit

  • New Features

    • Added a new UTS guide and reference docs for working with unit, integration, and proxy tests.
    • Introduced dedicated Gradle tasks for running UTS unit and integration test suites.
    • Added new integration coverage for channel history and token request flows.
  • Bug Fixes

    • Improved reconnect test timing to better capture connection retries.
    • Updated test helpers and infrastructure packaging for clearer separation between unit and integration support.

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Reorganizes the UTS test infrastructure into sub-packages (infra.unit, infra.integration, infra.integration.proxy), consolidates sandbox host constants into SandboxApp, adds new Gradle tasks (runUtsUnitTests, runUtsIntegrationTests) and a CI integration job, adds new integration tests and LiveObjects unit test helpers, and rewrites the AI skill with a module-directory resolution script, objects-mapping reference, and comprehensive developer documentation.

Changes

UTS infrastructure package reorganization and new tests

Layer / File(s) Summary
Gradle tasks and CI workflow updates
uts/build.gradle.kts, .github/workflows/check.yml, .github/workflows/integration-test.yml
Adds runUtsUnitTests and runUtsIntegrationTests tasks with package-filter patterns, adds :liveobjects runtime dependency and junit-jupiter-params, updates the check workflow to use the unit task, and adds a check-uts CI job that runs integration tests.
Infra package reorganization
uts/src/test/kotlin/io/ably/lib/uts/infra/Utils.kt, uts/src/test/kotlin/io/ably/lib/uts/infra/unit/*, uts/src/test/kotlin/io/ably/lib/uts/infra/integration/SandboxApp.kt, uts/src/test/kotlin/io/ably/lib/uts/infra/integration/proxy/ProxyManager.kt, uts/src/test/kotlin/io/ably/lib/uts/infra/integration/proxy/ProxySession.kt
Moves all unit infra classes to io.ably.lib.uts.infra.unit, moves integration classes from io.ably.lib.test.helper to io.ably.lib.uts.infra.integration.*, introduces SandboxApp.sandboxHost constant, removes ProxyManager sandbox host constants, updates ProxySession.create defaults to use SandboxApp.sandboxHost, removes old ConnectionDetails DSL from types/Utils.kt, and adds a reflective replacement in infra/unit/Utils.kt. Adds parseQueryString helper and PendingConnection interface in the unit sub-package.
Existing test updates
uts/src/test/kotlin/io/ably/lib/uts/unit/realtime/ConnectionRecoveryTest.kt, uts/src/test/kotlin/io/ably/lib/uts/integration/proxy/realtime/AuthReauthTest.kt
Updates package declarations and imports to reference the new infra sub-packages; adds a pollUntil gate in ConnectionRecoveryTest to wait for the second reconnect attempt before asserting query params, fixing a pre-existing race.
New direct-sandbox integration tests
uts/src/test/kotlin/io/ably/lib/uts/integration/standard/realtime/ChannelHistoryTest.kt, uts/src/test/kotlin/io/ably/lib/uts/integration/standard/realtime/TokenRequestTest.kt
Adds ChannelHistoryTest (parameterized RTL10d cross-client history with newest-first ordering assertion) and TokenRequestTest (RSA9a/RSA9g/RSA9 token request auth via SandboxApp).
LiveObjects unit test helpers
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/Helpers.kt
Adds Gson DSL, wire JSON builders for object states/operations/protocol messages, reflective buildPublicObjectMessage, STANDARD_POOL_OBJECTS fixture, SyncedChannel container, and setupSyncedChannel/setupSyncedChannelNoAck suspend helpers using MockWebSocket.

UTS-to-Kotlin AI skill and reference documentation

Layer / File(s) Summary
Package mapping JSON and resolver script
.claude/skills/uts-to-kotlin/uts-package-mapping.json, .claude/skills/uts-to-kotlin/scripts/resolve_uts.py
Adds uts-package-mapping.json defining testRoot and per-module tier-to-path mappings; adds resolve_uts.py that validates module directories, reads/writes the mapping, discovers spec Markdown files by tier, derives Kotlin class names and package strings, and outputs structured JSON.
SKILL.md rewrite
.claude/skills/uts-to-kotlin/SKILL.md
Replaces the single-spec workflow with a two-phase module-directory flow (Phase 1: resolve/validate/select; Phase 2: per-spec translate), updates KDoc/template conventions to use @UTS <uts-id>, adds tier-specific Gradle run commands, and rewrites integration tier and deviation honesty sections.
Objects UTS-to-ably-java translation reference
.claude/skills/uts-to-kotlin/references/objects-mapping.md
New comprehensive reference covering three-layer type disambiguation, entry-point/channel access, Promise-to-CompletableFuture, PathObject/Instance typed hierarchy, LiveMapValue writes, subscriptions/events, ValueType, ObjectMessage/operation types, error codes, internal-graph caveats, worked example, and symbol index.
Developer guide (README and HTML)
uts/README.md, uts/index.html
Adds full developer guide covering UTS concepts, three-tier structure, infra hook points, mock transport and proxy infrastructure, async helpers, three reference test walkthroughs, deviation handling, run instructions, cheat-sheet, and appendices; mirrored as a single-page HTML document with light/dark theming and scroll-spy sidebar.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • ably/ably-java#1209: Directly precedes this PR — removes ConnectionDetails(init) from io.ably.lib.types.Utils.kt and establishes LiveObjects groundwork that this PR builds on.
  • ably/ably-java#1210: Adds the ProxyManager/ProxySession proxy test helpers that this PR refactors into new sub-packages and updates default host constants for.
  • ably/ably-java#1216: Introduces the RealtimeObject API and ChannelBase.object accessor that the new Helpers.kt calls via channel.object.get().

Suggested reviewers

  • lawrence-forooghian

Poem

🐇 Hoppity-hop through packages new,
infra.unit and integration too!
The resolver script maps every spec,
sandboxHost constant — what a tech!
Three tiers of tests, all neatly aligned,
This bunny thinks the stack looks fine! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 34.55% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the core change: updating the uts-to-kotlin skill to generate tests, though it omits the broader docs and refactor work.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/liveobjects-uts-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 09:10 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 09:12 Inactive
Move the human-readable UTS guide and its self-contained HTML rendering
into the uts/ module as README.md and index.html. Both cover the UTS
concept, the three test tiers, the spec docs, the uts/ module layout,
mock/proxy infrastructure, the two example tests, deviations, and
appendices. Spec-doc references link to GitHub; paths are fully
qualified; the two artifacts are kept in sync.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 09:54 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 09:55 Inactive
@sacOO7 sacOO7 changed the title Generated UTS doc to better understand the existing UTS infra. [AIT-1008] Generate LiveObjects tests based on UTS test specs Jun 25, 2026
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 11:08 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 11:09 Inactive
…ier Gradle tasks

Reorganise the uts module under a domain-rooted io.ably.lib.uts package that
cleanly separates infrastructure from tests, and unit from integration:

  infra/                         shared awaits (Utils.kt)
  infra/unit/                    mock transports + ConnectionDetails builder
  infra/integration/             SandboxApp
  infra/integration/proxy/       ProxyManager, ProxySession
  unit/realtime/                 ConnectionRecoveryTest        (mocked)
  integration/proxy/realtime/    AuthReauthTest                (sandbox + proxy)

- The ConnectionDetails test builder no longer sits in io.ably.lib.types, so it
  obtains the package-private constructor reflectively (as liveobjects/TestUtils.kt does).
- Add runUtsUnitTests / runUtsIntegrationTests Gradle tasks (filtered by package),
  mirroring runLiveObjectsUnitTests / runLiveObjectsIntegrationTests.
- check.yml now runs :uts:runUtsUnitTests; integration-test.yml gains a check-uts
  job running :uts:runUtsIntegrationTests.
- Bring uts/README.md and uts/index.html in sync with the new structure.
@sacOO7 sacOO7 force-pushed the feature/liveobjects-uts-tests branch from 884ee87 to aa0504e Compare June 25, 2026 11:25
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 11:26 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 11:27 Inactive
Reflect the new test-source structure in the UTS guide, website, and the
uts-to-kotlin skill:

- Add the direct-sandbox (`integration/standard/<module>/`) tier alongside
  the existing unit and proxy tiers, and document that every tier is now
  organised by module (`realtime`, `liveobjects`, …).
- Update §2 tier table, §4.2 directory tree + mental model, §7.3 SandboxApp
  (shared by both integration kinds), and §12 run commands in README.md, and
  mirror all of it in index.html (tags verified balanced, sections intact).
- Generalise the skill's spec→test path mapping to `<module>`, add a
  direct-sandbox row, and split integration specs into fault-injecting (proxy)
  vs happy-path (direct sandbox) flows.
- Correct stale "both tiers" wording now that there are three tiers.
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 11:44 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 11:46 Inactive
…ckage mapping

Rework the uts-to-kotlin skill to translate a whole UTS module at once
instead of a single spec file:

- Take a UTS module directory (e.g. .../specification/uts/objects) and
  validate it sits directly under uts/ with a standard tier structure.
- Resolve the target ably-java package via uts-package-mapping.json (a new
  config file alongside the skill): a shared `testRoot` parent plus a
  `packages` table mapping each source module to its per-tier output dir
  (so objects -> liveobjects is explicit). Offer to create a mapping when
  one is missing.
- Let the user pick a tier (unit / integration / proxy) and then translate
  all specs or a selected subset, looping each through the existing
  per-spec translation steps.

Phase 1 (selection: Steps A-D) is new; Phase 2 (per-spec translation:
Steps 1-7) keeps the existing rules, with Step 1/2 adjusted to consume the
looped spec and the pre-resolved target.
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 12:27 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 12:29 Inactive
…aluate mode

Make the skill's selection phase deterministic and add an explicit
translate-vs-evaluate choice:

- Add scripts/resolve_uts.py — a bundled resolver that validates the module
  directory, reads uts-package-mapping.json, and emits JSON with, per tier,
  the target dir, Kotlin package, and the candidate specs with derived class
  names. This replaces the per-run hand-work (regex validation, path joins,
  snake_case->PascalCase) that the model previously improvised, so Phase 1 is
  byte-for-byte deterministic. Exclusions are checked relative to the tier
  base (robust to the checkout location), and --create guards the target name.
- Rewrite Phase 1 (Steps A-E) around the resolver: resolve, confirm/create
  mapping, choose tier, choose specs, choose translate-only vs evaluate.
- Gate Step 6 (run/fix) behind evaluate mode per writing-derived-tests.md's
  Translation (always) vs Evaluation (only when an implementation exists)
  split; translate-only stops after compile + review.
- Make the reference fetch mandatory (WebFetch added to allowed-tools).
- Fix the file template to use the resolver's package/className (no hardcoded
  realtime, no double Test suffix) and the spec's full @uts id; correct stale
  uts/test/... proxy doc paths.
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 16:58 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 16:59 Inactive
…rence

The objects UTS specs are written in ably-js-style pseudocode, but ably-java
is a typed SDK (RTTS1-10 partition). Add references/objects-mapping.md mapping
each ably-js symbol to its ably-java equivalent: entry point, async
(CompletableFuture/await), the typed PathObject/Instance hierarchies and as*
casts, the LiveMapValue write union, creation value types, subscriptions,
sync-state events, ValueType, message/operation getters, error codes, path
dot-escaping, and the internal-graph caveat for unit specs.

Wire it in deterministically: each module declares its translation reference
via a `notes` field in uts-package-mapping.json; resolve_uts.py resolves it to
`translationNotes` (absolute path, or null), and SKILL.md makes it required
reading before Phase 2 when present.
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 18:50 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 18:52 Inactive
…ap them

Translate objects/helpers/standard_test_pool.md into ably-java test helpers in
uts/.../unit/liveobjects/helpers.kt: the standard object pool, the protocol- and
object-message builders (emitting the integer-coded wire JSON the SDK's Gson
expects), setupSyncedChannel/NoAck over the existing MockWebSocket, and
buildPublicObjectMessage — which reaches the internal PAOM3/PAOOP3 construction
(WireObjectMessage -> DefaultObjectMessage) by reflection, so it runs today even
though the rest of :liveobjects is unimplemented. Add testRuntimeOnly(:liveobjects)
so that reflection resolves while keeping the compile classpath decoupled.

Wire the mapping reference at it: objects-mapping.md gains a "Unit-test helpers"
section mapping each spec helper to its Kotlin name, and §11/§13 are corrected to
note public_object_message.md is translatable via buildPublicObjectMessage rather
than internal-only.
@sacOO7 sacOO7 force-pushed the feature/liveobjects-uts-tests branch from 8276473 to 752b45e Compare June 25, 2026 19:45
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 19:46 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 19:48 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 20:56 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 20:58 Inactive
@sacOO7 sacOO7 force-pushed the feature/liveobjects-uts-tests branch from e38b889 to 752b45e Compare June 25, 2026 20:59
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/features June 25, 2026 21:00 Inactive
@github-actions github-actions Bot temporarily deployed to staging/pull/1221/javadoc June 25, 2026 21:01 Inactive
…efactor

- Add ChannelHistoryTest (RTL10d) and TokenRequestTest (RSA9/RSA9a/RSA9g)
  direct-sandbox integration tests, parameterised over json/msgpack.
- Add junit-jupiter-params dependency (@ParameterizedTest / @valuesource).
- Consolidate the sandbox host into a single SandboxApp.sandboxHost constant
  (removed from ProxyManager; ProxySession defaults both hosts to it).
- Sync uts/README.md, uts/index.html, and the uts-to-kotlin SKILL.md for the
  direct-sandbox tier: new ChannelHistoryTest walkthrough, integration-tests
  umbrella section, section renumber.
- Rename unit liveobjects helpers.kt -> Helpers.kt (case-only).
@sacOO7 sacOO7 changed the title [AIT-1008] Generate LiveObjects tests based on UTS test specs [AIT-1081] Update liveobjects uts-to-kotlin skill to generate tests Jun 29, 2026
@sacOO7 sacOO7 marked this pull request as ready for review June 29, 2026 18:53
@sacOO7 sacOO7 changed the title [AIT-1081] Update liveobjects uts-to-kotlin skill to generate tests [AIT-1081] Update liveobjects uts-to-kotlin skill to generate tests Jun 29, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
.claude/skills/uts-to-kotlin/SKILL.md (1)

353-374: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make this template tier-specific and use explicit imports.

This scaffold is presented as the resolver-selected tier template, but it still hardcodes unit-only setup (infra.unit.*, MockWebSocket, ConnectionDetails) and star imports. That makes the integration/proxy path easy to start from the wrong skeleton even though later sections require SandboxApp / ProxySession wiring. Split this into per-tier templates (or relabel this one as unit-only) and list concrete imports instead of *.

🤖 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-kotlin/SKILL.md around lines 353 - 374, This template
is still unit-only even though it is presented as the resolver-selected tier
scaffold, and it also relies on star imports. Update the Skill.md template
around the test scaffold to either split it into separate tier-specific
templates or clearly label this one as unit-only, and replace the wildcard
imports with explicit imports for the exact symbols used. Make sure the correct
tier-specific setup is shown for the resolver path, especially the symbols that
differ between unit and integration/proxy flows such as SandboxApp and
ProxySession.
🤖 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-kotlin/references/objects-mapping.md:
- Around line 530-545: The helper reference in the objects-mapping docs uses the
wrong filename casing, so update the guidance to point consistently to
Helpers.kt instead of helpers.kt. Fix the prose and the table entry that
references uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/Helpers.kt so
readers can locate the actual helper file and related symbols like
setupSyncedChannel, setupSyncedChannelNoAck, buildObjectSyncMessage, and
buildPublicObjectMessage without path mismatches.
- Around line 137-147: Add language tags to the fenced code examples in the
objects-mapping reference so markdownlint stops flagging them and syntax
highlighting is restored. Update the affected examples in the translation
guidance sections around the root.entries()/root.keys() snippets and the other
noted example blocks to use an appropriate tag such as kotlin, markdown, or
text, keeping the existing content unchanged.

In `@uts/build.gradle.kts`:
- Around line 44-54: The custom Test tasks runUtsUnitTests and
runUtsIntegrationTests only set filters, so they are not actually wired to the
UTS test source set. Update these tasks in uts/build.gradle.kts to inherit the
same testClassesDirs and classpath as the built-in test task, and keep the
existing includeTestsMatching filters so they execute the intended
io.ably.lib.uts.unit and io.ably.lib.uts.integration tests instead of passing
empty.

In `@uts/README.md`:
- Around line 42-59: The README contains unlabeled fenced code blocks that
trigger markdownlint and lose syntax highlighting. Update the fenced blocks in
the affected diagram/example sections to include an appropriate language
identifier such as text or kotlin, using the same README content around the
spec/test hierarchy and other referenced fenced blocks, so the markdown stays
lint-clean and renders correctly.

---

Nitpick comments:
In @.claude/skills/uts-to-kotlin/SKILL.md:
- Around line 353-374: This template is still unit-only even though it is
presented as the resolver-selected tier scaffold, and it also relies on star
imports. Update the Skill.md template around the test scaffold to either split
it into separate tier-specific templates or clearly label this one as unit-only,
and replace the wildcard imports with explicit imports for the exact symbols
used. Make sure the correct tier-specific setup is shown for the resolver path,
especially the symbols that differ between unit and integration/proxy flows such
as SandboxApp and ProxySession.
🪄 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: 27e70f9e-9e77-46d5-a860-34d544c796f2

📥 Commits

Reviewing files that changed from the base of the PR and between b56407f and 0c59d15.

📒 Files selected for processing (32)
  • .claude/skills/uts-to-kotlin/SKILL.md
  • .claude/skills/uts-to-kotlin/references/objects-mapping.md
  • .claude/skills/uts-to-kotlin/scripts/resolve_uts.py
  • .claude/skills/uts-to-kotlin/uts-package-mapping.json
  • .github/workflows/check.yml
  • .github/workflows/integration-test.yml
  • uts/README.md
  • uts/build.gradle.kts
  • uts/index.html
  • uts/src/test/kotlin/io/ably/lib/types/Utils.kt
  • uts/src/test/kotlin/io/ably/lib/uts/deviations.md
  • uts/src/test/kotlin/io/ably/lib/uts/infra/Utils.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/integration/SandboxApp.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/integration/proxy/ProxyManager.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/integration/proxy/ProxySession.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/ClientFactories.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/DefaultPendingConnection.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/DefaultPendingRequest.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/FakeClock.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/MockEvent.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/MockHttpClient.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/MockHttpEngine.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/MockWebSocket.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/MockWebSocketEngineFactory.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/PendingConnection.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/PendingRequest.kt
  • uts/src/test/kotlin/io/ably/lib/uts/infra/unit/Utils.kt
  • uts/src/test/kotlin/io/ably/lib/uts/integration/proxy/realtime/AuthReauthTest.kt
  • uts/src/test/kotlin/io/ably/lib/uts/integration/standard/realtime/ChannelHistoryTest.kt
  • uts/src/test/kotlin/io/ably/lib/uts/integration/standard/realtime/TokenRequestTest.kt
  • uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/Helpers.kt
  • uts/src/test/kotlin/io/ably/lib/uts/unit/realtime/ConnectionRecoveryTest.kt
💤 Files with no reviewable changes (1)
  • uts/src/test/kotlin/io/ably/lib/types/Utils.kt

Comment on lines +137 to +147
```
# spec
FOR [key, pathObj] IN root.entries(): …
ASSERT "name" IN root.keys()
keys = list(root.keys())
```
```kotlin
for ((key, pathObj) in root.entries()) { … } // Map.Entry destructures into (key, value)
assertTrue("name" in root.keys()) // Kotlin `in` -> Iterable.contains
val keys = root.keys().toList() // when the spec materialises a list / checks length
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add language tags to these fenced examples.

markdownlint is already flagging these code blocks. Adding kotlin, markdown, or text here will clear the lint noise and restore syntax highlighting in the main translation examples.

Also applies to: 191-198, 283-296, 316-323, 418-433, 563-592

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 137-137: 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-kotlin/references/objects-mapping.md around lines 137
- 147, Add language tags to the fenced code examples in the objects-mapping
reference so markdownlint stops flagging them and syntax highlighting is
restored. Update the affected examples in the translation guidance sections
around the root.entries()/root.keys() snippets and the other noted example
blocks to use an appropriate tag such as kotlin, markdown, or text, keeping the
existing content unchanged.

Source: Linters/SAST tools

Comment on lines +530 to +545
### Unit-test helpers — `standard_test_pool.md` → `helpers.kt`

Every objects unit spec opens with `setup_synced_channel` and constructs protocol/object messages with the
`build_*` helpers. These are implemented in
`uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/helpers.kt` — **call them; don't hand-roll the mock
setup or message JSON.**

| Spec helper | `helpers.kt` |
|---|---|
| `{ client, channel, root, mock_ws } = AWAIT setup_synced_channel("test")` | `val (client, channel, root, mockWs) = setupSyncedChannel("test")` (`suspend`, returns `SyncedChannel`) |
| `setup_synced_channel_no_ack(...)` | `setupSyncedChannelNoAck(...)` |
| `build_object_sync_message` / `build_object_message` / `build_ack_message` | `buildObjectSyncMessage` / `buildObjectMessage` / `buildAckMessage` → `ProtocolMessage` |
| `build_counter_inc` / `build_map_set` / `build_map_remove` / `build_map_clear` / `build_object_delete` / `build_counter_create` / `build_map_create` | same names camelCased → wire `JsonObject` |
| `build_object_state` / `build_object_message_with_state` | `buildObjectState` / `buildObjectMessageWithState` |
| `build_public_object_message(msg, channel)` | `buildPublicObjectMessage(wireJson, channel)` (reflective; §11) |
| `STANDARD_POOL_OBJECTS` | `STANDARD_POOL_OBJECTS` |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the helper filename casing.

This section points readers to .../unit/liveobjects/helpers.kt, but the helper added in this stack is uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/Helpers.kt. On Linux/macOS source navigation that path mismatch is enough to send people to a non-existent file, so the prose and table should use Helpers.kt consistently.

🤖 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-kotlin/references/objects-mapping.md around lines 530
- 545, The helper reference in the objects-mapping docs uses the wrong filename
casing, so update the guidance to point consistently to Helpers.kt instead of
helpers.kt. Fix the prose and the table entry that references
uts/src/test/kotlin/io/ably/lib/uts/unit/liveobjects/Helpers.kt so readers can
locate the actual helper file and related symbols like setupSyncedChannel,
setupSyncedChannelNoAck, buildObjectSyncMessage, and buildPublicObjectMessage
without path mismatches.

Comment thread uts/build.gradle.kts
Comment on lines +44 to +54
tasks.register<Test>("runUtsUnitTests") {
filter {
includeTestsMatching("io.ably.lib.uts.unit.*")
}
}

tasks.register<Test>("runUtsIntegrationTests") {
filter {
includeTestsMatching("io.ably.lib.uts.integration.*")
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

In Gradle Kotlin DSL, does a custom tasks.register("...")automatically inherittestClassesDirsandclasspathfrom the built-intesttask, or do those need to be set explicitly fromsourceSets["test"]?

💡 Result:

In Gradle, a custom task registered with tasks.register<Test>("...") does not automatically inherit testClassesDirs and classpath from the built-in test task [1][2]. By design, Gradle's JVM plugins automatically configure only the built-in test task to use the test source set [1][2]. When you create a custom Test task, you must explicitly configure both testClassesDirs and classpath using the appropriate sourceSets [1][2]. The recommended approach is to manually set these properties as shown below: val myCustomTest = tasks.register("myCustomTest") { testClassesDirs = sourceSets["test"].output.classesDirs classpath = sourceSets["test"].runtimeClasspath } If you are creating a custom test task for a different set of tests (e.g., integration tests), you should define a corresponding sourceSet and point the task properties to that specific source set instead: val integrationTest = tasks.register("integrationTest") { testClassesDirs = sourceSets["intTest"].output.classesDirs classpath = sourceSets["intTest"].runtimeClasspath } Relying on automatic inheritance is not supported, and attempting to do so will result in a task that does not correctly locate or execute your test classes [1][2].

Citations:


Wire the custom Test tasks to the test source set. tasks.register<Test>(...) here only adds the filter; it does not inherit testClassesDirs or classpath from the built-in test task, so these tasks can pass without running any UTS tests.

Suggested fix
+val testSourceSet = sourceSets["test"]
+
 tasks.register<Test>("runUtsUnitTests") {
+    testClassesDirs = testSourceSet.output.classesDirs
+    classpath = testSourceSet.runtimeClasspath
     filter {
         includeTestsMatching("io.ably.lib.uts.unit.*")
     }
 }
 
 tasks.register<Test>("runUtsIntegrationTests") {
+    testClassesDirs = testSourceSet.output.classesDirs
+    classpath = testSourceSet.runtimeClasspath
     filter {
         includeTestsMatching("io.ably.lib.uts.integration.*")
     }
 }
📝 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
tasks.register<Test>("runUtsUnitTests") {
filter {
includeTestsMatching("io.ably.lib.uts.unit.*")
}
}
tasks.register<Test>("runUtsIntegrationTests") {
filter {
includeTestsMatching("io.ably.lib.uts.integration.*")
}
}
val testSourceSet = sourceSets["test"]
tasks.register<Test>("runUtsUnitTests") {
testClassesDirs = testSourceSet.output.classesDirs
classpath = testSourceSet.runtimeClasspath
filter {
includeTestsMatching("io.ably.lib.uts.unit.*")
}
}
tasks.register<Test>("runUtsIntegrationTests") {
testClassesDirs = testSourceSet.output.classesDirs
classpath = testSourceSet.runtimeClasspath
filter {
includeTestsMatching("io.ably.lib.uts.integration.*")
}
}
🤖 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 `@uts/build.gradle.kts` around lines 44 - 54, The custom Test tasks
runUtsUnitTests and runUtsIntegrationTests only set filters, so they are not
actually wired to the UTS test source set. Update these tasks in
uts/build.gradle.kts to inherit the same testClassesDirs and classpath as the
built-in test task, and keep the existing includeTestsMatching filters so they
execute the intended io.ably.lib.uts.unit and io.ably.lib.uts.integration tests
instead of passing empty.

Comment thread uts/README.md
Comment on lines +42 to +59
```
┌──────────────────────────────┐
│ Ably features spec │ ← the ultimate authority (RSC*, RTN*, RTL* …)
│ (features.md) │
└──────────────┬───────────────┘
│ distilled into portable test specs
┌──────────────────────────────┐
│ UTS test specs (.md) │ ← language-neutral pseudocode, one file per feature
│ "writing-test-specs" │ e.g. realtime/unit/connection/connection_recovery_test.md
└──────────────┬───────────────┘
│ translated ("derived") per SDK
┌──────────────────────────────┐
│ Derived tests │ ← concrete, runnable tests in the SDK's language
│ (this repo: Kotlin in uts/) │ e.g. ConnectionRecoveryTest.kt
└──────────────────────────────┘
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add language identifiers to the fenced blocks.

These unlabeled fences are already tripping markdownlint. Please mark them as text, kotlin, or another appropriate language so the README stays lint-clean and renders with syntax highlighting.

Also applies to: 224-243, 850-878, 888-909

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 42-42: 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 `@uts/README.md` around lines 42 - 59, The README contains unlabeled fenced
code blocks that trigger markdownlint and lose syntax highlighting. Update the
fenced blocks in the affected diagram/example sections to include an appropriate
language identifier such as text or kotlin, using the same README content around
the spec/test hierarchy and other referenced fenced blocks, so the markdown
stays lint-clean and renders correctly.

Source: Linters/SAST tools

@ttypic ttypic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants