Skip to content

CI: make the configured test retries actually absorb a flake - #2274

Merged
maratal merged 2 commits into
fix/uts-issue-recordingfrom
ci/retry-absorbs-flakes
Sep 18, 2026
Merged

maratal merged 2 commits into
fix/uts-issue-recordingfrom
ci/retry-absorbs-flakes

Conversation

@maratal

@maratal maratal commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Split out of #2268. Stacked on #2273 (base: fix/uts-issue-recording), which is what makes flakes retryable in the first place.

Preparation for #2275

Why

The Fastlane lanes have set number_of_retries: 2 for a long time, and the comment beside it claims the retry "absorbs single-occurrence environmental timeouts on the CI runners". That was not true. Two separate reasons, one per commit.

1. scan judged the lane on every attempt. output_remove_retry_attempts defaults to false, so a test that failed once and passed on retry still failed CI. Observed on a UTS leg: three suites failed, retried, and passed —

::notice ::Test run with 752 tests passed after 578.122 seconds
Test Succeeded

| Number of tests    | 415 (and 830 retries) |
| Number of failures | 3                     |
[!] Tests have failed

xcodebuild said Test Succeeded; scan failed the lane on the superseded attempts. So the retry was doing nothing it was configured for.

2. fastlane 2.227.2 does not strip a failure when the retry skipped the test rather than passing it, so one phantom failure survived even with the option on (3 → 1). That is fastlane #30081, fixed in 2.237.0.

What

  • output_remove_retry_attempts: true in the shared lane arguments, with the comment corrected.
  • Gemfile.lock moved to fastlane 2.237.0 — the earliest release carrying the fix. Deliberately not 2.239.0, which also moves faraday to 2.x; 2.237.0 leaves it on 1.10.6. Gemfile is unchanged, so the dependency stays unpinned.

This does not weaken the signal: a genuine regression fails every attempt and still fails the lane. The option only discards attempts a later retry superseded.

It applies to every lane, so Integration Test benefits as much as the UTS legs — which is the point, since the flaw was never UTS-specific.

Verification

The argument mapping was checked by evaluating LaneConfig directly (number_of_retries=2 output_remove_retry_attempts=true), plus ruby -c on the Fastfile. bundle lock re-resolves to the same lockfile against the unpinned Gemfile, so the pin is not accidental.

One gap worth flagging: bundle install fails on my machine for unrelated reasons — bigdecimal, json and nkf cannot build native extensions into a .sitearchdir — so I have no local proof the bundle installs. CI's Install Dependencies step on a clean runner is the first real test of the lockfile, and it fails early and unmistakably if the resolution is bad.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Test runs now report results based on the final outcome after retries, reducing noise from superseded attempts.

@coderabbitai

coderabbitai Bot commented Sep 9, 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: 79f930e7-2fb4-4968-8a2c-d0526e18314a

📥 Commits

Reviewing files that changed from the base of the PR and between 0784d95 and 62ef761.

⛔ Files ignored due to path filters (1)
  • Gemfile.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • fastlane/Fastfile

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


Walkthrough

The Fastlane test configuration now removes superseded retry attempts from run_tests results. The lane evaluates each test by its final state.

Changes

Test retry result handling

Layer / File(s) Summary
Configure final test results
fastlane/Fastfile
Adds output_remove_retry_attempts: true to the run_tests arguments.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 62ef7

The change is narrowly scoped to ensuring retried tests are evaluated by their final result, with no unresolved merge-blocking risk identified.

🚥 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 clearly describes the main change: enabling configured CI test retries to absorb transient test flakes.
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 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ci/retry-absorbs-flakes

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 checks the test array,
Old retries hop away.
The final state now leads the way,
With cleaner results today.
Carrots cheer the passing suite,
And every test is neat.

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

@github-actions
github-actions Bot temporarily deployed to staging/pull/2274/features September 9, 2026 12:27 Inactive
@github-actions
github-actions Bot temporarily deployed to staging/pull/2274/AblyLiveObjects September 9, 2026 12:28 Inactive
@github-actions
github-actions Bot temporarily deployed to staging/pull/2274/jazzydoc September 9, 2026 12:31 Inactive
@github-actions
github-actions Bot temporarily deployed to staging/pull/2274/markdown-api-reference September 9, 2026 12:31 Inactive
@maratal
maratal added this pull request to stack #2276 September 9, 2026 12:51
@maratal
maratal requested a review from sacOO7 September 9, 2026 12:53
@maratal
maratal marked this pull request as ready for review September 9, 2026 12:53
maratal and others added 2 commits September 10, 2026 18:29
number_of_retries made xcodebuild re-run failed tests, but scan judged the
lane on every attempt, so a test that failed once and passed on retry
still failed CI. The retry was therefore doing nothing it was configured
for, while the comment beside it claimed otherwise.

Seen on the UTS iOS leg: three suites failed, retried, and passed;
xcodebuild reported

  Test run with 752 tests passed after 578.122 seconds
  Test Succeeded

and scan then reported "Number of failures | 3" and failed the lane.

output_remove_retry_attempts makes scan judge each test on its final
state. A genuine regression still fails, because it fails every attempt.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
output_remove_retry_attempts does not strip a failure when the retry
skipped the test rather than passing it, so a flake still failed CI even
with retries configured: xcodebuild reported "Test Succeeded" while scan
reported one failure. 2.237.0 carries the fix for that (#30081).

2.237.0 rather than the current 2.239.0: it is the earliest release with
the fix, and it leaves faraday on 1.10.6, where 2.239.0 moves it to 2.x.

Gemfile is unchanged; the dependency stays unpinned and the lockfile
records the resolved version.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@maratal

maratal commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@maratal
maratal merged commit 356a206 into integration/v2 Sep 18, 2026
57 of 65 checks passed
@maratal
maratal deleted the ci/retry-absorbs-flakes branch September 18, 2026 19:26

This branch was successfully deployed

4 active deployments
staging/pull/2274/AblyLiveObjects 62ef761f Deployed Sep 11, 2026 by github-actions[bot]
staging/pull/2274/markdown-api-reference 62ef761f Deployed Sep 10, 2026 by github-actions[bot]
staging/pull/2274/jazzydoc 62ef761f Deployed Sep 10, 2026 by github-actions[bot]
staging/pull/2274/features 62ef761f Deployed Sep 10, 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