tinypilot
tinypilot copied to clipboard
Use shellcheck for bats tests without workaround
Unfortunately, the shellcheck+bats situation is a bit messy right now. Once they get things in order again, we can refactor our bats bash tests:
- Remove the workaround for the warnings about the
$output
/$status
/$lines
variables (see below) - Use the
@test
prefix notation instead of the#@test
comment notation.- I.e.,
@test test_some_command() {
instead oftest_some_command() { #@test
- I.e.,
Backstory / Workaround
shellcheck has built-in support for .bats
files, even if these files use the (otherwise non-standard) @test
prefix. However, there was a regression in shellcheck, where shellcheck now produces false positive info statements about potential modifications of the $output
/$status
/$lines
variables, which is confusing and annoying.
Our current workaround is to put the following block at the beginning of every .bats
file:
{
# Silence shellcheck for global bats variables.
# https://github.com/tiny-pilot/tinypilot/issues/1718
# shellcheck disable=SC2154
echo "${output}" "${status}" "${lines}" >/dev/null
}
This silences the SC2154
warnings that would otherwise appear.
Note that this workaround has to be wrapped in a custom “scope” (subshell), because of the shellcheck gotcha that directives immediately after the shebang apply to the entire file. The {...}
ensures that the directive only ever applies to the echo
statement, regardless of what else is between it and the shebang.