Skip to content

bats: treat run as setting status, output and lines - #3552

Open
kolyshkin wants to merge 1 commit into
koalaman:masterfrom
kolyshkin:bats-run
Open

kolyshkin wants to merge 1 commit into
koalaman:masterfrom
kolyshkin:bats-run

Conversation

@kolyshkin

@kolyshkin kolyshkin commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

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. This does not help
a helper function in a separate (non-.bats) file which calls run and
checks the results, so it gets SC2154. On the other hand, $stderr and
$stderr_lines are treated as always set, even in non-bats scripts.

This PR:

  • teaches ShellCheck that run sets these variables (stderr and
    stderr_lines only with --separate-stderr);
  • stops treating $stderr and $stderr_lines as always set;
  • does not report variables set by run (or by a @test, see below)
    as unused (SC2034), as checking them is up to the caller;
  • removes the pretend references to these variables from a @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 of run cmd, or a leftover local status),
    and caused SC2031 to be reported on a @test line.

A @test still counts as setting these variables for SC2154. I tried
removing that, but on ~1000 .bats files from various projects
(podman, buildah, cri-o, bats-core, etc.) it resulted in ~730 new
SC2154 warnings, as tests commonly use wrappers around run (like
run_podman) defined in files loaded via bats' load, which ShellCheck
does not follow.

Testing

Compared with the current master on the same ~1000 .bats files:

  • 198 SC2030/SC2031 and 2 SC2154 warnings are gone (the ones I checked
    were all false positives, e.g. reading $output after a fresh run);
  • 1 new SC2030/SC2031 pair, which is a real bug:
    ( cd dir && run git reset ... ); [ "$status" -eq 0 ];
  • 2 new SC2034 (local status where status is never read, and a bats
    test fixture which deliberately sets status in teardown).

In runc integration tests, this makes two # shellcheck disable=SC2154
annotations (in a helper calling run) unnecessary. It also found a few
pointless assignments (output=$(cmd) instead of run cmd, and unused
local 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

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>
@kolyshkin

Copy link
Copy Markdown
Contributor Author

@e-kwsm @koalaman PTAL; this improves checking bats files.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant