Repository navigation
test: prune duplicate CLI report coverage and refresh cost audit - #7600
Conversation
Quest-Claim: fb8a6f3ae81e7e83
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Delete the finished m1 quest, repoint prune-ci at the recurring audit, and drop validation narrative from the m2 baseline now that #7595 has landed. 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 27 minutes. 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
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: c131ec7
No actionable bugs found. Removing this process test and its private TS rewriting helpers is a proportionate simplification: the retained mid-import/late-sink tests cover report delivery, while the importer’s silent-PID cases explicitly cover silence, unaffected peers, and resumed counts. The recurring audit and release prerequisite remain intact; the cost notes correctly distinguish slot time from CPU and whole-gate speedups.
Verification: static GitHub review only; no tests or benchmarks run, and timing totals were not independently reproduced. The PR’s disclosed pre-existing moq-cli-only listener::Io build dependency on #7091 remains a landing caveat, not a defect introduced here.
(Written by OpenAI)
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Delete the redundant CLI silence report test — removes
report::import_reports_a_silent_video_pidand its privatepcr/suppresshelpers fromrs/moq-cli/tests/integration/report.rs; the keptimport_reports_count_what_each_rendition_wroteanda_late_sink_reads_the_final_reportare untouched and still cover the CLI-to-relay-to-report path. - Retire the m1 prune-tests quest — deletes
quest/m1/prune-tests.md, drops its row fromquest/m1/README.md, and repointsquest/m1/prune-ci.md's test link at/quest/m2/audit-test-cost.md. - Refresh the m2 test-cost audit —
quest/m2/audit-test-cost.mdnow carries the current hosted/local baselines, a rewritten "check first" list, and the owners of competing work (#6987, #7549, #7593, #7489); this deletion is recorded with its measured cost and the retained coverage.
The redundancy claim holds up: pcr/suppress have no other consumer in rs/moq-cli, no reference to prune-tests survives anywhere in the tree, and no import is left unused (relevant because the ci profile denies build.warnings). The removed property is covered directly by import_reports_a_silent_video_pid in rs/moq-mux/src/container/ts/import_test.rs:941 (via assert_stalls: quiet growth, a busy peer, and resumption) and by liveness_steps_over_a_corrupt_pcr_and_the_wrap at rs/moq-mux/src/container/ts/import.rs:4776. The doc's "ci profile marks a test slow at 60 seconds and uses zero retries" claim matches .config/nextest.toml.
One thing to keep in mind outside this PR's code: it touches rs/moq-cli, so its gate will hit the pre-existing unused Io import at rs/moq-relay/src/listener.rs:20 on the moq-cli-only selection (denied by build.warnings). That is on main, owned by #7091, and already documented in the PR body as a landing dependency, so no action is requested here.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Merge
origin/mainatbc2468e8b—dd4db16ebbrings in only upstream main commits (4372e25, da5c2af, bc2468e); no change to the PR's own files. - Conflict resolution in
quest/m1/README.md— the merge's lone conflict was resolved correctly: the two new main quests (fmp4-refusal-framing,mkv-leaf-limits) are inserted, while the PR'sprune-testsrow stays deleted.quest/m1/prune-tests.mdis absent andquest/m1/prune-ci.mdstill links to/quest/m2/audit-test-cost.md, so no dangling reference.
The PR diff against main is unchanged from the prior review, so the earlier assessment stands. Nothing new to act on.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
Main finished quest/m1/prune-ci.md (#7603) and repointed prune-tests.md's CI link; this branch finishes prune-tests.md, so both quests stay deleted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Problem
The off-cycle test audit in
quest/m1/prune-tests.mddefaults to deleting tests whose cost exceeds their unique regression signal. Its recurring baseline still pointed at an older 9,312-test queue run and work now owned elsewhere.Approach
report::import_reports_a_silent_video_pidand its private PCR reader/TS suppressor, 131 lines in one CLI commit. It used 6.932/6.948 seconds in the October 31/November 1 hosted gate runs and 7.763/7.152 seconds in two local selections. These are per-test elapsed slot seconds, not CPU or whole-gate speedups.import_reports_count_what_each_rendition_wrote, which already exercises live CLI report delivery through a relay. The removed test repeats that path to check silence/resumption, covered directly byliveness_steps_over_a_corrupt_pcr_and_the_wrapand importer/stats coverage. Its extra paced clip, two child processes, and TS rewriting harness do not earn their cost.quest/m2/audit-test-cost.mdin place with current hosted/local measurements, partial-run caveats, nightly failure classification and ownership. The recurrence and release prerequisite stay; no successor is created.quest/m1/prune-tests.md. (quest/m1/prune-ci.md, whose boundary link this PR used to repoint, was finished and deleted onmainby moq-dev/moq#7603; the merge keeps it deleted.) Trim the m2 baseline's PR-validation narrative and note that moq-dev/moq#7595 has landed.Coordination and recorded choices
The default-to-deleting decision was recorded by the maintainer on November 1. The concrete deletion and retained cases above are this attempt's judgment under that decision.
moq-dev/moq#6987 owns the multi-crate CPU cuts; moq-dev/moq#7549 owns refused-fill scaling and accounts for the already-landed recovery/tracing/stats cuts; moq-dev/moq#7593 owns io_uring grouping/startup diagnostics. Their changes and savings are not duplicated or claimed here. moq-dev/moq#7595 already fixes the observed relay stack abort without weakening its test. The export failover harness overlaps moq-dev/moq#7489 and is untouched. Dependency manifests, hang/AV1 and JS catalog files are untouched; no dependency becomes unused.
Impact
Alternatives
Moving the redundant test to nightly still pays for duplicate coverage. Retaining its TS rewriting helpers has no consumer after the deletion. Broad CPU optimization would duplicate the open changes above.
Validation
161332deb3(main merged at96244dd854),quest gate nix develop --command just ci origin/mainwith PATH deduplicated as below: passed. Every check module passed.test rsselected moq-cli and ran 290 tests (290 passed, 17 skipped), including the keptreport::import_reports_count_what_each_rendition_wroteandreport::a_late_sink_reads_the_final_report.mainnow cfg-gateslistener::Io, so the temporary feat(cli): report capture stats and solicit feedback from capture #7091 hunk below was no longer needed.quest check --base origin/main: 586 documents ok.Earlier heads:
All commands use the repository's Nix environment and standard mbx launcher. Each local gate run first deduplicated PATH in the calling shell with the same line moq-dev/moq#7594 adds to the shellHook. That is an environment fix, not part of this PR.
c131ec7461(main merged atbcaaef1521; rerun ondd4db16eb1after mergingbc2468e8be, same results),quest gate nix develop --command just ci: the mbxE2BIGfailure is gone. Cargo docs, fmt, shear and every lint module passed.test rsfailed to build the moq-cli-only selection (--features 'moq-cli/play moq-cli/openh264') on the existing unused importwarning: unused import: Io --> rs/moq-relay/src/listener.rs:20:34, whichbuild.warningsdenies. That import is onmain, and this PR doesn't touch it.listener.rshunk applied uncommitted (it cfg-gatesIobehindwebsocket), then restored:just cipassed. 739 Rust tests ran (moq-cli, moq-relay, moq-studio selections): 739 passed, 26 skipped, including the keptreport::import_reports_count_what_each_rendition_wroteandreport::a_late_sink_reads_the_final_report.backend::quic_client_close_drains_finished_trackUnroutablefailure tracked by moq-dev/moq#7590. The import/report selection went from 99 to 98 passing tests after the cut.rs/moq-mux/src/container/ts/import_test.rs:911-935checks a PID going quiet and becoming less quiet after it resumes.liveness_steps_over_a_corrupt_pcr_and_the_wrap(rs/moq-mux/src/container/ts/import.rs:4776) covers the PCR path.quest check --base origin/main: 612 documents ok.check flakedidn't run in thisjust ciselection. The known@moq/netclose-drain flake ("a peer close drains the overtaken group over moq-lite-0N") didn't show up, since no JS was selected. It stays untouched and owned by moq-dev/moq#7590 and moq-dev/moq#7296.Landing dependency
Resolved:
mainnow cfg-gateslistener::Iobehindwebsocket(rs/moq-relay/src/listener.rs:21), so the moq-cli-only selection builds without moq-dev/moq#7091.Review
Head
161332deb3f159d2b692fe745b940e58009b5880, againstorigin/mainat103e98e362:quest review, Codexgpt-6.1-solat medium effort (the other harness to this Claude Code session), round 1: 0 findings.This head merges
origin/mainat96244dd854.mainhad deletedquest/m1/prune-ci.md(moq-dev/moq#7603) and editedquest/m1/prune-tests.md's CI link. Both modify/delete conflicts were resolved by keeping both files deleted. That dropped the PR'sprune-ci.mdhunk, so the diff againstmainchanged and a new review round ran. Thereport.rs,audit-test-cost.md, andquest/m1/README.mdhunks are unchanged apart from context. The head also merges cleanly withmainat103e98e362(moq-dev/moq#7296).Automated reviews on earlier heads (OpenAI review on
c131ec7461; Pullfrog onc131ec7461anddd4db16eb1) reported no findings. CodeRabbit was rate-limited.Earlier rounds on
c131ec7461: Codexgpt-6.1-solmedium and Claude Codeclaude-opus-5-5medium, 0 findings each.Follow-ups
None. The
listener::Iolanding dependency is resolved onmain.No successor quest is created; the recurring m2 audit keeps its schedule.
(Written by GPT-6.1 Sol; iterated by Claude Opus 5.5)