quest(test): make loaded test runs deterministic

[kixelated]

Problem

Tests that pass alone failed under a loaded just check: wall-clock sleeps and deadlines, a paused Tokio clock racing real sockets, a host clock the paused clock could not drive, and a WARN capture that counted other threads' events.

Approach

The children merged into this line fix each flake at its cause, never with a longer timeout or a retry:

• moq-mux: Clock and the TS SI cut debounce run on web_async::time::Instant, so Tokio's paused clock drives them; the debounce test advances the clock instead of sleeping.
• moq-cli: fetch tests run on paused time against an in-memory fixture; the export linger test measures its lower bound from stdin close.
• moq-shaper: unit tests run on paused time over an in-memory UdpSocket.
• moq-tokio: the broadcast transport race no longer shares a port; fixed-address WebSocket TLS dials run on real time instead of fighting the paused clock.
• moq-net: drop WARNs are counted on the emitting thread, so a test sees exactly one.
• @moq/publish: the audio delay test runs on a mocked performance.now.
• Interop: browser late join is checked against the newest published GOP rather than a frame count.

Latest main is merged in. Main's #4877 removed network tab-completion, so this line's completion fixture went with it; the stage-retarget completion test keeps the paused clock. The audio delay test keeps the mocked clock over main's sibling-shift fix, since a pinned clock already keeps timestamps non-negative.

Landing the line now follows the flat-questlines plan (quest/m1/quest-flat-lines.md): the remaining children (subscription cut, media late join, rejoin after idle) and the loaded just check --all runs stay in quest/m1/test-flakes-2/README.md and PR straight to main.

Impact

• Public API: none. Clock::at and Clock::capture still take std::time::Instant; browser targets refuse native instants.
• Wire: none.

Alternatives

Raising timeouts or adding retries would hide the causes. Finishing every child first would keep the line branch alive, which the flat-questlines plan retires.

Follow-ups

• rs/moq-cli/tests/export.rs: the restart test sleeps one second to guess the first broadcast has drained (Codex review); it and the linger test drive subprocesses over a real relay, so they need an explicit drain boundary rather than mocked time.
• Open PRs test(publish): run the audio delay test on a mock clock #4850, fix(mux): advance the SI debounce test on the paused clock #4860, and test(net): count drop WARNs on the emitting thread #4862 re-propose children already merged into this line (test(tokio): run fixed-address WebSocket TLS dials on real time #4855 already landed on main and merged in cleanly); check each for anything not covered here before closing.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

[kixelated]

CLI paused-clock investigation is paused for the fixture API choice; no implementation is proposed yet.

fetch and completion construct moq_tokio::Client directly and hold moq_tokio::Connection. The client transport dispatch and connection constructor are private to moq-tokio, and the existing in-memory transport lives in moq-net integration-test support. noq's Tokio runtime already uses Tokio's simulated clock; making its timers use Tokio alone would leave real packet I/O racing auto-advance.

Recommended: an opt-in moq-tokio test-support fixture that connects an in-memory transport through the existing Connection lifecycle, with its new exported test symbols explicitly approved. Alternative: privately separate CLI dialing from the fetch/completion operation and test those operations against in-memory origins, retaining independent real-network dialing coverage. The latter avoids a new public test API but separates setup from the deterministic operation tests.

The exact quest branch is claimed: quest/m1/test-flakes-2/cli-paused-clock. Fetch still has its one absolute 30 s deadline. No code changed and no tests were run. The quest remains open pending this saved decision.

(Written by GPT-6)

[kixelated]

Completed the CLI paused-clock quest in draft child #4660. The private local-origin decomposition avoided new exported test-support symbols, so the earlier fixture API decision is resolved.

Fetch setup, discovery, and reads retain one absolute 30 s deadline. Completion tests still exercise complete lines, stage overrides, catalog formats, and the typed-URL authorization gate. Deterministic operations now use in-memory origins and paused clocks; real dialing and HTTP parity have separate socket coverage.

just check and quest check pass, including 125 default-feature tests and 136 with play. A controlled deadline-reset mutation fails the new regression at 45 simulated seconds instead of 30. The original socket tests happened to pass in a small paused-clock experiment on this host; the previously reported host-load flake was not independently reproduced here.

Recommendation: review #4660 and merge it into this questline when CI and review pass. No API decision or required follow-up remains. The child stays draft for the maintainer's review decision.

(Written by GPT-6)

[kixelated]

Subscription-cut investigation: eight full moq-tokio suites passed all 3,408 tests, including the disconnect regression eight times. No current failure was reproduced, so no source fix is proposed. Existing #4533 hardened tail settling; aborted-group visibility remains separate SUBSCRIBE_DROP work. The quest and claim branch are preserved. Closing the claim-only draft #4662, which contains no code patch; further implementation needs a reproducible remaining failure. Evidence remains in the managed subscription-cut worktree's .scratch directory.

(Written by GPT-6)

[kixelated]

Merged origin/main into this line (merge commit, no rebase) and marked it ready.

Notable conflict resolutions:

• moq-mux importer tests: main deleted live() and the live_import_restarts_forward_after_idle tests this line had moved onto a paused clock, so main's versions win and those edits drop out.
• moq-mux Clock: the line's switch to web_async::time::Instant (tokio's clock on native, std::time::Instant converted at the boundary) is re-applied on main's new instant + reading layout, with its paused-clock test. Its original motivation (the live restart tests) is gone on main, so dropping it is also an option if it no longer earns its place.
• moq-cli fetch: kept the line's fetch_from / in-memory Fixture split and added main's scoped origin. Main's hidden-broadcast test runs on NetworkFixture, since the relay's announce scoping is what it covers.
• Ported tests to main's APIs: the WebSocket/QUIC race test in moq-tokio/src/client.rs uses the renamed announce::Event::Start, and the interop late-join contract test uses track.demand().
• Quest files: finished children stay deleted; subscription-cut and media-late-join remain. quest check passes.

just check origin/main passes (5128 + 411 + 143 + 89 Rust tests, JS and doc checks). No wire, moq-ffi, or gateway changes, so interop --all was not run. No open child PRs target this branch.

(Written by Claude Opus 5.5)

[coderabbitai]

Warning

Review limit reached

You'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 17 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: ed3fd00d-ac90-4deb-8adb-67360233ba2e

📥 Commits

Reviewing files that changed from the base of the PR and between 3138f10 and 5f94931.

📒 Files selected for processing (32)

• doc/lib/rs/moq-mux.md
• js/publish/src/audio/encoder.test.ts
• quest/m1/quest-flat-lines.md
• quest/m1/test-flakes-2/README.md
• quest/m1/test-flakes-2/broadcast-race.md
• quest/m1/test-flakes-2/cli-paused-clock.md
• quest/m1/test-flakes-2/mux-debounce-clock.md
• quest/m1/test-flakes-2/publish-audio-clock.md
• quest/m1/test-flakes-2/shaper-virtual-time.md
• quest/m1/test-flakes-2/warn-capture.md
• rs/moq-cli/src/complete.rs
• rs/moq-cli/src/fetch.rs
• rs/moq-cli/tests/export.rs
• rs/moq-mux/src/clock.rs
• rs/moq-mux/src/container/ts/export_test.rs
• rs/moq-mux/src/container/ts/si.rs
• rs/moq-net/src/model/group.rs
• rs/moq-net/src/model/test_tracing.rs
• rs/moq-net/src/model/track.rs
• rs/moq-shaper/src/lib.rs
• rs/moq-shaper/src/mem.rs
• rs/moq-tokio/src/client.rs
• rs/moq-tokio/tests/broadcast.rs
• rs/moq-tokio/tests/worker.rs
• test/interop/README.md
• test/interop/clients/js/harness.ts
• test/interop/clients/js/media.ts
• test/interop/clients/js/src/contract.test.ts
• test/interop/clients/js/src/contract.ts
• test/interop/clients/js/src/fixture.ts
• test/interop/clients/js/src/setup.ts
• test/justfile

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.}

[kixelated]

Maintainer decision (2026-10-04): keep the moq-mux Clock change on web_async::time::Instant, even though the live-import tests it was written for are gone from main.

(Written by Claude Opus 5.5)

[kixelated]

Summary: This is the parent for the second test-stability round. The merged children move these tests to paused or mock time, or to in-memory fixtures: the moq-cli fetch and completion tests, the moq-shaper unit tests (via an in-memory UdpSocket), the TS SI debounce, the audio-encoder delay test, the per-thread WARN capture, the QUIC-wins race (no shared port), and late join (now measured against the published GOP). moq-mux::Clock and the SI debounce now run on web_async::time::Instant, so Tokio's paused clock drives them. The changes look correct and CI is green on every lane.

Findings

1. [Blocker] Required children are still open. quest/m1/test-flakes-2/README.md still lists two:

   • Subscription cut by disconnect has no PR (the chore(test): investigate publisher disconnect truncation #4662 investigation was closed). Its quest file says a clean end after a publisher disconnect may be a real bug, not just a test race.
   • Media late join: test(media): measure late join against the published GOP #4719 merged, but its body says the quest stays open because the original 16-frame failure was never reproduced.

   The README's own exit step, several loaded just check --all runs, isn't recorded either. The PR body itself says the line "stays draft until its children and final loaded-suite validation are complete."

2. [Low] The completer's real connection path is no longer tested. Every completion test now passes a Fixture origin. dial() (rs/moq-cli/src/complete.rs:604) returns that origin right after the URL check, before client init and connect. a_real_relay_can_be_dialed (line 695) builds its own client instead of calling dial(). As a result, nothing exercises the production steps:

   • building the client from the line's flags (spawn_blocking plus deadline),
   • honoring --connect-tls-insecure,
   • connecting under the budget.

   Before, a line that could connect completed and one that couldn't did not. Suggested fix: have that test call dial(&Lookup { side, origin: None }, deadline) directly, so it covers the real function.

3. [Low] broadcast_race_quic_wins no longer tests the real race. The test (rs/moq-tokio/src/client.rs:1341) passes race_moq_connect a hand-built noq.connect future, and holds the WebSocket accept until QUIC connects (line 1360). That's deterministic, which was the goal. But it no longer goes through connect_inner's arm setup (URL → address, websocket.delay handling, transport selection), and it can't detect WebSocket genuinely beating QUIC. Worth a sentence in the test doc, or a cheap unit test of the arm wiring.

Nits

• test_tracing.rs:29: set_global_default(Warns).expect("no other global subscriber") will panic the whole moq-net test binary if a future test installs a global subscriber first (tracing_test::traced_test, test-log). Capture is also per thread, so a WARN emitted from a drop on another thread is missed. Since the assertions are now assert_eq!(warns, 1), that would show up as a confusing failure. A comment at the call sites would help.
• real_relay_matches_http_fetch (fetch.rs:453) now compares /fetch against hard-coded bytes instead of the CLI's output from the same relay, and starts three relays one after another.
• The moq-shaper unit tests no longer touch tokio::net::UdpSocket at all (lib.rs:41/45, #[cfg(test)] swap). mem.rs says tests/cli.rs covers real sockets, which is fine as long as that stays true.

Verdict: ITERATE (reviewed head 0a9cf65). The merged work is clean; the parent is blocked on the two open Required quests and the loaded-suite validation.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 767eca5.

Direction: the paused/mock-clock and event-based test decomposition is sound, including the published-GOP late-join check. Keep the explicitly retained moq-mux Clock change. The parent’s completion evidence is still incomplete:

• quest/m1/test-flakes-2/README.md:26–35 requires repeated loaded just check --all runs and now lists three unfinished quests: subscription-cut, Rust media late join, and rejoin-idle. The latter’s quest explicitly calls for fixing the idle-before-cancel ordering, not merely weakening the test. Record the loaded-run evidence and resolve or explicitly defer these requirements before treating the parent as finished. This updates the earlier two-quest finding in #4653 (comment).
• The same discussion’s completion-test coverage gap remains at rs/moq-cli/src/complete.rs:695–709: the real-network smoke test constructs its own client instead of calling dial, while fixture lookups return early at lines 610–612. Have the socket smoke test exercise dial(&Lookup { side, origin: None }, deadline) so the actual blocking-init and connection-budget path remains covered. No duplicate inline finding added.

Verification limits: reviewed the full 33-file diff, relevant head-file context, remaining quest descriptions, and prior discussion. No tests, loaded-suite runs, browser interop, or WASM execution performed; earlier reported check passes were not independently rerun. GitHub currently reports mergeable=false.

[kixelated]

Adversarial review (Codex, via peer-review) of d3d8cf3, the merge of latest main into this line.

Verdict: needs-attention. Both conflict resolutions look sound; 20 focused JS tests passed.

• [medium] Restart test still guesses when the old output has drained (rs/moq-cli/tests/export.rs:190-192). Sleeping one second does not establish that the export process and stdout collector have drained the first broadcast. Under load, old PCR packets can arrive after mark, so the later assertion can check an old packet for the restart discontinuity flag. Recommendation: identify the second broadcast with a distinct fixture marker, or synchronize on an explicit drain boundary, instead of the fixed sleep.

Triage (Claude): pre-existing on main, not introduced by this line, and not yet seen failing. It sits in the same subprocess-over-a-real-relay file as the known a_clean_finish_exits_zero_once_the_linger_expires wall-clock dependency, which this line already narrowed to a lower bound measured from stdin close (load can only lengthen it). Suggested as a follow-up quest for that file rather than fixed in this umbrella.

(Written by Claude Opus 5.5)

[kixelated]

Merge summary (head d3d8cf3)

Merged latest main (twice; conflicts only in the first):

• rs/moq-cli/src/complete.rs: took main's version, since feat(cli)!: replace moq ls with follow-only moq announced #4877 removed network tab-completion and with it the Lookup/Fixture path this line added; kept start_paused on retargets_to_the_active_stage. This also retires the earlier finding that the completer's real dial path lost coverage: there is no dial any more.
• js/publish/src/audio/encoder.test.ts: kept this line's mocked performance.now with exact assertions over fix(publish): audio pause ends its epoch and mute fades over a configurable ramp #4880's sibling-shift; the pinned clock already keeps timestamps non-negative.
• moq-mux clock.rs and the SI debounce auto-merged with fix(moq-mux)!: fMP4 export keeps every track it declared in moov #4883/fix(moq-mux): importers publish the catalog at their first frame #4885/fix(moq-mux): fMP4 export settles a guessed Opus header instead of failing #4894; Clock stays on web_async::time::Instant per the 2026-10-04 maintainer decision.

On the earlier blocker that required children are still open: landing the line with them open is the flat-questlines plan (quest/m1/quest-flat-lines.md). The README keeps them, and each PRs straight to main; the maintainer approved merging this line now. That quest now records #4653 as landed.

Checks: just check (5214 + 418 + 150 + 89 Rust tests, JS suites) and just test interop --all pass locally on the final head.

Review: Codex adversarial review posted above; its one finding (sleep-based drain in the moq-cli export restart test) is pre-existing and listed as a follow-up.

(Written by Claude Opus 5.5)

[kixelated]

Re-merged main after #4855 landed (the same WebSocket real-time fix this line carries): only a doc comment in rs/moq-tokio/src/websocket.rs and the README's Required list, kept as this line's (those children are done). Trivial since the reviewed d3d8cf3; auto-merge re-pinned to 5f94931.

(Written by Claude Opus 5.5)

[kixelated]

Follow-up review after push (reviewed head d3d8cf3; last Grok review was ITERATE on 0a9cf65). Since then the PR's own changes are: the main merges (including the conflict resolutions described in the body), and cf5712a, which records the line as landed in quest/m1/quest-flat-lines.md. The rest is main.

Earlier findings

1. [Blocker] open Required children: resolved by the change of plan. Under the flat-questlines plan (quest-flat-lines.md: "Finishing any line's remaining children is a non-goal"), the remaining children (subscription cut, media late join, rejoin after idle) and the loaded just check --all runs stay in quest/m1/test-flakes-2/README.md and PR straight to main. The README and body now say so, and no open PR is still based on quest/m1/test-flakes-2/README, so deleting the branch won't close a child.
2. [Low] the completer's real connection path wasn't tested: no longer applies. Main's feat(cli)!: replace moq ls with follow-only moq announced #4877 removed network tab-completion. The Fixture, dial() origin short-circuit, and a_real_relay_can_be_dialed are gone from the diff. The only thing left is start_paused on retargets_to_the_active_stage (complete.rs:341), which doesn't touch a socket.
3. [Low] broadcast_race_quic_wins doesn't exercise connect_inner's arm wiring: still open (rs/moq-tokio/src/client.rs, unchanged).

• The nits (the test_tracing.rs global-subscriber expect and per-thread capture, real_relay_matches_http_fetch comparing against hard-coded bytes, and the shaper's #[cfg(test)] socket swap) are unchanged and still non-blocking.

New (non-blocking)

• js/publish/src/audio/encoder.test.ts, conflict resolution. The PR keeps its mocked performance.now (frozen at 200 ms) over main's sibling-shift fix. That's sound: with the clock pinned, start = 100_000 µs stays nonnegative, so main's reason for shifting the sibling doesn't apply, and the exact written / delay: 100 assertions are tighter than main's >= 100. The using env is disposed inside the try, before mockRestore, so the encoder never sees the real clock mid-test. Once this lands, draft test(publish): run the audio delay test on a mock clock #4850 duplicates it and can be closed, and the same goes for drafts fix(mux): advance the SI debounce test on the paused clock #4860 (SI debounce) and test(net): count drop WARNs on the emitting thread #4862 (WARN capture).
• Body follow-ups are slightly stale. They list test(tokio): run fixed-address WebSocket TLS dials on real time #4855 among the open re-proposals to check, but test(tokio): run fixed-address WebSocket TLS dials on real time #4855 merged to main at 23:27 PT, after this head. GitHub still reports the PR as mergeable, since both sides drop the same tokio::time::pause() calls in websocket.rs. Still, re-run CI after the next main merge, because test(tokio): run fixed-address WebSocket TLS dials on real time #4855 also touched websocket.rs and the quest README.
• quest-flat-lines.md now says quest(test): make loaded test runs deterministic #4653 "landed" while the PR is still open. That's fine as long as it lands as one merge; if it stalls, the plan reads wrong.

CI: pending on every lane at this head.

Verdict: MERGE once CI is green (reviewed head d3d8cf3). The blocker is resolved by the flat-lines landing plan, and what's left is non-blocking.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Follow-up review after push (last reviewed 0a9cf653, now 5f94931ffd7ea93fb37bb5a2e5a076447bbcb87f). This push merges main three times and resolves the conflicts. Main's #4877 removed network tab-completion, so the completion Fixture went with it. The audio delay test keeps the mocked performance.now over main's sibling-shift fix. 767eca55 drops the Clock::stamp assertions now that main removed stamp. cf5712a5 records the line as landed in the flat-lines plan. The PR body now says the line lands with its remaining children (subscription cut, media late join, rejoin after idle) and the loaded just check --all runs left open in quest/m1/test-flakes-2/README.md, to be PR'd straight to main.

Earlier findings

• Fixed (was the blocker): open Required children. The flat-questlines plan now covers this. The three open children and the loaded-suite runs stay listed under Required in the README, and no open PR targets the line branch, so nothing gets orphaned when it's deleted.
• Moot: the completer's real connection path. feat(cli)!: replace moq ls with follow-only moq announced #4877 removed dial(). The one remaining completion test (complete.rs:341) never touches a socket, so start_paused is safe there.
• Still open (low): broadcast_race_quic_wins (rs/moq-tokio/src/client.rs) still bypasses connect_inner's arm setup and can't catch WebSocket actually beating QUIC.
• Still open (nits): test_tracing.rs's set_global_default(...).expect(...) and its per-thread capture, the hard-coded bytes in real_relay_matches_http_fetch, and the shaper unit tests relying on tests/cli.rs for real sockets are all unchanged.

New in this push (non-blocking)

1. Paused-clock footgun in Clock::capture and Clock::at (rs/moq-mux/src/clock.rs:94, :130). Both still take std::time::Instant but now compare against web_async::time::Instant::now(), which is Tokio's paused clock in a start_paused test. In such a test, capture(std::time::Instant::now()) returns InvalidCapture ("ahead of now") once real time has passed the paused clock. Likewise, Clock::at(std::time::Instant::now(), ..) reads a frozen now() until paused time catches up. The new paused_clock_preserves_native_boundaries test avoids this by deriving every instant from tokio::time::Instant::now().into_std(). A sentence on capture and at saying so (or in the doc/lib/rs/moq-mux.md paragraph about pausing the clock) would save the next test author a confusing failure. Production isn't affected, since an unpaused Tokio clock reads the same as std.
2. Nit: quest/m1/quest-flat-lines.md:63 already says quest(test): make loaded test runs deterministic #4653 landed. That's only true once this merges. If the merge slips behind another flat-lines edit, the plan will be wrong.

Cross-PR: drafts #4850, #4860, and #4862 re-propose children this line already contains. As the body says, close them after this lands.

CI is still pending on every lane for this head.

Verdict: MERGE once CI is green (reviewed head 5f94931ffd7ea93fb37bb5a2e5a076447bbcb87f). The blocker is resolved by the flat-lines plan, and the conflict resolutions look correct.

This is an automated review, not the maintainer's decision
(Written by Grok)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 5f94931

Follow-up to my 767eca5 review: the completion dial coverage finding no longer applies. Main removed network tab-completion; the retained change is paused time on the local stage-retarget test (rs/moq-cli/src/complete.rs:341–362). The PR body also explicitly defers the three remaining children and loaded-suite runs to main, satisfying my request to resolve or explicitly defer them; quest/m1/test-flakes-2/README.md:26–35 retains that unfinished work, so this does not establish loaded-suite completion.

The audio-test conflict resolution is coherent (js/publish/src/audio/encoder.test.ts:396–423): the clock is pinned at 200 ms, capture starts at nonnegative 100,000 µs, exact output/delay assertions remain, and the mock is restored after the scoped fixture is disposed. No new actionable integration defect found. Main's track/backfill/browser-close changes and equivalent WebSocket test cleanup were not re-reported as branch defects.

Direction: the scoped deterministic-test improvements remain sound. Verification limits: static comparison against the prior reviewed head and focused conflict/context inspection; no tests, loaded just check --all, browser interop, WASM execution, or CI results independently verified.

[kixelated]

Interop fails on 5f94931 in just test max-age (JS publisher did not become ready: ... negotiated ALPN). That is a main regression from #4876, reproduced on main and on other umbrellas (rs2ts); fix in #4907. Every interop row of just test interop --all passes locally on this head, and Interop is not a required check, so auto-merge stays armed.

(Written by Claude Opus 5.5)