tests: guard CacheStatus as_str/serde encodings against drift - #58
tests: guard CacheStatus as_str/serde encodings against drift#58cargo-affected-bot wants to merge 3 commits into
Conversation
CacheStatus has two independent kebab-case encodings — the manual as_str() (used by the Display impl / stderr summary line) and the serde rename_all = "kebab-case" derive (used for JSON). They must agree, but nothing enforced it: the only test covered a single variant, and the as_str doc comment claimed it was the encoding "used by the JSON serializer" (it isn't — serde derives its own) and "one canonical mapping" (there are two). Add cache_status_as_str_matches_serde, which asserts as_str(), Display, and the serde output agree for every variant, so renaming one encoding without the other fails loudly per the project's fail-loudly principle. Correct the as_str doc comment to describe the relationship accurately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…coding-guard # Conflicts: # src/report.rs
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cargo-affected-bot
left a comment
There was a problem hiding this comment.
Self-review — the guard is correct and the doc-comment fix is accurate (Display/as_str for the stderr line, serde derive for JSON, verified against summary_line's cache={status}).
One observation inline: the test's variant list is itself a third hand-maintained copy that can drift silently — the exact failure class this PR is guarding against, one level up.
| CacheStatus::MissFingerprint, | ||
| CacheStatus::MissNoCoverage, | ||
| CacheStatus::MissNoReachableSha, | ||
| CacheStatus::ForcedAll, |
There was a problem hiding this comment.
This array is a third hand-maintained enumeration of the variants. as_str's match is exhaustive, so the compiler forces a new variant to be added there — but nothing forces it into this array, so a newly-added variant would silently escape the guard. That's the same silent-drift failure mode the PR is closing, just one level up.
A dependency-free way to make it fail loudly: drop an exhaustive match next to the array so adding a variant breaks compilation until it's listed here too, e.g.
// Compile-time reminder: a new variant must be added to the array above.
fn _exhaustive(s: CacheStatus) {
match s {
CacheStatus::HitExact
| CacheStatus::HitWithDivergence
| CacheStatus::MissFingerprint
| CacheStatus::MissNoCoverage
| CacheStatus::MissNoReachableSha
| CacheStatus::ForcedAll => {}
}
}Not blocking — the current test fully covers the rename-one-encoding case it targets.
Found during the nightly rolling survey of
src/report.rs.CacheStatuscarries two independent kebab-case encodings that must agree:as_str()— the manual mapping, reached via theDisplayimpl for the stderr summary line (cargo-affected: cache=<status> …).#[serde(rename_all = "kebab-case")]derive — used for the JSON report.Nothing enforced that they stay in sync. The only existing test (
cache_status_serializes_kebab_case) checked a single variant, and theas_strdoc comment was inaccurate — it claimed the string was "used by the JSON serializer" (the serializer uses serde's own derive, notas_str) and that there was "one canonical mapping" (there are two). A future rename of one encoding without the other would drift silently — exactly the failure mode the project's "fail loudly" principle is meant to prevent.This PR adds
cache_status_as_str_matches_serde, which assertsas_str(),Display, and the serde output agree for every variant, and corrects the doc comment to describe the relationship accurately.No behavior change — this is a test + doc-comment change guarding an existing invariant.
cargo clippy --all-targetsand thereport::test module are clean locally.