Skip to content

Address the review feedback on the UTS split - #2290

Merged
maratal merged 3 commits into
integration/v2from
docs/uts-split-review-feedback
Sep 18, 2026
Merged

maratal merged 3 commits into
integration/v2from
docs/uts-split-review-feedback

Conversation

@maratal

@maratal maratal commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #2275, and addresses the review feedback on it.

Three commits, one per finding.

ProxySession's usage example bypassed the seam (r4026592486). The doc comment at Test/UTS/infra/integration/proxy/ProxySession.swift:149 showed ARTRealtime(options: options), so a reader copying it got a client that runs core-side whatever UTS_SIDE says. It now builds through makeRealtimeForSide(options:), like every other realtime client in the suite.

Trailing whitespace in uts.yaml (r4026524234). Two blank lines in the job comment carried trailing spaces. .editorconfig scopes trimming to *.{swift,m,h}, so nothing in CI catches it.

Test/UTS/README.md's CI section described a state that no longer held (r3987939748). It said there was no UTS-specific CI job and that the suite ran as part of the full test suite, while naming uts.yaml in the same sentence. The section now describes the workflow: the lanes pass suite:uts, which fastlane turns into -only-testing:UTS; six legs run iOS, tvOS and macOS twice each, once per UTS_SIDE value; and integration-test.yaml passes suite:sdk, which skips this target.

Not addressed here, because neither needs a code change:

  • The absence of an automated cross-leg comparison of pass counts (r4026524222) is informational, and the suggested action is to eyeball both legs on the first green run.
  • The spec-check outcome (r4026592244) wants an answer on the thread, not an edit. Test/UTS/deviations.md is still untouched by any split work.

Summary by CodeRabbit

  • Documentation
    • Updated contributor guidance to clarify that UTS runs twice per platform, covering both supported UTS_SIDE values.
    • Documented the dedicated UTS CI workflow, including its triggers, platform coverage, test tiers, and execution behavior.
  • Chores
    • Cleaned up whitespace in the static analyzer report upload workflow without changing its behavior.

The "Where CI runs them" section said there was no UTS-specific CI job and that
the suite ran as part of the full test suite, while naming uts.yaml in the same
sentence. It now describes the workflow: six legs running the UTS target, with
integration-test.yaml skipping it.
@maratal
maratal requested a review from ttypic September 17, 2026 23:25
@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 0112ce9a-c8ab-4790-ab60-c34a2bee1333

📥 Commits

Reviewing files that changed from the base of the PR and between 5d3de1e and 4eaf493.

📒 Files selected for processing (3)
  • .github/workflows/uts.yaml
  • CONTRIBUTING.md
  • Test/UTS/README.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The PR removes trailing whitespace from the UTS workflow and updates contributor and UTS README documentation to describe the dedicated UTS CI workflow and its coverage.

Changes

UTS CI alignment

Layer / File(s) Summary
UTS workflow and coverage documentation
.github/workflows/uts.yaml, CONTRIBUTING.md, Test/UTS/README.md
The workflow receives whitespace-only cleanup. Documentation now states that UTS runs twice per platform and describes the dedicated workflow, triggers, lanes, platforms, tiers, and SDK suite separation.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Suggested reviewers: sacoo7

Merge Risk: ⚪ Minimal · up to 4eaf4

The documented workflow and remaining TODO describe different CI capabilities; no actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is related to the changes. It identifies review feedback and the UTS split, which are the main themes of the documentation and workflow updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

A rabbit tidies trailing space,
Then charts six UTS legs in place,
Two sides hop through every tier,
The workflow path is bright and clear,
Docs and CI now race as one.

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

Two blank lines in the job comment carried trailing spaces. EditorConfig scopes
trimming to Swift and Objective-C files, so nothing in CI catches it.
The workflow section said the two workflows cover the test plan exactly once,
which holds for target ownership but not for execution: the UTS target runs
twice per platform, once per UTS_SIDE value.
An error occurred while trying to automatically change base from split/uts-per-side to ci/retry-absorbs-flakes September 18, 2026 19:26
@maratal
maratal changed the base branch from split/uts-per-side to integration/v2 September 18, 2026 19:50
@maratal
maratal merged commit 0f088f8 into integration/v2 Sep 18, 2026
34 checks passed
@maratal
maratal deleted the docs/uts-split-review-feedback branch September 18, 2026 19:52

This branch was successfully deployed

4 active deployments
staging/pull/2290/markdown-api-reference — 4eaf4937 Deployed Sep 18, 2026 by github-actions[bot]
staging/pull/2290/jazzydoc — 4eaf4937 Deployed Sep 18, 2026 by github-actions[bot]
staging/pull/2290/AblyLiveObjects — 4eaf4937 Deployed Sep 18, 2026 by github-actions[bot]
staging/pull/2290/features — 4eaf4937 Deployed Sep 18, 2026 by github-actions[bot]
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