Serialize the LiveObjects integration suites - #2288
Merged
Merged
Conversation
Each case in these suites stands up its own realtime client, channel and object sync against the sandbox. swift-testing runs a suite's cases in parallel unless told otherwise, so NumberPrecisionTests alone opened twelve connections at once — four test functions over three argument values. On CI that collapses. In the run linked below, all twelve cases started inside four seconds, the cheapest of them then took 128 seconds, and the counter tests — which make twice the round trips — crossed the 30 second provisioning timeout and failed with `Error -1001 - The request timed out`. The same job has failed this way on five unrelated branches, always on a simulator platform, never on macOS. The UTS integration tier already settled this: Test/UTS/README.md states the convention as `@Suite(.serialized) final class … : UTSTestCase`. These three suites never adopted it. Note that serializing is slower where there is no contention: locally NumberPrecisionTests goes from 3.6s to 11.9s. The trade is worth it on CI, where the parallel version costs minutes and then fails.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. WalkthroughThree integration test suites now use serialized execution. No test logic changed. ChangesIntegration test execution
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The integration suites now run serially as intended, with no remaining merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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. A rabbit hops through tests in line Comment |
This branch was successfully deployed
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.
Each case in
LiveObjects/Tests/AblyLiveObjectsTests/Integrationstands up its own realtime client, its own channel and its own object sync against the sandbox:swift-testing runs a suite's cases in parallel unless told otherwise, and none of these three suites said otherwise.
NumberPrecisionTestsalone is four test functions over three argument values, so twelve concurrent clients.What that does on CI
From the
Xcode, iOS (Xcode 16.4)job on run 35081997799:A test that performs one
setand one poll took over two minutes. The counter tests make twice the round trips — create, poll, increment, poll — so they were the ones to crossSandboxEnvironment.provisioningTimeout(30s) and fail withNSURLErrorTimedOut.It is not specific to any branch
Per-attempt data for the same job, which run-level conclusions hide because the failures were cleared by re-running:
split/agent-identifiersci/retry-absorbs-flakessplit/uts-per-sidedeps/raise-platform-floorfix/test-crash-on-timeoutThose five branches have nothing in common,
NumberPrecisionTestscame frommaininfbb638b3and none of them touch it, and tvOS is affected too — so this is contention, not a platform quirk. macOS has never failed this way, being the host rather than a simulator.LiveObjects/BuildToolpasses no-retry-tests-on-failure, so one timeout in one of twelve concurrent cases reddens the whole job and somebody has to press the button. Adding that flag is a separate, complementary change.The convention already exists
Test/UTS/README.mdstates it for the UTS integration tier — "Suites are@Suite(.serialized) final class … : UTSTestCase" — andIntegrationTestCaserepeats it. These three suites never adopted it.The trade, measured
Serializing is slower where there is no contention, and I checked rather than assumed: locally
NumberPrecisionTestsgoes from 3.6s to 11.9s. That is the cost. The benefit is on CI, where the parallel version currently spends minutes and then fails — twelve sequential cases at a few seconds each should beat twelve concurrent ones at 128 seconds each.I cannot demonstrate the CI improvement from a local run, because the contention comes from the runner's latency to the sandbox. CI on this PR is the measurement.
Verification
All three suites pass serialized:
NumberPrecisionTests(4 tests, 11.4s),AblyLiveObjectsTests(418 tests in 109 suites, 11.7s),ObjectLifetimesTests(3 tests, 3.3s).Summary by CodeRabbit