Skip to content

feat(assert): assert_true/assert_false only accept a bare command name, and requiring 'eval' is surprising #994

Description

@Chemaclass

Summary

assert_true and assert_false run their argument as a single command word. bashunit::run_command_or_eval never splits on whitespace, so anything with arguments is looked up as a command whose name contains spaces, and fails:

assert_true "my_function"            # works — bare name
assert_true "eval test -d /tmp"      # works — eval prefix
assert_true "test -d /tmp"           # fails: unknown command: test -d /tmp
assert_true "grep -q foo file"       # fails: unknown command: grep -q foo file

The eval prefix is a real, supported branch in run_command_or_eval — it just was not documented until #993, which is why the restriction read as a bug for anyone who hit it.

Why it is worth revisiting rather than only documenting

The failing forms are the ones people write. assert_true "grep -q foo file" is the obvious way to express "this file contains foo", and it is wrong in a way that looks right. Before #991 it failed with a bare exit code: 127, which pointed nowhere; it now says unknown command: grep -q foo file, which at least names the argument. Documentation plus a clear message is a real improvement, and it may be enough — but the interface is still one where the natural spelling is the broken one.

Worth stating plainly: the current behaviour is safe. Not splitting means not evaluating, and a user-supplied string is never re-parsed unless they explicitly ask with eval. Any change here trades that away, which is why this is a question rather than a patch.

Options

  1. Leave it; documentation only. Now done (docs(assert): correct the assert_true guidance — arguments need an eval prefix #993). Zero risk, and the failure message names the cause. The bar for changing anything is whether people still get caught after reading it.
  2. Split on whitespace when the first word resolves to a command or function. assert_true "grep -q foo file" would work. But it silently changes what a string means, quoted arguments containing spaces break in a new way, and it makes the assertion sometimes-word-splitting — the kind of "depends what you passed" behaviour that is hard to reason about.
  3. Accept a variadic form. assert_true grep -q foo file — arguments as real arguments, no re-parsing, no quoting ambiguity. Backwards compatible: a single argument keeps today's meaning exactly. This is my preference if anything changes.
  4. Point people at assert_exec, which already handles commands with arguments properly and asserts on exit code, stdout and stderr. Possibly the honest answer: assert_true is for a bare predicate function, and anything more belongs in assert_exec. If so, say that in the docs rather than teaching the eval workaround.

Constraints

  • Public API. Option 3 is additive; option 2 changes the meaning of existing calls and would need a very careful look at what currently passes.
  • Bash 3.0+; "$@" handling only, no new syntax surface.
  • Per-assertion path stays fork-free — see .claude/rules/perf-fork-budget.md.
  • Whatever is decided, docs/assertions.md should stop presenting eval as the normal way to do this if a better one exists.

Acceptance criteria

  • A decision is recorded between documentation-only, variadic, and "use assert_exec"
  • If the interface changes, existing single-argument calls behave identically — verified against the full suite, not assumed
  • docs/assertions.md reflects the decision, and the eval guidance is kept only if it remains the recommended form
  • make sa · make lint · ./bashunit --parallel --simple --strict tests/ · bash build.sh bin -v

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions