Skip to content

fix test that was broken if sh points to dash#2436

Open
evgunter wants to merge 1 commit into
mainfrom
ev/dash-syntax-check
Open

fix test that was broken if sh points to dash#2436
evgunter wants to merge 1 commit into
mainfrom
ev/dash-syntax-check

Conversation

@evgunter

@evgunter evgunter commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

or could just ensure it doesn't use dash


Problem

libs/mngr/imbue/mngr/hosts/host_test.py's test_build_remote_lock_command_* tests validated the generated remote lock command with subprocess.run(["sh", "-n", "-c", cmd]), intending a no-exec syntax check.

But dash ignores -n when the script comes via -c (it only honors -n for stdin/file input). Since /bin/sh -> dash on Debian/Ubuntu, the command actually ran, executing its mkdir -p /mngr/.... This passed on CI (root, writable /) but failed locally on an unprivileged or read-only / with mkdir: cannot create directory '/mngr': Permission denied.

Fix

Feed the script via stdin (subprocess.run(["sh", "-n"], input=cmd, text=True, ...)), where dash honors -n and performs a genuine no-exec syntax check. Verified: dash executes sh -n -c 'echo X' (prints X) but does not execute echo X | sh -n, while still catching syntax errors (rc 2).

Latent test fragility, not a product bug.

🤖 Generated with Claude Code

The _build_remote_lock_command tests ran `sh -n -c <cmd>` intending a
no-exec syntax check, but dash (default /bin/sh on Debian/Ubuntu) ignores
-n for -c scripts and executes them. The command's `mkdir -p /mngr/...`
therefore ran and failed on an unprivileged or read-only /. Feed the
script via stdin (`sh -n` with input=cmd), where dash honors -n and does a
true no-exec syntax check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@evgunter evgunter changed the title Fix sh -n syntax-check tests that actually executed under dash fix test that was broken if sh points to dash Jul 11, 2026
@evgunter evgunter marked this pull request as ready for review July 11, 2026 23:40
@evgunter evgunter requested a review from joshalbrecht July 12, 2026 02:54
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