test: executed coverage for Justfile shell-sources and lint recipes - #344
kubestellar-hive[bot] wants to merge 3 commits into
Conversation
hanthor
left a comment
There was a problem hiding this comment.
One of the 15 new tests fails, and it fails for a reason that will not go away on a re-run
BATS unit tests on this PR is red (run 34380001749), and I reproduced it locally against origin/main + this branch:
not ok 157 lint aborts with a diagnostic when shellcheck is not installed
# (in test file tests/unit/justfile-shell-sources_test.bats, line 281)
# `[[ "$output" == *"shellcheck could not be found"* ]]' failed
Root cause
@test "lint aborts with a diagnostic when shellcheck is not installed" {
...
rm -f "${STUB_BIN}/shellcheck" # <-- only removes the stubsetup() prepends STUB_BIN to PATH; it never removes the rest of it. Deleting the stub just un-shadows the real shellcheck, which is preinstalled on ubuntu-latest (and present on my machine at /usr/local/bin/shellcheck, ShellCheck 0.11.0). So lint finds shellcheck, runs it on the empty sandbox a.sh, and exits 1 with an SC2148 report instead of the guard message.
Direct reproduction, no bats involved:
$ printf 'a.sh\n' > .shellcheck-scope && : > a.sh && just lint
Shellchecking 1 scripts:
a.sh
In a.sh line 1:
^-- SC2148 (error): Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
error: recipe `lint` failed with exit code 1
$ echo $?
1
Note that [ "$status" -ne 0 ] on line 280 passes — but for entirely the wrong reason. The assertion that actually distinguishes the guard from a shellcheck finding is the one that fails. This is the failure mode the rest of this PR is explicitly written to prevent, so it is worth fixing properly rather than loosening.
Fix: run the recipe with a PATH that contains only the stub dir, e.g.
run bash -c "cd '${SANDBOX}' && PATH='${STUB_BIN}' just lint 2>'${err_log}'"(or PATH=/nonexistent), so removing the stub genuinely removes shellcheck. The other 14 cases pass — ok 143 through ok 156 — so this is a one-test fix.
Second point: validate is also red
Job 102562218778 concluded failure. Worth confirming that is not a second, independent problem before this lands.
Baseline, for the record
Unmodified origin/main @ 1db684b is fully green — bats tests/unit = 180/180 ok, just lint exit 0, just check exit 0. Merging this branch into main gives 194 ok / 1 not ok. The failure is introduced by this PR; there is no pre-existing failure to attribute it to.
Non-blocking: the two BUG: tests
BUG: shell-sources exits non-zero when a glob matches a non-regular file and BUG: lint masks the shell-sources failure instead of failing closed pin current incorrect behaviour as the expected result. That is a legitimate characterization-test pattern and the comments are clear about it, but it does mean a future fix for #343 lands as a red test suite rather than a green one. Please make sure #343 references these two test names so whoever fixes it knows to invert them.
Generated by Claude Code
tests/unit/shellcheck-scope_test.bats asserts on the contents of .shellcheck-scope but re-implements the glob expansion inline, so the private shell-sources recipe and its only consumer, lint, have never been executed by a test. Adds tests/unit/justfile-shell-sources_test.bats: 15 BATS cases run against a sandbox copy of the Justfile with a synthetic .shellcheck-scope and a shellcheck stub on PATH, covering comment stripping, whitespace trimming, globstar/nullglob expansion, no-trailing-newline input, the missing-manifest guard, lint's argv, its empty-scope and missing-shellcheck guards, and shellcheck failure propagation. Two cases are prefixed BUG: and pin current behaviour for #343 — shell-sources exits 1 when the final glob match is not a regular file, and lint masks that status through mapfile instead of failing closed. Refs #343 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
fe57d51 to
787cda6
Compare
The 'lint aborts with a diagnostic when shellcheck is not installed' case removed the shellcheck stub from STUB_BIN, but CI runners ship a real shellcheck later on PATH. The lint recipe's 'command -v shellcheck' guard therefore succeeded, shellcheck ran against the empty sandbox script, and the recipe failed with SC2148 instead of the expected diagnostic. The test passed locally only because no shellcheck was installed. Add minimal_path_without, which builds a bin directory holding symlinks to just the interpreters the recipes need (bash, just), and run that one case with PATH restricted to it. The probed tool is then genuinely absent in both environments. Verified with shellcheck 0.11.0 on PATH: the case fails before this change and passes after; full suite 211/211. Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
The 'trims leading and trailing whitespace' case wrote its manifest from a heredoc containing a literal trailing tab. pre-commit's trailing-whitespace hook rewrites that line, which both fails the validate gate and deletes the only trailing whitespace the test exists to assert is trimmed. Emit the fixture with printf ' a.sh\t\n' instead, so the tab is produced at run time and no trailing whitespace is stored in the file. Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
Test Improvement
Adds
tests/unit/justfile-shell-sources_test.bats— 15 BATS cases giving the rootJustfilerecipesshell-sources(private) andlinttheir first executed coverage. No production files are touched; this PR adds one test file only.Cluster claimed:
tests/unit/justfile-shell-sources_test.bats(new file), exercisingJustfilerecipesshell-sourcesandlintand the.shellcheck-scopemanifest parser. Disjoint fromtests/unit/shellcheck-scope_test.bats, which asserts on manifest content and consumer wiring without ever running the recipes.Why
.shellcheck-scopeis the single source of truth for lint scope (#324). The existing drift gate re-implements the glob expansion inline, so the recipe's own parser has never been executed by a test. Covered here:#), whitespace trimming, blank linesglobstar/nullglobexpansion, non-matching patterns dropped, duplicates preserved.shellcheck-scope→ diagnostic on stderr, non-zero exitlintargv passed toshellcheck, empty-scope guard, missing-shellcheckguard,shellcheckfailure propagationRecipes run against a sandbox copy of the
Justfilewith a synthetic manifest and ashellcheckstub on PATH, so the real tree is never read or linted.Pinned defects
Two cases are prefixed
BUG:and pin today's behaviour for #343:shell-sourcesexits 1 when the last glob match is not a regular file (e.g. a directory named*.sh), even though the printed list is complete —[[ -f "$f" ]] && printf ...leaks its test result into the loop status underset -euo pipefail.lintmasks that status becausemapfile -t sources < <(just shell-sources)discards it — which would equally mask a genuinely truncated source list.A fix to either will fail these tests loudly, by design. The fix itself is production code and deliberately left out of this PR.
Verification
bats tests/unit— 195 passing (180 before, 15 new), bats 1.14.0 + just 1.58.0.Related Issue
Refs #343 — the coverage half. #343 stays open for the recipe fix (
continue/|| trueinshell-sources, fail-closedlint), which quality does not write.Filed by quality agent (hold-gated mode). Human review required.
— hive: agent=quality backend=copilot model=claude-opus-5