Repository navigation
quest(m1): finish the test cost audit - #6174
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test waited out the stalled re-check's 10 s bound on the wall clock against a wiremock server. The in-process Script already drives the lease on Tokio's paused clock, so the same case now takes milliseconds. The refusal and invalid-grant re-check tests nudge a re-check instead of waiting out the one-second cadence, which the cadence test still covers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It simulated 900 s of media to see the export fail once a 400 ppm source used half the delay. The jitter buffer's own test times that failure at 40 and 400 ppm, and the far-off source case covers the export surfacing the same error within seconds of media. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The server binds IPv4 only. On a host whose /etc/hosts lists ::1 first for localhost, as the NixOS queue runner does, each of the ten QUIC dials first waited out the 250 ms Happy Eyeballs stagger on a dead ::1, so the three tests took 3 s each instead of 0.5 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A connection closed before its handshake drains for three PTOs, and the default 333 ms initial RTT made that three seconds in each split-accept test that waits for the endpoints to go idle. A 10 ms initial RTT, as the handshake timeout test already uses, keeps the same states and takes about 0.2 s. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A Rust crate's benches/ and a JS workspace's bench/ are no dependency of anything else, and the gate's test build never compiles Rust benches. Yet selection walked their owners' dependents: a moq-stats bench edit ran the moq-relay and moq-cli suites, about 40% of the workspace's test time, plus the play build, and a js/net bench edit checked and tested 22 workspaces and built the WASM. Each now selects only its owner, whose lint and typecheck still cover the file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Deletes the m1 run and adds its successor at the end of m2's Required, behind the release cut, with this run's baselines and leads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
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 21 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe m1 test-cost audit requirement and document were removed, and the audit links now point to the m2 audit plan. The m2 roadmap and audit document define recurring measurements, record test timing data, and list investigations. Test changes update revalidation, export timing, and QUIC test setup. JavaScript and Rust package selection now handles benchmark paths separately from dependency expansion. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 8 files. (4 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies completion of the m1 test cost audit, which is the primary change in the pull request. Description check The description directly explains the audit work, test and CI changes, measurements, validation results, and m2 follow-up planning. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 8 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
✨ Finishing Touches
-
✨ Simplify code
-
- Commit to this branch
- Create a new PR
-
- Autofix · 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.
Comment @coderabbitai help to get the list of available commands.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review (Written by Claude Opus 5.5) |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Outcome: Codex found nothing on 4e8a0c2. Queued with the head pinned. This PR leaves no open decisions. Three follow-ups need the maintainer and are listed in the description and in
(Written by Claude Opus 5.5) |
# Conflicts: # quest/m1/README.md # quest/m2/README.md
|
Merged (Written by Claude Opus 5.5) |
# Conflicts: # quest/m1/README.md
|
Merged The only conflict was (Written by Claude Opus 5.5) |
# Conflicts: # quest/m0/ci/audit.md # quest/m1/README.md # quest/m1/audit-test-cost.md # quest/m2/README.md # rs/moq-auth/src/client.rs
|
Merged
(Written by Claude Opus 5.5) |
Finishes the m1 run of the recurring test cost audit (
quest/m1/audit-test-cost.md) and plans the m2 run behind the release cut. It removes about 50 test-seconds from a full queue selection and stops benchmark-only edits from running their dependents' suites.Changes (one commit each)
Scriptthe other lease tests use. The refusal and invalid-grant re-check tests nudge a re-check instead of waiting out the 1 s cadence, whichrevalidate_runs_on_cadence_and_applies_the_replystill covers.a_source_past_the_limit_fails_once_half_the_delay_is_gone. It simulated 900 s of media in a debug build to see a 400 ppm source fail once it used half the delay.jitter::tests::a_source_past_the_drift_limit_failsalready times that failure at ±40 and ±400 ppm, anda_source_far_off_real_time_fails_within_secondscovers the export surfacing the same error.127.0.0.1. The server binds IPv4 only. The NixOS queue runner lists::1first forlocalhost, so each of the ten QUIC dials per test first waited out the 250 ms Happy Eyeballs stagger on a dead::1. I reproduced this locally by bind-mounting such an/etc/hostsin a user namespace. A whole-suite run under it showed only these three tests regressing.handshake_timeouttest already uses 10 ms.benches/or a workspace'sbench/, and the gate's test build never compiles Rust benches (test_unexecuted_bench_is_not_compiled). Before this change, a moq-stats bench edit ran the moq-relay and moq-cli suites, about 40% of the workspace's test time, plus theplaybuild. A js/net bench edit checked and tested 22 workspaces and built the WASM. Now each selects only its owner, whose lint or typecheck still covers the file. This answers the quest's "still to measure" item.quest/m2/audit-test-cost.mdat the end of m2'sRequired(it requires the release cut), and points the two links at it.Measured
a_close_is_reported_while_a_recheck_is_stalleda_refusal_on_recheck_revokes/an_invalid_grant_on_recheck_revokesclose_code::*(×3),::1-first hostssplit_accept_*(×4)Local times are from 3 to 5 repeats on a 24-core host at load average 20 to 30.
The baseline is queue run 39546261474, a full selection on the 8-core runner VM. nextest ran 9,312 tests in 127 s, and their summed time was 995 s, so the run is limited by slot time. This PR cuts about 50 test-seconds from that sum, about 6 s of wall time at 8 threads.
Public API and wire
None. These are test-only and CI selection changes.
Checks
just check: the select.sh change selects the whole workspace. Lint, JS, gh and every other module pass. The Rust tests pass exceptexport::redundant_importers_switch_through_bufferbloat_on_lite_07, the known load flake that moq-dev/moq#6113 fixes. The rerun with--no-fail-fastgave 9,316 of 9,317 passing. The capture and play test builds did not run after that failure. This PR touches nothing they build.python3 sh/rs/select.test.pypasses with a new case.python3 sh/js/workspace.test.pypasses excepttest_dispatcher_passes_only_js_impact_paths, which also fails on main: it chmods thejuststub before writing it. moq-dev/moq#6145 fixes that.quest checkpasses.Suggested follow-ups
NEXTEST_TEST_THREADS=24for the queue job only, after a few loadedjust check --allruns at that setting. The risk is more load on timing-sensitive tests, where a flake ejects a queue entry. Only the known redundant-importer flake failed, once, at 16 threads.stopcloses a client whose dial to the unreachable test relay never got an answer, and the close waits three PTOs. A user stopping a pipeline whose relay is down waits the same.--quic-idle-timeoutnor moq-uring'sTransportsets it.moq-dev/moq#6113 already cuts the three slowest tests, the redundant-importer exports at 40, 29 and 18 s, by shortening the killed importer's idle timeout. It edits the quest file this PR deletes, so whichever lands second resolves that by dropping the edit.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code