Repository navigation
ci(rs): lint benches and examples in the PR gate - #7786
Conversation
The moq-pattern matching bench's varint match had overlapping ranges (`..64`, `..16384`), which `cargo clippy --all-targets` rejects as `match_overlapping_arm`. The gate's clippy runs through the `--tests` nextest build, which never compiles bench or example targets, so only Nightly could see it. Make the varint ranges disjoint, and have the gate's test build (sh/rs/select.sh `test` and `just rs check-test`) also compile `--benches --examples` so clippy-driver lints them; nextest's default filter still leaves the benches unrun. 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 11 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 head b45ec9d.
No actionable findings. The disjoint varint arms preserve the intended buckets, and both selected test builds and check-test now include bench/example targets while the nextest filter excludes benchmark execution. The selector tests reflect the new flags, and keeping ordinary targeted rs test unchanged limits unnecessary release builds.
Verification: GitHub-only static review of the gate, selector, nextest configuration and changed bench. I did not run CI, tests, or the timing measurements.
There was a problem hiding this comment.
ℹ️ One coverage gap worth confirming; the rest looks right.
Reviewed changes
- Gate builds bench/example targets:
sh/rs/select.sh'stestaction now appends--benches --examples, so the clippy-driver-wrapped nextest build lints them in PRs instead of only in Nightly. - Bench lint fix:
varint's overlapping match arms become disjoint0..64/64..16384, preserving the 1/2/4-byte buckets thatmatch_overlapping_armrejected. - Targeted test recipe unchanged:
just rs testkeeps--tests; onlycheck-testand the selector'stestaction add the new targets, so--releasebenchmark runs don't build them. - Docs and tests updated:
.config/nextest.tomlandrs/justfilecomments plussh/rs/select.test.pyexpectations match the new flags.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
Cargo skips a bench or example whose `required-features` are off without a word, so `--benches --examples` still left moq-quic's benches, moq-stats' `producer_tick`, and on a bench-only change moq-net's `fuzz` and moq-relay's `session` benches unlinted. sh/rs/select.sh's `test` action now reads the built crates' target `required-features` from `cargo metadata` and turns them on in the one test build, beside the device picks. They are all additive. `just rs check-test` drops `--benches --examples`: it forwards the same args to `cargo doc`, which has no `--benches`, so it could not carry the features, and nothing in CI runs it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head 415e69b, focusing on the update since b45ec9d.
No new actionable finding. The test selector now gathers required features for selected bench and example targets, including the isolated crates cited in the earlier review, addressing the gap in gate coverage. The ordinary check-test recipe no longer includes those targets, but the gate uses the selector's test action.
Verification: GitHub-only static inspection of the selector, manifests and tests. I did not run the gate, tests or clippy.
(Written by review, OpenAI)
There was a problem hiding this comment.
ℹ️ No critical issues — one minor test-organization suggestion inline.
Reviewed changes
Reviewed the delta since the prior pullfrog review (b45ec9d), which adds required-feature coverage to the gate.
- Bench/example required-features enabled:
sh/rs/select.sh'stestaction now reads every bench and examplerequired-featuresfromcargo metadatafor the crates in the build and appends them to the existing device/runtime feature picks. - Build membership tracked:
buildingcarries the selected crates plus the media packages the action adds, so targets' features stay scoped to what the build holds. - Tests extended:
sh/rs/select.test.pyderives the workspace feature list from metadata and asserts four bench-only selections enable their required feature; the metadata fixture expectation gains--benches --examples. - Comment updated:
rs/justfilenow describes the required-features behavior.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: 1a135b0.
The test-only update restores the moq-stats library-change dependent-selection assertion to its own test with the intended bench path, rather than inheriting the feature loop's last path. No additional actionable finding in this delta. Verification: static GitHub inspection; I did not run the selector tests, gate, or clippy.
(Written by review (OpenAI))
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since the prior pullfrog review (415e69b): a test-only follow-up in sh/rs/select.test.py.
- Restored the dependent-propagation check: the moq-stats bench plus
rs/moq-stats/src/lib.rsselection assert thatmoq-relayis pulled in now lives back intest_a_bench_selects_only_its_crate, wherebenchis the moq-stats target it was written for. - Cleaned up the required-features test:
test_a_bench_alone_enables_its_required_featuresno longer reuses the loop's leftoverbenchvariable after its loop.
python3 sh/rs/select.test.py runs 24 tests; the only failures are the known env-only ones (sudo no-new-privileges / missing just). The reorganized tests pass.
deepseek-v4.1-flash-1017 (free via Pullfrog for OSS) | 𝕏

Problem
cargo clippy --all-targetsfails on main, with default features too: the moq-pattern matching bench added in moq-dev/moq#7536 matches a varint size with..64and then..16384, andclippy::match_overlapping_armrejects the overlap. The PR gate didn't catch it. The gate runs clippy through the--testsnextest build (moq-dev/moq#5531), and that build never compiles bench or example targets. Only Nightly'sfeaturesjob lints them, and it runs after the merge.sh/rs/select.shstill selected a crate when itsbenches/changed "for its lint", but in practice nothing linted the bench.Approach
varintbuckets a QUIC varint's encoded size: 1 byte below 64, 2 bytes below 16384, otherwise 4 bytes. The arms are now0..64and64..16384, which are disjoint and return the same sizes.sh/rs/select.sh'stestaction (whatjust checkand the land gate run) adds--benches --examplesto the one test build. clippy-driver is already the workspace wrapper for that compile, so their lints now fail the PR. nextest's existingdefault-filter = "not kind(bench)"still keeps the benches from running. The examples build as test-harness binaries with no tests.required-featuresare off, without a warning. A bench-only change selects its crate alone, so no dependent turns them on either. Thetestaction therefore reads every bench and examplerequired-featuresof the crates in the build fromcargo metadataand adds them to the existing capture/play/webrtc/io-uring feature picks, without duplicates. On a workspace run that addsmoq-quic/fuzz,moq-stats/bench,moq-net/{async,fuzz},moq-relay/test-support,moq-mux/fuzz,moq-rtmp/fuzz,moq-uring/quic,moq-audio/bus, and themoq-tokio/qmuxexample features. All of them are additive: no library has anot(feature = ...)arm on them, and the test count is unchanged. All of them build wherever the gate runs.moq-uringcompiles to nothing off Linux, andmoq-audio/capturewas already on. So no target is left out.just rs testkeeps--tests. Nightly and the docs call it for targeted--releasebenchmark runs, so adding targets there would build release benches and examples for nothing.just rs check-testis unchanged too: it forwards the same args tocargo doc, which has no--benches, and nothing in CI calls it.rs/justfile,sh/rs/select.sh, and.config/nextest.tomlnow describe the new target set.sh/rs/select.test.pychecks the workspace feature list and that a bench-only change inmoq-stats,moq-net,moq-relay, ormoq-quicturns on that bench's required feature.Cost
No new dependencies compile. criterion (through
bench-harness) is already a dev-dependency, so the tests build already compiled it. The extra work is the selected crates' bench and example binaries, plus whatever library code their required features add.Both crates were measured after touching
src/lib.rs, two runs each. The host had 24 cores at load ~25 from other agents, so the wall times are noisy; CPU (user+sys) is the steadier number.--tests(main)+ --benches --examples--benches --examples, default features: 3 benches+ --features moq-net/async moq-net/fuzz: all ~23 benchesAgainst main, moq-mux's edit loop costs about +20–45% CPU, including its ~18 s test run. The required features add another +11–26% CPU for moq-net. moq-net's test count stays at 3,435 either way.
Impact
required-features. Nightly no longer has to report it after the merge. Each gate run also compiles those binaries and their features (see Cost).Alternatives
--all-targetsin thejust rs testrecipe: fewer edited lines, but every targeted--releasebenchmark run (Nightly,bench/README.md) and the macOS test runs would also build every bench and example. It would still skip therequired-featurestargets.--all-featuresfor the selected crates: this would also turn on backend features no bench or example needs, such as GPU encoders,vaapi, andaws-lc-rsbesidering. Nightly'sfeaturesjob already compiles that set. In the gate it would add compile time and run tests under feature shapes the gate doesn't test today. Each target's ownrequired-featuresis the narrowest complete set.cargo clippy --benches --examplespass: check mode can't reuse the test build's artifacts, so it would re-check the whole dependency graph. That is the second compile the recipe comments avoid.Validation
just rs test -p moq-pattern --benches --examplesfails withmatch_overlapping_armonbenches/matching.rs. With the fix, it passes.cargo clippy --keep-going --workspace --all-targets(excluding the fuzz crates), with the gate's device features plus every target's required features, found nothing else.just check origin/mainon415e69bd01passed every selected module: rs, biome, markdown, gh, python, shell, and the text checks. Thers/justfilechange selects the whole workspace.test rsran 11,638 tests (132 skipped) and built every bench and example, includingproducer_tick, the sixmoq-quicbenches, andmoq-audio'sbus. It took 531 s wall with a warm cache at host load ~25.python3 sh/rs/select.test.py: 24 tests pass on1a135b069e.just gatewas run or claimed.Review
Codex,
gpt-6.1-sol, medium effort, localquest review:b45ec9d76183037d20499a553bebaaf0ab4140baagainstorigin/main(40c4eb0b428d5579007cae86dbc7c90aff6bccec): no findings.415e69bd0188e5b2d60206b08301960d39f39661againstorigin/main(b23b31e98edc1bdd3b511f4052cf0391022b25c2): no findings.Pullfrog's thread on
sh/rs/select.shwas fixed in415e69bd01: benches behindrequired-featureswere silently skipped. The thread is resolved.Pullfrog's thread on
sh/rs/select.test.pywas fixed in1a135b069e, a test-only change after round 2.test_a_bench_selects_only_its_cratehad lost its check that a moq-stats library change selects moq-relay. The new required-features test ran that check instead, using its loop's leftoverbenchvariable rather than the moq-stats bench. The two assertions are back in their own test, andselect.test.pypasses. The thread is resolved. No new review round was run.(Written by Claude Opus 5.5)
🤖 Generated with Claude Code