fix(util)!: send diagnostics to stderr and exit non-zero on failed repositories - #60
Open
bircni wants to merge 4 commits into
Open
fix(util)!: send diagnostics to stderr and exit non-zero on failed repositories#60bircni wants to merge 4 commits into
bircni wants to merge 4 commits into
Conversation
The logger ran in `TerminalMode::Mixed`, which puts everything below
`Warn` on stdout. In a debug build that meant `--json` emitted debug
records ahead of the opening brace:
21:06:24 [DEBUG] git_statuses::scan: Failed to open repository at ...
{"failed":["broken"],"repositories":[...
so nothing downstream could parse it. A release build only escaped this by
accident: the single `log::info!` sits in `repositories_table`, which the
JSON path never reaches. Any new info-level record anywhere would have
broken every consumer silently.
Diagnostics now go to stderr and stdout carries the report alone.
These tests live in a new `tests/` target because the invariant only
exists at the process boundary. Driving `run` in-process cannot catch it:
the printer writes to the buffer the test owns while the logger writes to
the real stdout, so an in-process test passes either way.
BREAKING CHANGE: anything capturing diagnostics from stdout must read
stderr instead. Output that was already being parsed is unaffected, and
is now reliable rather than accidentally correct.
The process always exited 0, even when every repository in the scan failed to open. A script piping the output had no way to tell a clean scan from a total failure - the only signal was a warning on stderr. `run` now returns an `Outcome` describing what it reported, and `main` turns a non-empty failed list into exit code 1. The failures were already being counted and printed; this just stops throwing the fact away. BREAKING CHANGE: a scan that finds a repository it cannot process now exits 1 instead of 0. Scanning a tree that contains one broken checkout will start reporting failure to any script that checks the exit status.
…rives `fetch_origin` fetches whatever remote `get_remote_name` returns, which is "origin" only when one exists - the name promised something the function does not do. It is now `fetch_remote`, with a note on why it shells out to `git` rather than using `git2`: the user's credential helpers, SSH agent and proxy settings apply as they do on the command line, and matching that through `git2` would mean reimplementing authentication. `RepoInfo` and `Status` derived `Deserialize`, but nothing ever reads them back - the JSON output is write-only.
The gate measures total line coverage, so deleting well-covered code lowers it even when nothing became less tested. That nearly blocked #56, where removing dead code dropped the ratio despite the change adding assertions - and the incentive it creates is to keep dead code or write filler tests. Coverage sits at 96% today; the lower bound is headroom for refactors that remove tested code, not a target.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Warning
Two breaking changes: diagnostics move from stdout to stderr, and a scan that finds
an unreadable repository now exits
1. See Compatibility.Four commits, each independently buildable.
1.
--jsonwas not reliably machine-parseableThe logger ran in
TerminalMode::Mixed, which puts everything belowWarnon stdout:A release build only escaped this by accident — the single
log::info!sits inrepositories_table, which the JSON path never reaches. Any new info-level recordanywhere would have broken every consumer silently.
Diagnostics now go to stderr; stdout carries the report alone.
2. The process always exited 0
Even when every repository failed to open. A script had no way to distinguish a clean
scan from a total failure.
runnow returns anOutcome, and a non-empty failed listbecomes exit code 1. The failures were already counted and printed — this stops
discarding the fact.
3 & 4. Cleanups
fetch_origin→fetch_remote: it fetches whateverget_remote_namereturns, whichis
originonly when one exists. Documented why it shells out togit(credentialhelpers, SSH agent, proxy config — matching that via
git2means reimplementing auth).DeserializeonRepoInfoandStatus; the JSON output is write-only.code lowers it even when nothing became less tested — that nearly blocked refactor: remove redundant per-repository work and make output testable #56. Actual
coverage is 96.88%; the lower bound is headroom, not a target.
On the tests
The new
tests/target exists because these invariants only hold at the processboundary. My first attempt asserted them in-process against
run— and it passed withthe bug still present, because the printer writes to the test's buffer while the logger
writes to the real stdout. Each of the four new tests was confirmed to fail with its fix
reverted.
Compatibility
being parsed is unaffected — and now reliable rather than accidentally correct.
exit status will start seeing failure.
README documents both, including the now-working
git-statuses --json | jq …pipeline.