ci: add an all-features build job and fix the --nocapture flag - #894
Conversation
Add a `test-all-features` job to the Rust CI workflow that builds and lints the workspace with `--all-features --all-targets`, so feature-gated code (tracing, metrics, debug-* etc.) is compiled and clippy-checked in CI instead of being allowed to rot. It runs `cargo build` and `cargo clippy -D warnings`; it intentionally skips `cargo test` because `--all-features` enables the `danger-skip-*` features that disable security verification. Three target roots overflowed the default layout-query recursion limit under `--all-features` (the feature-unified futures get larger); bump them to 512 to match the limit already set in `src/lib.rs` / `examples/demo.rs`. Also fix the silently-ignored libtest flag: the correct spelling is one word, `--nocapture`, not `--no-capture`, in both `main.yml` and `e2e.yml`.
📝 WalkthroughSummary by CodeRabbit
WalkthroughTwo GitHub Actions workflows have ChangesCI and Compiler Attribute Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/main.yml:
- Line 79: The actions/checkout action is currently configured to persist Git
credentials in the local git config, which poses a security risk since this CI
job doesn't require Git write access after checkout. Add the
`persist-credentials: false` parameter to the actions/checkout@v6 step to
disable credential persistence and reduce the attack surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bdfd57db-ce82-4032-b578-0cf44c570c0c
📒 Files selected for processing (5)
.github/workflows/e2e.yml.github/workflows/main.ymlexamples/benchmark.rstests/bench-integration/src/main.rstests/e2e/src/lib.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e9ee32fcd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
📦 Binary size report
.text per crate
Baseline: |
There was a problem hiding this comment.
1 issue found across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
What
Two CI changes, plus the minimal code fixes the first one surfaced.
test-all-featuresjob in.github/workflows/main.yml("Build & Lint (all features)"). It mirrors the existingtestjob's setup (same pinnednightly-2026-06-16, protoc, sccache, rust-cache) and runs:cargo build --workspace --exclude e2e-tests --all-features --all-targets --verbosecargo clippy --workspace --exclude e2e-tests --all-features --all-targets -- -D warnings--nocaptureflag fix inmain.ymlande2e.yml: the libtest flag is one word (--nocapture), so the previous--no-capturewas silently ignored.#[recursion_limit = "512"]added to three target roots (examples/benchmark.rs,tests/bench-integration/src/main.rs,tests/e2e/src/lib.rs). Under--all-features, feature unification enlarges those crates' async futures past the default layout-query depth, so the build failed without this. 512 matches the limit already set insrc/lib.rs/examples/demo.rsfor the same reason.Why
Feature-gated code (
tracing,metrics,debug-diagnostics,debug-snapshots,tracing-pii, ...) was never built in CI on the full feature set, so it could rot unnoticed. Building + clippy-checking with--all-features --all-targetscloses that gap.The job intentionally does not run
cargo test --all-features:--all-featuresenablesdanger-skip-cert-chain-verify/danger-skip-tls-verify, which disable security verification (the cert-chain negative test is even#[cfg]'d out under it). The suite would therefore change behavior rather than catch rot, so build + clippy already cover the "doesn't rot" goal. A comment in the workflow notes this.Benchmark workflows (
bench-integration.yml,codspeed.yml,binary-size.yml) stay non-gating and are untouched.Verification
On the pinned
nightly-2026-06-16, all run clean locally:cargo build --workspace --exclude e2e-tests --all-features --all-targetspasses.cargo clippy --workspace --exclude e2e-tests --all-features --all-targets -- -D warningsis clean.cargo fmt --all -- --checkis clean.