ci: run shellcheck over every tracked .sh, ratcheted against a recorded baseline - #1075
Open
fujibee wants to merge 2 commits into
Open
ci: run shellcheck over every tracked .sh, ratcheted against a recorded baseline#1075fujibee wants to merge 2 commits into
fujibee wants to merge 2 commits into
Conversation
…ed baseline No linter ran over the 97 tracked shell scripts in CI, so every "shellcheck clean" was a claim about an unknown local setup -- once made on a machine with no shellcheck at all, where no output read as no findings (#1068). The checker counts every finding, all severities, over `git ls-files '*.sh'` and compares with .github/shellcheck-baseline, which records the shellcheck version the count was measured with and the count: 0.10.0 and 159 on main (error 7, warning 39, note 113; -x changes nothing because the sourced paths are variables). Above the baseline is exit 1 with the findings listed; below it is exit 1 asking for the baseline to be lowered; at it is exit 0. It is exit 2, never 0, when shellcheck is missing, when the version that ran is not the one the baseline names (counts from different versions are not comparable), when no tracked .sh exists, or when the baseline cannot be read. The version and path of the tool that ran are the first line of every run. The workflow pins shellcheck 0.10.0 by version and by the tarball's sha256, and runs the checker's --positive-control before the verdict: a throwaway repository with one broken script and a zero baseline must produce exit 1 from the checker itself, with the same binary. A job that has never gone red is indistinguishable from one that never ran. The checker's own fixture writes a literal `$` and carries an inline disable with its reason, which is the policy for a rule broken on purpose; without it, tracking the script would have moved the count to 160. tests/test_check_shellcheck.bats pins the ratchet in all three directions, the version attribution, each exit-2 case, the positive control, the baseline file's shape, and the workflow's pin, digest and step order; the tests skip visibly where no binary is available.
…list of severity words Review asked whether the finding regex -- error|warning|note|style -- had dropped `info`, and whether the baseline of 159 was therefore low. Measured with shellcheck 0.10.0 on main: the gcc text formatter prints exactly three level words (error 7, warning 39, note 113 -- it spells both info and style as `note`), so the regex matched all 159 lines, and `-f json1` reports the same 159 as error 7 / warning 39 / info 100 / style 13. The count was right and the baseline is unchanged; the way it was taken was a proxy, and a proxy can miss a level a formatter spells differently. The count is now the number of comments in `-f json1`, and the listing on a red is rendered from the same structure with the real level names, so an info finding is visible as `info`. The positive control requires three things from the checker: exit 1, the finding at its position, and its level spelled `info` -- the level a hand-listed set loses first. A new control runs a tree with one finding of each level and requires the checker's count to equal shellcheck's own json count for the same files. Calibrated: ignoring findings, or dropping the info level from the count, each turn the positive control into exit 2 and redden the tests. Also found by the tool itself: a comment line beginning with its name followed by a word is parsed as a directive, and a malformed one is a finding. Reworded.
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
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.
Closes #1068.
No linter ran over the 97 tracked
.shfiles in CI, so every "shellcheckclean" was a claim about an unknown local setup -- once made on a machine with
no shellcheck at all, where "no output" read as "0 findings". This runs it in
one known place, ratcheted against a recorded baseline, with the tool pinned
and its version printed by the check itself.
What lands
.github/scripts/check-shellcheck.sh-- runs shellcheck over every tracked.sh, counts the findings from its structured output (-f json1, onecomment per finding, every level: error, warning, info, style -- gating at
errorwould hide the levels where the real bash bugs live) and compareswith
.github/shellcheck-baseline. The listing shown on a red is renderedfrom the same structure with the real level names. Above the baseline: exit 1, listing the
findings. Below it: exit 1, asking for the baseline to be lowered. At it:
exit 0. Same shape as
check-enforced-assertions..github/shellcheck-baseline-- two lines: the shellcheck version the countwas measured with, then the count. Today:
0.10.0/159..github/workflows/shellcheck.yml-- on push and pull_request for main andintegration/**, like verify-versions. Fetches shellcheck v0.10.0 by URLderived from the pinned version, verifies the tarball's sha256, runs the
positive control, then the check.
tests/test_check_shellcheck.bats-- 14 controls (below).Measured
The baseline is that number, recorded, not inferred at run time. Lowering it
is the burn-down: separate PRs per area, each lowering line 2. A rule broken
on purpose gets an inline
# shellcheck disable=SCxxxxwith a reason (thechecker itself carries one, for the literal
$in its own fixture).What it refuses to be green about
Exit 2, never 0: shellcheck is missing or fails for a reason other than
findings; the shellcheck that ran is not the version the baseline names
(counts from different versions are not comparable -- bumping the version
means re-measuring in the same change); no tracked
.shfiles (an emptyscan); an unreadable baseline. The first line of every run is the version
that ran and the path it ran from.
The job goes red on purpose first
--positive-controlbuilds a throwaway repository with one broken script anda zero baseline and requires three things from the checker itself: exit 1,
the finding named at its position, and its level spelled
info-- the levela hand-listed severity set loses first. Ignoring findings, or dropping the
info level from the count, each turn the control into exit 2 (measured). The workflow runs it before the verdict, with the same binary. A job
that has never gone red is indistinguishable from one that never ran.
Controls (tests/test_check_shellcheck.bats)
is green and says how many files and which version; below it is red and
asks for the baseline to be lowered
shellcheck's own json count for the same files, and all four level names
appear in the listing
the right version is green (differential pair)
.shis exit 2;an untracked
.shis not scanned; a missing or malformed baseline is exit 2--positive-controlexits 0 and says it fired; with no binary it exits 2the version the baseline names, verifies a digest, and runs the control
before the verdict
with the pinned version; skips visibly otherwise)
The tests need a binary. Without one they skip -- as
ok # skip, visible inthe TAP -- and the workflow job, which pins its own binary, is the check that
matters. Calibrated: making the checker ignore findings turns the positive
control into exit 2 and reddens its tests; making it ignore the version
mismatch reddens the version test.
Out of scope
25 tracked files have a bash shebang and no
.shextension; the issue scopesthis to
.sh, and they are not counted. Lowering the baseline is follow-upwork, per area.
Also learned on the way: a comment line that begins with the tool's name
followed by a word is parsed as a directive, and a malformed directive is
itself a finding -- the checker flagged its own comment. Reworded.
Local: bats 14/14 with shellcheck 0.10.0 (release binary, not installed
system-wide) and 14 with 11 visible skips without one; the checker run under
/bin/bash 3.2;
check-enforced-assertionsat its baseline.