fix(ci): resolve two pre-existing test-infra failures (event-bus flake + gate-smoke allowlist)#5063
Conversation
…lake Under cfg(test) the crate's ~12.6k unit tests share two process-global bounded resources sized for production: the event bus's 256-slot broadcast channel (src/core/event_bus/bus.rs) and the learning candidate ring's 1024 entries (src/openhuman/learning/candidate.rs, whose drainer does not run in the unit suite). Under the concurrent full-suite publish load a slow subscriber lags and drops events (RecvError::Lagged), and the candidate ring evicts a test's own candidates — intermittently failing bus-delivery tests such as learning::startup::tests::learning_subscriber_fires_with_no_channel_configured. Raise both floors to 65536 for the TEST BINARY ONLY (cfg(test)); production keeps 256 / 1024 and ships byte-identical. No new dependency, no serialization, no weakened assertion — the shared globals simply get enough headroom that neither drops nor evicts under the artificial suite-wide load.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 4 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: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTest builds now allocate larger capacities for the global event bus and learning buffer to reduce drops during concurrent tests. The CI feature-gate smoke-test allowlist also includes an additional gated test module. ChangesTest capacity configuration
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
The `Rust Feature-Gate Smoke` lane guards that every feature-gated test module is acknowledged in an EXPECTED allowlist. `composio/action_tool.rs` gained a `#[cfg(feature = "flows")]` test in tinyhumansai#4853 but was never added to the list, so the guard's ACTUAL-vs-EXPECTED diff fails on it — on every PR since. This is the guard's own sanctioned maintenance (its error message says "Update the EXPECTED allowlist"), not a weakening: the guard still validates every other file. Replicated the guard's grep+diff locally to confirm this is the only drift and the diff is now empty.
YellowSnnowmann
changed the title
coderabbitai
Bot
added
bug
rust-core
…city floor The earlier test-only capacity floor (5c49a96) did not fix the flake: the two failing tests share the process-global broadcast bus, so under the parallel suite they receive events published by *other* tests — a capacity bump cannot fix cross-test interference. Replace it with real isolation. - learning::startup: rewrite `learning_subscriber_fires_*` to publish a DocumentCanonicalized event on a private `EventBus::create(64)` with a locally-subscribed `EmailSignatureSubscriber`, instead of asserting on the shared global bus. Add a synchronous `register_email_signature_subscriber` guard test for the channel-independent registration path (tinyhumansai#5003). - web_chat::event_bus: the automation-halt test reads from the shared web-channel bus; drain foreign events and match on `event == "automation_halt"` rather than taking whatever is first in the buffer. - Revert the `#[cfg(test)]` capacity floors in event_bus/bus.rs and learning/candidate.rs back to origin/main — no longer needed.
1 participant
Summary
learning::startup::tests::learning_subscriber_fires_*web_chat::event_bus::tests::automation_halt_subscriber_handle_publishes_correct_payloadcargo test --lib(~12,600 tests in one process, run concurrently) they observedDomainEvents published by other tests — cross-test interference, not a capacity shortfall.#[cfg(test)]-only; the shipped build is byte-identical.Problem
The event bus is a single
OnceLock<EventBus>shared by every unit test (they all run in one process). Two tests read from that shared global:learning_subscriber_fires_*subscribed to the global bus and published aDocumentCanonicalized, then asserted the email-signature subscriber consumed it — but any of the other ~12.6k tests publishing to the same global bus could be delivered first.automation_halt_subscriber_handle_publishes_correct_payloadcalledhandle(...)directly, thentry_recv()'d the shared web-channel bus — under load it picked up a foreign test's event first and failed with an "event name mismatch".Both pass in isolation and on the changed-files fast lane, and fail only under the full parallel suite — which any
Cargo.toml-touching PR triggers (therust-core-fullcoverage lane).Solution
learning::startup— rewrite the subscriber test to publishDocumentCanonicalizedon a privateEventBus::create(64)with a locally-subscribedEmailSignatureSubscriber, asserting delivery on that owned bus instead of the shared global. Add a synchronousemail_signature_subscriber_registers_on_channel_independent_pathguard covering the channel-independent registration path (fix(learning): email-signature subscriber + rebuild trigger silently depend on channels being configured #5003).web_chat::event_bus— drain foreign events from the shared web-channel bus and match onevent == "automation_halt"rather than taking whatever is first in the buffer.#[cfg(test)]capacity floors inevent_bus/bus.rsandlearning/candidate.rsback toorigin/main.No new dependency, no
serial_test, no weakened assertion, no production change.Verification
cargo test --lib learning::startup web_chat::event_bus→ 11 passed).Second fix: gate-smoke allowlist drift
Acknowledges
openhuman/composio/action_tool.rsin theRust Feature-Gate Smokelane'sEXPECTED allowlist. That file gained a
#[cfg(feature = "flows")]test in #4853 but wasnever allowlisted, so the guard's ACTUAL-vs-EXPECTED diff has failed on every PR since.
This is the guard's own sanctioned maintenance (its error says "Update the EXPECTED
allowlist"), not a weakening — it still validates every other file. Replicated the guard's
grep+diff locally to confirm this is the only drift and the diff is now empty.
Submission Checklist
#[cfg(test)]test bodies, executed by the run itself.N/A: test-infrastructure fix, no user-facing feature.N/A.N/A:#[cfg(test)]-only; the shipped build is byte-identical.Closes #NNN—N/A: no tracking issue; see## Related.Impact
#[cfg(test)]-only.Related
rust-core-fullvia aCargo.tomlchange). The same fix is cherry-picked into both and self-drops on rebase once this merges tomain.src/openhuman/learning/startup.rs(fix(learning): email-signature subscriber + rebuild trigger silently depend on channels being configured #5003 introduced the subscriber; this isolates its test harness).