Repository navigation
fix(qa): enforce semver gate post-1.0 + count unwraps after inline cfg(test) (#2002) - #2012
Conversation
…g(test) (#2002) Two release-QA gates under-checked. Closes #2002. semver.sh: unconditionally `exit 0` even on breaking changes, despite the header promising a post-1.0 hard gate. The workspace is now 1.0.0-rc1, so the gate never engaged. Read the workspace version and hard-fail (exit 1) on breakage once it's >= 1.0; keep the soft warning for 0.x (and if the version can't be read). unwrap-budget.sh: Rule 3 truncated each file at the FIRST `#[cfg(test)]` line to EOF, so `.unwrap()`/`.expect(` in production code AFTER an interspersed test block were silently excluded from the ratchet (e.g. `calculate_char_sum` after a `#[cfg(test)]` helper in binaries/cli/src/output.rs). Now skip each `#[cfg(test)]` block by dedent (the block-closing `}` back at the attribute's indent) and count the rest. Dedent, not brace-counting, because `{`/`}` inside string literals and `format!("{}")` macros in test code make raw brace balance unreliable. All cfg(test) items in the counted source are brace blocks (mod/fn/impl); brace-less items do not occur. Verified: the count is unchanged (163) on current code — the previously-hidden regions happen to have no unwraps today — but new production unwraps in those regions will now be caught. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Review follow-up. The dedent-based block skip assumed the closing `}` sits on
its own dedented line, but a self-contained one-line item
(`#[cfg(test)] fn helper() {}`, which rustfmt preserves) opens and closes on
the same line. The old branch set `in_test` and then skipped every production
line until the next column-aligned `}`, undercounting unwrap/expect in the
first production item after such a helper -> a false-negative for the budget.
Distinguish a multi-line opener (line ends with `{`) from a one-line item: only
enter block mode for the former; a one-line `{...}` body or a `;`-terminated
decl (`mod sub;`) is skipped on its single line, so following production code is
still counted. Verified with a fixture covering all patterns (one-line attr+item,
one-line item on next line, decl, multi-line block) and unchanged total (163) on
current source.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good catch — fixed in You're right: the dedent-based skip assumed the block-closing Fix: distinguish a multi-line opener (line ends with Verified with a fixture covering every pattern: #[cfg(test)]
fn helper() {} // one-line, attr on prev line
fn prod_b() { y.unwrap(); } // <- now counted (was hidden)
#[cfg(test)] fn helper2() {} // one-line, attr+item same line
fn prod_c() { z.expect(\"e\"); } // <- now countedThe fixture's 4 production unwraps are counted and the 3 test unwraps excluded (awk -> 4). Full count on current source is unchanged at 163 (no current one-line cfg(test) items exist, so this is latent-defeat-only, same spirit as the original issue). |
Summary
Closes #2002. Two release-QA gates report "green" while under-checking.
1.
scripts/qa/semver.sh— never failed on breaking changes (post-1.0)The script's header promised a post-1.0 hard gate, but the code unconditionally
exit 0'd on breakage. The workspace is now1.0.0-rc1, so the documented gate never engaged —make qa-deep/qa-release-gatewould print "found breaking changes" and pass.Fix: read the workspace version from
Cargo.toml([workspace.package] version) and hard-fail (exit 1) on breakage when it's ≥ 1.0; keep the soft warning for0.x(and as a safe fallback if the version can't be read).(
cargo-semver-checkscompares against a git-tag baseline and is version-aware, so the intentional 0.x→1.0 major bump is not flagged — the hard gate only trips on changes that violate the current version's contract.)2.
scripts/qa/unwrap-budget.sh— Rule 3 hid production code after inline#[cfg(test)]The awk truncated each file at the first
#[cfg(test)]line to EOF, so.unwrap()/.expect(in production code after an interspersed test block were excluded from the ratchet. Example:binaries/cli/src/output.rshas a#[cfg(test)]helper fn, then productioncalculate_char_sum, then the realmod tests. Any unwrap added to that middle region was silently uncounted — a latent ratchet defeat.Fix: skip each
#[cfg(test)]block by dedent (the block-closing}back at the attribute's indentation) and count the rest, instead of truncating to EOF.{/}inside string literals andformat!("{}")macros that pepper test code make brace balance unreliable (my first attempt over-counted test unwraps for exactly this reason). The block-closing brace is the only}back at the attribute's indent.#[cfg(test)]items in the counted source are brace blocks (mod/fn/impl, verified across 73 occurrences); brace-less attributed items don't occur.Verified: count is unchanged at 163 on current
main(old-awk and new-awk agree, no per-file diffs) — the previously-hidden regions happen to have zero unwraps today — but new production unwraps in those regions will now be caught. The budget (185) is untouched; the gate passes.bash -nclean on both.🤖 Generated with Claude Code