fix(test-runner): make junit report delivery resilient on starved CI VMs - #419
Conversation
The Jasmine suite reports results through a single fire-and-forget POST to the Embassy server in the XCUITest host. On overloaded CI VMs that POST can be lost (the server's accept loop runs at .background QoS and gets starved), so the host waits out its full 600s budget and the job fails with the generic 'unfulfilled Jasmine tests expectation' error even though every spec passed. Same-VM step retries then fail the same way (run 30567093340). - Retry the report POST up to 10 times with a 30s per-request timeout, logging each failure to the app console so future flakes are diagnosable from the xcresult diagnostics. - Run the Embassy event loop at .userInitiated QoS instead of .background. - When the retry budget is exhausted, POST a best-effort sentinel to <REPORT_BASEURL>/delivery_failed so the host fails immediately with a delivery diagnostic instead of a 600s timeout.
|
Warning Review limit reached
Next review available in: 13 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughJUnit delivery now uses a launch-relative deadline, bounded retries, and a diagnostic failure callback. The XCTest harness captures delivery reasons, while CI preserves failed test results and uploads simulator and host diagnostics. ChangesTest delivery and diagnostics
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant TestServer
participant testRuntime
participant CI
TestRunner->>TestServer: POST JUnit XML
TestServer-->>TestRunner: Delivery failure
TestRunner->>TestRunner: Check deadline and retry budget
TestRunner->>TestServer: POST /delivery_failed diagnostic
TestServer->>testRuntime: Store delivery failure reason
testRuntime-->>testRuntime: Fail completed run with reason
CI->>CI: Preserve failed xcresult
CI->>CI: Collect and upload diagnostics
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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: 1
🤖 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 `@TestRunner/app/tests/index.js`:
- Around line 29-66: Update the retry flow in attemptReport and its
maxAttempts/retryDelayMs configuration to enforce a shared delivery deadline,
ensuring all report attempts, delays, and the final delivery_failed sentinel
complete within the host’s total wait budget. Preserve the existing retry
diagnostics while stopping retries early enough to reserve time for the
sentinel.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 35a00143-4272-40d3-b1a2-9ab606d49200
📒 Files selected for processing (2)
TestRunner/app/tests/index.jsTestRunnerTests/TestRunnerTests.swift
The host's 600s wait covers the suite run plus report delivery, so an unbounded retry ladder (up to ~345s) could still push the delivery_failed sentinel past the host timeout after a slow suite. The host now passes REPORT_DEADLINE_SECONDS (wait budget minus a 60s margin, counted from app launch) and the app stops retrying while there is still room for one final sentinel request.
The in-step retry used to rm -rf the failed attempt's result bundle, so the evidence of the very failure being retried never reached the uploaded artifact. Rename it aside instead and include it in the test-results upload. On failure, a new collect-test-diagnostics composite action gathers what the xcresult does not carry: host-side crash reports (.ips), CoreSimulator logs, and a process-filtered unified-log slice from a still-booted simulator. It deliberately never boots a device post-mortem and never collects a full logarchive - the xcresult's exported diagnostics already contain the app's stdout/stderr and testmanagerd logs.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
TestRunnerTests/TestRunnerTests.swift (1)
44-54: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake expectation fulfillment idempotent across retries.
The app can POST once for the initial report, retry up to 9 times, then POST
/delivery_failed; each accepted POST currently callsruntimeUnitTestsExpectation.fulfill(). If a posted request is accepted but its response is lost, a later retry or sentinel can callfulfill()again, causing an XCTest API-violation and replacing the captured failure reason. Gate fulfillment with a one-shot flag while still retaining the first report or failure payload.🤖 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 `@TestRunnerTests/TestRunnerTests.swift` around lines 44 - 54, Update the request-handling flow around runtimeUnitTestsExpectation.fulfill() to guard fulfillment with a one-shot flag, ensuring retries and the /delivery_failed sentinel cannot fulfill the expectation more than once. Preserve the existing behavior of retaining the first accepted report or failure payload, including reportDeliveryFailureReason, while subsequent accepted POSTs only return their response.
🤖 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 @.github/actions/collect-test-diagnostics/action.yml:
- Around line 29-31: Update the unified-log collection command in the
diagnostics step to run through a bounded timeout, while preserving any output
produced before the timeout and the existing non-failing behavior. Keep the
current simulator predicate, output file, and stderr handling intact.
In `@TestRunner/app/tests/index.js`:
- Around line 4-5: Recheck the REPORT_DEADLINE_SECONDS budget when each attempt
actually begins, including the setTimeout callback and the initial
attemptReport(1) entry. In attemptReport, if Date.now() has reached deadlineMs,
send the existing sentinel directly and do not start another POST; preserve the
retry flow only when remaining time is available.
---
Outside diff comments:
In `@TestRunnerTests/TestRunnerTests.swift`:
- Around line 44-54: Update the request-handling flow around
runtimeUnitTestsExpectation.fulfill() to guard fulfillment with a one-shot flag,
ensuring retries and the /delivery_failed sentinel cannot fulfill the
expectation more than once. Preserve the existing behavior of retaining the
first accepted report or failure payload, including reportDeliveryFailureReason,
while subsequent accepted POSTs only return their response.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: af91cdf2-62ed-49ba-a843-b44fe4a1660e
📒 Files selected for processing (5)
.github/actions/collect-test-diagnostics/action.yml.github/workflows/npm_release.yml.github/workflows/pull_request.ymlTestRunner/app/tests/index.jsTestRunnerTests/TestRunnerTests.swift
…ostics The first real-world run of the diagnostics path exposed three holes: - The collect step runs under bash -e -o pipefail, so the UDID lookup aborted the whole script when no device was booted (grep exit 1 propagates through the command substitution into the assignment). - xcodebuild leaves an extensionless staging path at -resultBundlePath when it dies early (e.g. 'Unable to find a device'), and the retry then fails with 'Existing file at -resultBundlePath'. The retry cleanup now removes everything at the result path except the preserved test_results_attempt1.xcresult. - simctl blocks indefinitely when CoreSimulatorService is wedged - the very state the diagnostics step tends to run in - so log show now runs under a 120s watchdog. Also re-check the report retry deadline when the timer fires rather than when it is scheduled: under load timers run late, and a retry started past the window would consume the delivery_failed sentinel's slot.
2 participants
Problem
The intermittent CI failure
Asynchronous wait failed: exceeded 600 seconds with unfulfilled "Jasmine tests" expectationis not a crash and not a runtime hang mid-suite. Diagnostics from failed runs (e.g. 30567093340) show the Jasmine suite completing normally (~50s of spec activity, crash watchdog never fires), after which the single junit report POST to the Embassy server in the XCUITest host never completes and the app goes silent. Two contributing factors, both addressed here:DispatchQueue.global(qos: .background)thread; Darwin throttles background-QoS threads aggressively (CPU and I/O), which on a loaded 3-core CI VM can starve the one thread the test outcome depends on.There is also a rarer variant (reproduced locally while validating this PR) where the app's main thread wedges right at suite completion — the POST gets initiated but main-queue callbacks never run again. Retries cannot help that variant; the diagnostics upload below is what makes it debuggable, and it is being investigated separately.
Changes
TestRunner/app/tests/index.js): up to 10 attempts, 5s apart, 30s per-request timeout. Each failure is logged to the app console so future flakes are diagnosable straight from the xcresult diagnostics.REPORT_DEADLINE_SECONDS(wait budget − 60s, counted from app launch); the app stops retrying while there is still room for one final request, so the sentinel always lands before the host times out.<REPORT_BASEURL>/delivery_failed; the host then fails immediately withcould not deliver the junit report: <reason>instead of a generic 600s timeout..userInitiatedQoS (TestRunnerTests/TestRunnerTests.swift).pull_request.yml,npm_release.yml): the retry used torm -rfthe failed attempt's bundle, destroying the evidence of the very failure being retried. It is now renamed totest_results_attempt1.xcresultand included in thetest-resultsartifact.collect-test-diagnosticscomposite action, used by both workflows onfailure()): host-side crash reports (.ips), CoreSimulator logs, and a process-filtered unified-log slice from a still-booted simulator. It deliberately does not boot a device post-mortem or collect a full logarchive — the xcresult's exported diagnostics already carry the app's stdout/stderr and testmanagerd logs.Duplicate/late POSTs are harmless: over-fulfilling the expectation is already relied upon by the crash watchdog, and an extra junit attachment is benign.
Testing
Full local simulator suite (
run_tests.sh) passes with these changes (results delivered through the new retry path end-to-end). Workflow/action YAML validated. One local run reproduced the pre-existing end-of-suite wedge described above — with these changes it produces the same outcome as before (600s timeout), but the new artifacts capture the evidence needed to chase it.Summary by CodeRabbit
.xcresultas anattempt1artifact instead of discarding it.