Repository navigation
test(module): wait for the setup future to be reattached before asserting it is retained - #111
Conversation
…ting it is retained A setup poll takes the initial future out of its slot, polls it with no lock held and puts it back. The rejected-spawn retention test sampled the slot directly, so a slow runner could catch it mid-poll and see it empty. Wait, bounded, for the future to be back in its slot, as the physical-completion test already does for its anchor.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Comment |
Tiny Sweeper reviewReviewed head: Verdict: Ready for maintainer review · Priority: none Diagnostics and supporting detailsFull summaryPull request purpose (title): test(module): wait for the setup future to be reattached before asserting it is retained. This test-only change addresses a flaky failure in `terminal_runtime_tests::actual_linked_rejected_physical_spawn_retains_received_resources_without_retry`, which panicked asserting that the setup future was still in its slot. Because `TerminalSetupPoll::poll` removes the future from the slot, polls it without holding the lock, and then reattaches it, the test could sample an empty slot mid-poll. The fix waits (bounded by a 5s deadline) for the future to be reattached before locking resources and asserting retention; the pre-existing retention assertions remain unchanged. The latest push delta is unavailable. Review snapshot
Completeness: Complete What changedIn `crates/tinybus-module/src/lib_terminal_tests.rs`, a bounded `tokio::time::timeout` wait loop (5s) now spins with `yield_now` until the cloned `initial_future` slot is no longer `None`, replacing a direct read that could race against an in-flight poll. After the wait, the existing assertions that `retained_factory`, `runtime`, the initial payload, and the initial future are retained remain in place. No production code, API, or behaviour changes. Features
Tests
FindingsNo active actionable findings. Before mergeNone. Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
Summary
Main CI went red on
module-ci / coverage(run 39949099619, at 8dc8a33):terminal_runtime_tests::actual_linked_rejected_physical_spawn_retains_received_resources_without_retrypanicked atlib_terminal_tests.rs:803, which asserts thatinitial_futureis still in its slot. The same test failed the same way on 2026-10-22 (run 39785116193,feature-tests (ubuntu-latest)), so this is not specific to one change.Cause:
TerminalSetupPoll::polltakes the setup future out of its slot, polls it with no lock held, and puts it back, so no mutex is ever held across user code. The test read the slot directly, so a slow runner could sample it in the middle of a poll and find it empty. The future was still owned, by the poll that was running.Fix (test only): wait, with a 5 s deadline, until the future is back in its slot before taking the resources lock and asserting retention. The physical-completion test above already waits for its anchor the same way (
lib_terminal_tests.rs:667-678). If the future really were dropped, the wait times out and the test still fails.Related issue
None.
API or behavior changes
None. Test only.
Validation
Red/green proof on our remote check host, using two probe branches on the fork that will not be merged. Each adds a 300 ms delay between the poll and putting the future back, so the race happens on every run:
probe/terminal-race-before: main + probe, expected to FAIL at line 803;probe/terminal-race-after: this fix + probe, expected to PASS.The results will be posted here.
Full module CI on this head (GitHub, and our remote check host).
Tests
actual_linked_rejected_physical_spawn_retains_received_resources_without_retrynow waits for the poll to put the future back instead of reading its slot directly. It still asserts that the factory, the runtime, the initial payload and the initial future are all retained.Documentation
Not needed.
Checklist
#[allow(...)],#[ignore], or relaxed lintssleepused as synchronisation: a deadline around an awaited condition.envcontents in the diff or the description