Skip to content

install-whitaker's test harness runs fragments with bash -c, which is not how a runner runs them #449

Closed

Description

@leynos

Problem

install-whitaker's test harness does not invoke a composite fragment the way a
runner does, and the difference is observable on the macOS runner image.

A runner writes a composite run: fragment to a file and executes that file.
.github/actions/install-whitaker/tests/_fragment_runner.py:210 passes the same
text to bash -c instead:

    process = subprocess.run(  # noqa: S603,TID251 - exercise the Bash fragment.
        [bash_executable(), "-c", script],

Those are not equivalent. Bash 3.2, which macOS runners ship, replaces itself
with the last command of a bash -c string when that command is external,
discarding any ERR trap along with the shell. Bash 5 suppresses that
optimization when a trap is set.

Evidence

Six probes on macos-15 and ubuntu-latest, differing only in what fails,
where, and how the script is invoked. The macOS shell is
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin24) at /bin/bash;
the Linux shell is 5.2.21(1)-release.

Case Bash 3.2 (macOS) Bash 5.2 (Linux)
file, builtin last fired fired
file, external last fired fired
file, external then no-op fired fired
bash -c, builtin last fired fired
bash -c, external last not run fired
bash -c, external then no-op fired fired

So the only affected shape is a bash -c string whose last command is
external, on Bash 3.2. A file is never affected, which is why a runner is never
affected.

Impact today: latent, not live

install-whitaker reports seven failure metrics from ERR traps, and its tests
assert several of them (test_install_whitaker_install.py,
test_install_whitaker_windows_paths.py). Those assertions are sound at
present: no install-whitaker fragment ends in a bare external command. Their
last commands are builtins, function calls, or redirections, none of which the
optimization applies to.

The gap is that nothing keeps it that way. A fragment that grows an external
command at its end would, on the macOS leg of python-tests, report a failure
with no ::error annotation and no metric, and the test asserting that metric
would fail for a reason that has nothing to do with the action. That is not
hypothetical: it is exactly what happened while #443 was being written, and it
cost a wrong diagnosis, a manifest test that banned ERR traps on false
grounds, and a retracted claim that Bash 3.2 does not run traps at all.

Proposal

Fold install-whitaker's private harness onto the shared
composite_fragments.py added in #443, which writes each fragment to a file and
runs the file, and which carries two regression tests for exactly this:

  • one asserting a fragment's $0 names a file rather than bash;
  • one asserting a fragment's ERR trap fires when its last command fails.

install-whitaker's copy predates the shared module and is otherwise the same
code, so this is a deletion and an import, plus whatever small differences the
two have drifted into. Doing it also removes a 240-line duplicate.

No change to install-whitaker's ERR traps is needed. They work on a runner.

Not affected

install-nixie uses no ERR traps.

Notes for whoever takes this

It touches .github/actions/install-whitaker/tests/, which had in-flight work
in #448. Rebase onto that first.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingrefactorBehaviour-preserving restructuring that improves code health.testingTest coverage, test infrastructure, and verification tooling work.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions