ci: run rustfmt, clippy and the test suite against rainix-static/ itself - #346
Conversation
Every job in the self-test matrix runs with `working-directory: test/fixture`, whose workspace has exactly one member (`crates/test-rs`), so `cargo test` and `rainix-rs-static` there resolve against the fixture — never against `rainix-static/`, the crate implementing release-guard, soldeer-gate, snapshots-append-only and rpc-preflight. Adds a named `rainix-static` job that runs `cargo test` (both platforms) and `rainix-rs-static` (ubuntu) with `working-directory: rainix-static`. The fixture matrix is untouched: it proves the tasks work for a downstream consumer, which is a different claim. The commands run in the dev shell, not as `nix build .#rainix-static`, because that derivation's checkPhase only runs on a cache miss. Closes #345 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 47 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe workflow adds dedicated Changesrainix-static CI coverage
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change adds CI coverage for the repository's own Rust tests, formatting, and clippy checks without changing production code or runtime behavior. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
…static-crate-coverage
Merging main brought #343's Rust under the new crate-scoped job, and it does not pass: 11 clippy errors and a dirty `cargo fmt`. Neither was ever visible, because nothing has run either tool against this crate. - 10 x `doc_overindented_list_items` in `release_guard.rs`'s module doc. `a.` and `b.` are not markdown list markers — markdown ordered lists are digits only — so the block is a continuation paragraph of item 4 and belongs at item 4's content indent, not a nested one. Flattened to that, with a blank `//!` between the two so they still render apart. The labels stay: line 371 refers to "4a." in a code comment. - `frozen_tags.iter().any(|t| *t == dir)` -> `frozen_tags.contains(&dir)`, from `cargo clippy --fix`. Same `PartialEq` comparison over the same `Vec<String>`; no behaviour change. - `cargo fmt` over the crate, which is what the job now enforces. Verified in the dev shell on this tree: `cargo fmt --all -- --check` clean, `cargo clippy --all-targets --all-features -- -D warnings -D clippy::all` zero errors, 174 tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD origin/master)"
git diff --stat "$base"...HEAD
git diff --numstat "$base"...HEAD
git diff -- "$base"...HEAD -- .github/workflows/test.yml flake.nixLength of output: 392 SIZE=S You are interacting with an AI system. |
#346 landed the crate-scoped rustfmt/clippy/test job after this branch was cut, so 833e8a7 would have shipped #343's checks and left #346's inert — a second bump behind the first. Merging main and repointing at its tip makes one bump cover both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
rainix-static/is the crate that implements every org-wide gate —release-guard,soldeer-gate,snapshots-append-only,rpc-preflight— and rainix's own CI could notsee it. Every job in the self-test matrix runs with
working-directory: test/fixture(
.github/workflows/test.yml:28), andtest/fixture/Cargo.tomlis a workspace whose onlymember is
crates/test-rs, so that matrix'scargo testand itsrainix-rs-static(
cargo fmt --all -- --checkpluscargo clippy --all-targets --all-features -- -D warnings -D clippy::all) resolve against the fixture and never againstrainix-static/.This adds a named
rainix-staticjob that runscargo test(ubuntu + macos) andrainix-rs-static(ubuntu, mirroring the matrix's own "no rust static analysis onmultiple platforms" rule) with
working-directory: rainix-static. The fixture matrix isuntouched — it proves the tasks work for a downstream consumer, which is a different
claim from covering rainix's own crate, and both claims are now made.
The commands run in the dev shell rather than as
nix build .#rainix-staticonpurpose, for the reason the next section measures: that derivation's tests are
cache-conditional, and
cargo fmt/cargo clippyare not in it at all.The open question, settled by measurement
They do — sometimes. It is decided by the Cachix cache, not by the code, and it is not
nix flake checkthat decides it. Three measurements, no reasoning from defaults:1.
nix flake checkevaluates the derivation but does not build it. From thecheck-shell run on #343's branch (run
32457734763):
No build follows in that step.
nix flake checkbuildschecks.*;packages.*it onlyevaluates. The build happens later, in a different step, because
rainix-staticis incommon-shell-inputs(flake.nix:367) and instantiating a dev shell realises it:2. On a Cachix hit nothing runs — same commit, same crate, opposite outcome per
runner. In that same run, the macOS job reached the same step and got the cache:
A substituted path runs no phases at all, so on that commit the crate's unit tests
executed on ubuntu and did not execute on macos. On unchanged
mainthe hit isguaranteed, not incidental:
nix eval --raw .#rainix-staticat74bfa3bgives/nix/store/b185bvvj9v0wnn7vfgcny8669gzgf6ki-rainix-static-0.1.0, andso every CI job on today's
mainsubstitutes that output and runs zero of its tests.3. When it is built, the tests genuinely run — and only the tests. Forcing a real
build of
main's derivation (nix build .#rainix-static -L --rebuild --option substituters "https://cache.nixos.org"):(that command exits 1 only on
--rebuild's reproducibility comparison —may not be deterministic: output ... differs— not on a test failure.)cargoCheckHookrunscargo testand nothing else:cargo fmtandcargo clippynever run in thisderivation under any cache state, which is why the answer to the open question does not
change the conclusion.
flake.nix's comment claiming the tests "run inside the nix buildvia doCheck" was true but load-bearing in a way it could not carry; it is corrected in
place to say what it actually guarantees and to point at the job that is the coverage.
Formatting:
mainis clean, #343 is not — stated, not fixed hereThe issue predicts a red first run on
cargo fmt --check. Measured rather than assumed:at
main(74bfa3b) the crate is already clean —so there is nothing to format on this branch, and the new job is green here. The diffs the
issue cites are real, but they live on #343's branch (
0b46830), which is not merged.Running the same check in a read-only worktree of
origin/2026-08-20-issue-341-release-determinismexits 1 with three hunks —frozen_snapshots.rs:111,frozen_snapshots.rs:142, and in production coderelease_guard.rs:251(aread_dir(...).unwrap_or_else(...)that rustfmt wants as ablock) — while that PR reports 14 green checks. That is the bug this job fixes, observed.
#343 is not touched by this PR, and there is a sequencing consequence to state plainly:
whichever of the two merges second produces a
maincommit carrying both this job and#343's unformatted code, so
maingoes red onrainix-static (ubuntu-latest, rainix-rs-static)untilcargo fmtis run over the crate. #343 should runcargo fmtin
rainix-static/before it merges. That is the check working, not a defect in it.QA
run 32458880233:
rainix-static (ubuntu-latest, cargo test)→Run nix develop .. --command cargo test…
running 159 tests…test result: ok. 159 passed; 0 failed;rainix-static (macos-latest, cargo test)→ the same159 passed; 0 failed;rainix-static (ubuntu-latest, rainix-rs-static)→++ cargo fmt --all -- --check,++ cargo clippy --all-targets --all-features -- -D warnings -D clippy::all,Checking rainix-static v0.1.0 (/home/runner/work/rainix/rainix/rainix-static). Thatlast path is the point: the lints are now resolving against this repo's own crate on a
real runner. Also run locally beforehand with the identical invocation from
rainix-static/afterrm -rf target, both exit 0.fix(tag-release): determinism check that never removes the frozen record #343's branch —
nix develop .. --command rainix-rs-staticfromrainix-static/exits 1there (
release_guard.rs:251, production code) and exits 0 fromtest/fixture/, which iswhat base CI runs. Verified by running both invocations in a read-only worktree of
origin/2026-08-20-issue-341-release-determinism(0b46830): base's fixture-scopedcommand passes the code the new crate-scoped command fails. On this branch, at
main74bfa3b, both are green, so the job is not merely red-by-construction.nix develop .#rust-shell --command pre-commit run --files flake.nix .github/workflows/test.yml): deadnix, nil, nixfmt, statix, yamlfmt, no-consumer-prettierall Passed; the rest skip with no matching files.
flake.nixcomment —no Rust logic changed (
git diff main --stattouches.github/workflows/test.ymlandflake.nixonly, zero files underrainix-static/src/), so there is nothing formutation-probeto mutate. The mutation-relevant effect of this PR is the oppositedirection: it is what makes
rainix-static/'s existing 159 tests, and its lint gates,actually run on every push.
implementation —
test.yml:28pinsworking-directory: test/fixture;test/fixture/Cargo.tomllistsmembers = ["crates/test-rs"];flake.nix:342-348defines
rainix-rs-staticas fmt + clippy over--all;flake.nix:367putsrainix-staticincommon-shell-inputs. Cargo resolves--allagainst the workspacerooted at the working directory, so no invocation from
test/fixturecan reachrainix-static/.rainix-rs-staticand thetest suite against
rainix-static/itself, alongside the fixture matrix rather thanreplacing it; (B) determine and state whether the crate's unit tests execute in CI today;
(C) expect a red first run on formatting and fix it by formatting the crate, never by
loosening the check. Covered A and B. C is n/a with a reason, measured: at
main74bfa3bthe crate is already fmt-clean and clippy-clean (exit 0, shown above), so thereis nothing here to format and nothing was loosened — the diffs the issue saw are on
fix(tag-release): determinism check that never removes the frozen record #343's unmerged branch, are reproduced above, and are flagged for fix(tag-release): determinism check that never removes the frozen record #343 to format rather
than silenced.
Closes #345
🤖 Generated with Claude Code