Skip to content

test: prune duplicate CLI report coverage and refresh cost audit - #7600

Merged
kixelated merged 7 commits into
mainfrom
quest/m1/prune-tests
Nov 2, 2026
Merged

kixelated merged 7 commits into
mainfrom
quest/m1/prune-tests

Conversation

@kixelated

@kixelated kixelated commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The off-cycle test audit in quest/m1/prune-tests.md defaults 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

  • Delete report::import_reports_a_silent_video_pid and 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.
  • Keep 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 by liveness_steps_over_a_corrupt_pcr_and_the_wrap and importer/stats coverage. Its extra paced clip, two child processes, and TS rewriting harness do not earn their cost.
  • Keep late-sink reports, stdin-EOF catalog/media cases, compressed-cache memory bounds, io_uring backlog/deadline cases, macro UI diagnostics, ACME behavior, and reconnect coverage. Their boundaries are distinct.
  • Refresh quest/m2/audit-test-cost.md in place with current hosted/local measurements, partial-run caveats, nightly failure classification and ownership. The recurrence and release prerequisite stay; no successor is created.
  • Finish the m1 quest: delete quest/m1/prune-tests.md. (quest/m1/prune-ci.md, whose boundary link this PR used to repoint, was finished and deleted on main by 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

  • Public API: none.
  • Wire: none.
  • Production behavior and remaining test assertions: unchanged. One redundant process test is deleted.

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

  • Head 161332deb3 (main merged at 96244dd854), quest gate nix develop --command just ci origin/main with PATH deduplicated as below: passed. Every check module passed. test rs selected moq-cli and ran 290 tests (290 passed, 17 skipped), including the kept report::import_reports_count_what_each_rendition_wrote and report::a_late_sink_reads_the_final_report. main now cfg-gates listener::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.

  • Head c131ec7461 (main merged at bcaaef1521; rerun on dd4db16eb1 after merging bc2468e8be, same results), quest gate nix develop --command just ci: the mbx E2BIG failure is gone. Cargo docs, fmt, shear and every lint module passed. test rs failed to build the moq-cli-only selection (--features 'moq-cli/play moq-cli/openh264') on the existing unused import warning: unused import: Io --> rs/moq-relay/src/listener.rs:20:34, which build.warnings denies. That import is on main, and this PR doesn't touch it.
  • Same head, rerun with moq-dev/moq#7091's listener.rs hunk applied uncommitted (it cfg-gates Io behind websocket), then restored: just ci passed. 739 Rust tests ran (moq-cli, moq-relay, moq-studio selections): 739 passed, 26 skipped, including the kept report::import_reports_count_what_each_rendition_wrote and report::a_late_sink_reads_the_final_report.
  • Earlier attempt: the full default-feature workspace selection ran 10,623 tests before source edits, with the known backend::quic_client_close_drains_finished_track Unroutable failure tracked by moq-dev/moq#7590. The import/report selection went from 99 to 98 passing tests after the cut.
  • Coverage of the removed property: rs/moq-mux/src/container/ts/import_test.rs:911-935 checks 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 flake didn't run in this just ci selection. The known @moq/net close-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: main now cfg-gates listener::Io behind websocket (rs/moq-relay/src/listener.rs:21), so the moq-cli-only selection builds without moq-dev/moq#7091.

Review

Head 161332deb3f159d2b692fe745b940e58009b5880, against origin/main at 103e98e362:

  • quest review, Codex gpt-6.1-sol at medium effort (the other harness to this Claude Code session), round 1: 0 findings.

This head merges origin/main at 96244dd854. main had deleted quest/m1/prune-ci.md (moq-dev/moq#7603) and edited quest/m1/prune-tests.md's CI link. Both modify/delete conflicts were resolved by keeping both files deleted. That dropped the PR's prune-ci.md hunk, so the diff against main changed and a new review round ran. The report.rs, audit-test-cost.md, and quest/m1/README.md hunks are unchanged apart from context. The head also merges cleanly with main at 103e98e362 (moq-dev/moq#7296).

Automated reviews on earlier heads (OpenAI review on c131ec7461; Pullfrog on c131ec7461 and dd4db16eb1) reported no findings. CodeRabbit was rate-limited.

Earlier rounds on c131ec7461: Codex gpt-6.1-sol medium and Claude Code claude-opus-5-5 medium, 0 findings each.

Follow-ups

None. The listener::Io landing dependency is resolved on main.

No successor quest is created; the recurring m2 audit keeps its schedule.

(Written by GPT-6.1 Sol; iterated by Claude Opus 5.5)

kixelated and others added 2 commits November 1, 2026 18:20
Quest-Claim: fb8a6f3ae81e7e83
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
Co-Authored-By: GPT-6.1 Sol <noreply@openai.com>
@kixelated kixelated changed the title Delete the tests that don't earn their cost test: prune duplicate CLI report coverage and refresh cost audit Nov 2, 2026
kixelated and others added 2 commits November 1, 2026 22:05
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>
@kixelated
kixelated marked this pull request as ready for review November 2, 2026 06:36
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

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 27 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
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 45573a0a-3cf2-47a8-a4ab-d54e4d9a7c39
Commits

Reviewing files that changed from the base of the PR and between 96244dd and 161332d.

Files selected for processing: 4
  • quest/m1/README.md
  • quest/m1/prune-tests.md
  • quest/m2/audit-test-cost.md
  • rs/moq-cli/tests/integration/report.rs
  • 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.

Share

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Delete the redundant CLI silence report test — removes report::import_reports_a_silent_video_pid and its private pcr/suppress helpers from rs/moq-cli/tests/integration/report.rs; the kept import_reports_count_what_each_rendition_wrote and a_late_sink_reads_the_final_report are 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 from quest/m1/README.md, and repoints quest/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.md now 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.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Merge origin/main at bc2468e8b — dd4db16eb brings 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's prune-tests row stays deleted. quest/m1/prune-tests.md is absent and quest/m1/prune-ci.md still 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.

Pullfrog  | View workflow run | Using 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>
@kixelated
kixelated added this pull request to the merge queue Nov 2, 2026
Merged via the queue into main with commit 46ebdbe Nov 2, 2026
9 of 10 checks passed
@kixelated
kixelated deleted the quest/m1/prune-tests branch November 2, 2026 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant