test(moq-gst): decide the cancelled pump race instead of sleeping on it

[kixelated]

Problem

moq-gst source::imp::session_tests::a_subscription_resolving_after_cancellation_creates_no_pad fails in CI. It blocked #3867, which is the first PR in a while whose selection covers the whole workspace, so nothing else was running moq-gst's tests to notice.

The test cleared the catalog and answered the held subscription immediately after, assuming the session had reconciled the removal in between. Nothing made that true: follow_catalog reconciles on its own runtime, and when the answer lands first the pump wins its select!, goes live and takes the pad the test forbids. That is the documented behavior for a rendition that goes live before its removal is seen (#3864), so the test was calling a correct outcome a failure. A 500 ms sleep after the answer only decided how often.

It failed 2/2 on GitHub runners and passed 12/12 locally: isolated, pinned to one core, under 16 busy loops, and as the full moq-gst suite on two cores.

Approach

The interleaving is not the test's to choose, so it stops trying.

• The session test waits for the cancelled pump to drop its subscription, an edge it can observe through Request::poll_unused, then answers and asserts with no sleep at all. It runs in 0.020 s instead of 0.523 s, and is renamed to what it now proves: a_rendition_delisted_while_subscribing_takes_no_pad.
• What that wait gives up is the other side of the race, a subscription resolving after the teardown, which PumpState decides. Its compare-and-swap pair gets a unit test, a_pump_either_goes_live_or_is_cancelled, covering both directions and the double-claim of each.

No production code changes: the guard was already correct.

Impact

• Public API: none.
• Wire: none.

Alternatives

• Widening the sleep: hides the race behind a slower test and still fails on a slower runner.
• Asserting the invariant in either order (pad or no pad): the assertion becomes a tautology, since a pad is legitimate when the answer wins.

Verification

• just check and just test pass; 132 moq-gst tests, including both tests here.
• Mutation-checked the new unit test: replacing go_live's compare-exchange with an unconditional store fails it. The old test still passed under the same mutation, which is what "only caught it by luck" means.

Follow-ups

• quest/next/flaky-timing-tests.md lists three more real-clock tests with the same shape. This one was a fourth and is now fixed; the quest is unchanged.

(written by Opus 5)

🤖 Generated with Claude Code

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 7dd0dd1f-e65b-4e3a-bb85-a0c830fe3c6e

📥 Commits

Reviewing files that changed from the base of the PR and between 84b34f5 and 7a833f4.

📒 Files selected for processing (1)

• rs/moq-gst/src/source/imp.rs

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

---

Walkthrough

The test module now covers exclusive PumpState transitions. The session test now describes rendition delisting during subscription. It replaces a fixed 500 ms sleep with a bounded wait for the cancelled pump to drop its subscription before the no-pad assertion.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 7a833

The tests now synchronize on subscription release instead of sleeping, improving reliability without changing production behavior. The PR is ready to merge.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: replacing an uncontrolled sleep-based race decision with deterministic cancellation handling in the moq-gst test.
Description check	✅ Passed	The description directly explains the flaky test, the race condition, the deterministic synchronization approach, the new unit test, and the verification results.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files.
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

✨ Simplify code

• Commit to this branch
• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[moq-bot]

No issues found.
Verdict: approve
(Written by Muse Spark)

opencode session  |  github run

[kixelated]

Summary of changes:

• Replaces the 500ms sleep in the flaky moq-gst session test with a real edge wait on Request::poll_unused (the cancelled pump dropping its subscription), so the test stops trying to choose the race. Renamed to a_rendition_delisted_while_subscribing_takes_no_pad; runs in ~0.02s instead of ~0.52s.
• Covers the other side of the race (subscription resolving after teardown) with a new PumpState compare-exchange unit test, a_pump_either_goes_live_or_is_cancelled, both directions plus double-claim.
• Test-only: no production code, public API, or wire changes. Single file: rs/moq-gst/src/source/imp.rs.

CI is green (Check, Test, meta-review), moq-bot approved, CodeRabbit found nothing actionable. Enabling auto-merge (squash). This also unblocks #3867.

(Written by MiMo)