Repository navigation
test(tokio): stream_stopped awaits its stop event, not a 100 ms deadline - #8237
Conversation
Quest-Claim: 31c2250f1d2ed6a1
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
changed the title
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 13 seconds. View limit details
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. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed b45c6a5.
No actionable findings. Both stream-stopped regressions now await the stop/close event rather than asserting it arrives within 100 ms. The first preserves the three observers and stop-code assertions, including the observer polled after dropping the stream; the second preserves wake registration before finishing and dropping the sender. Removing the quarantine and inventory exemption restores the intended coverage.
Direction: retaining event-based assertions is preferable to relaxing the wall-clock budget. The workspace nextest timeout still bounds an actual hang.
Verification limits: GitHub-only static review of the four-file diff, complete affected tests and nextest settings. No tests, stress runs or builds performed. Rechecked open/non-draft state, full head and published reviews immediately before this COMMENT.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
stream_stoppedawaits its stop event: the 100 mstimeoutaround the client future and theclient.expect("timeout")that consumed it are gone, and the test's#[ignore]is removed, so the stop codes it already asserted are now the only signal and nextest'sterminate-afteris the sole hang bound.stream_stopped_2gets the same fix:send_stream.closed()is no longer wrapped in a 100 ms deadline, and the assertion drops the timeout's extraOklayer (Ok(Ok(None))→Ok(None)), which matchesclosed()'s real output type.- Quarantine bookkeeping removed: the
sh/gh/test-inventory.allowentry andquest/m0/test-flakes-2/quic-stream-stopped.mdare deleted along with the epic README's## Requiredline, and no dangling references to the quest or the test's ignore remain.
The Duration and timeout imports are still used by many other tests in the file, _certs is on through the default net feature so the module compiles in the normal suite, and closed() returns an owning 'static future, so dropping the wrapper leaves the poll-once-then-select structure in stream_stopped_2 unchanged.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) ｜ 𝕏
Drops the 100 ms wall-clock deadlines from moq-tokio's
stream_stoppedandstream_stopped_2, so each awaits the stop/close event it asserts; nextest'sterminate-afterstill bounds a real hang. Un-ignoresstream_stopped, removes itssh/gh/test-inventory.allowentry, and deletes the quest.Neither test asserted the deadline itself, so
stream_stopped_2got the same fix.Verified:
cargo nextest run -p moq-tokio stream_stopped --stress-count 200with 16 busy-loop CPU hogs running: 200/200 passed.Overlap: moq-dev/moq#8208 only appends a new test to the same file; no conflict expected.
Public API: none. Wire: none.
Models
Size: XS
Implementer: Claude Code, claude-opus-5-5, low effort
🤖 Generated with Claude Code
(Written by Opus 5.5 with low)
Review
Rewritten by
quest reviewafter each round.01a1b8bf-40f0-78a0-a279-ab56055ecefd, round 1The changes remove load-sensitive deadlines while preserving stop-code and completion assertions and consistently removing quarantine records; source review found no actionable defects, with tests not run as instructed.