Repository navigation
Conversation
In bats, $status, $output and $lines (and, with --separate-stderr, $stderr and $stderr_lines) are set by the run helper. ShellCheck, however, only knows that a @test somehow sets them, which does not help a helper function in a separate (non-.bats) file which calls run and checks its results, so it gets SC2154. Also, $stderr and $stderr_lines are treated as always set, even in non-bats scripts. Teach ShellCheck that run sets these variables, and stop treating $stderr and $stderr_lines as always set. Variables set by run are not reported as unused (SC2034), as checking them is up to the caller. A @test still counts as setting these variables (for SC2154), since tests commonly use wrappers around run (like podman's run_podman), defined in files loaded via bats' load, which ShellCheck does not follow. These are not reported as unused either. Remove the pretend references to these variables from a @test. They were only needed to avoid SC2034 for the above, and they were hiding SC2034 for manual assignments like `output=$(cmd)` or `local status` which are not used, and caused SC2031 to be reported on a @test line. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Contributor
Author
This branch has not been deployed
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.
In bats,
$status,$outputand$lines(and, with--separate-stderr,$stderrand$stderr_lines) are set by therunhelper. ShellCheck,however, only knows that a
@testsomehow sets them. This does not helpa helper function in a separate (non-
.bats) file which callsrunandchecks the results, so it gets SC2154. On the other hand,
$stderrand$stderr_linesare treated as always set, even in non-bats scripts.This PR:
runsets these variables (stderrandstderr_linesonly with--separate-stderr);$stderrand$stderr_linesas always set;run(or by a@test, see below)as unused (SC2034), as checking them is up to the caller;
@test.They were only needed to avoid SC2034 for the above, but they also
hid SC2034 for manual assignments which are never used (like
output=$(cmd)instead ofrun cmd, or a leftoverlocal status),and caused SC2031 to be reported on a
@testline.A
@teststill counts as setting these variables for SC2154. I triedremoving that, but on ~1000
.batsfiles from various projects(podman, buildah, cri-o, bats-core, etc.) it resulted in ~730 new
SC2154 warnings, as tests commonly use wrappers around
run(likerun_podman) defined in files loaded via bats'load, which ShellCheckdoes not follow.
Testing
Compared with the current master on the same ~1000
.batsfiles:were all false positives, e.g. reading
$outputafter a freshrun);( cd dir && run git reset ... ); [ "$status" -eq 0 ];local statuswherestatusis never read, and a batstest fixture which deliberately sets
statusinteardown).In runc integration tests, this makes two
# shellcheck disable=SC2154annotations (in a helper calling
run) unnecessary. It also found a fewpointless assignments (
output=$(cmd)instead ofrun cmd, and unusedlocal status), which are being fixed on the runc side.Note this touches the same test area as #3550, so there will be a
trivial conflict (adjacent
prop_lines) when merging both.Refs #356