Skip to content

Tests assert one outcome, and a per-crate run cannot pass empty - #450

Merged
tobert merged 4 commits into
mainfrom
test/hedged-assertions
Sep 13, 2026
Merged

Tests assert one outcome, and a per-crate run cannot pass empty#450
tobert merged 4 commits into
mainfrom
test/hedged-assertions

Conversation

@tobert

@tobert tobert commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Several tests accepted two outcomes, so they could not fail when the behavior they name broke. Each is now checked against its documented contract and the value the code produces, and asserts that one value. Each tightened assertion was shown to fail when its expected value is changed.

cancellation: code == 130 || code == 143        # left for the sibling fix that makes it 130
background job: starts_with("failed:") || "done:1"   ->  "failed:1"
wc -l: trim() == "3" || contains("3")           ->  "3\n"
glob with no match: Ok arm or Err arm           ->  Ok, exit 1, "no matches"
tree / diff / grep / ls over fixed fixtures     ->  exact output
overlay NoLocal error: contains any of 3 words  ->  the exact message

Three tests passed for the wrong reason. test_external_command_basic ran uname, which is a builtin; it now runs /usr/bin/printf by absolute path so it reaches external dispatch. test_background_job_basic raced a 100ms sleep against the job; it waits on job 1 and expects done:0. The glob validation test ran in the real $HOME, where *.txt may or may not exist; it runs in an empty directory and expects exit 1.

Five kaish-kernel test files and the kernel.rs unit tests are gated on subprocess or localfs, and subprocess is not a default feature, so cargo test -p kaish-kernel compiled them to nothing and exited 0. A self dev-dependency enabling the features would also turn them on in CI's no-default-features leg, which then ran all 16 subprocess cancellation tests, so it was not used. An ungated test now fails with the command to run:

cargo test -p kaish-kernel                                  # fails, names the fix
cargo test -p kaish-kernel --features subprocess,localfs    # all suites
KAISH_ALLOW_REDUCED_TESTS=1 cargo test -p kaish-kernel --no-default-features

CI's no-default-features step sets KAISH_ALLOW_REDUCED_TESTS, and CLAUDE.md lists the full kernel test command.

This stacks on #445.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Co-Authored-By: Claude Opus 5 noreply@anthropic.com

🤖 Generated with Claude Code

tobert and others added 4 commits September 12, 2026 16:01
@tobert @claude
test: tighten assertions that hedge on two outcomes
4cbc6fb 
Several tests matched two acceptable outcomes with `||` (or an Ok/Err
match asserting different things in each arm), so they could not fail
when the behavior they name broke. Each was checked against its actual
value and the documented or code-level contract, then either tightened
to the single real outcome, or left with a comment naming the genuine
race or environment dependence that makes both values possible.

Tightened to the pinned contract:
- background_execution_tests.rs: `false &` always reports status
  "failed:1" (job.rs status_string never emits "done:N" for N != 0);
  `wc -l` piped output is the bare count plus newline ("3\n"), not a
  loose substring match.
- realworld_builtin_tests.rs: the fixture's app.log and src/ are fixed
  data, so `grep -v DEBUG` always keeps both INFO and ERROR lines, and
  `ls` in src/ always lists both main.rs and lib.rs — asserted as two
  required facts instead of an either/or.
- tail_c_plus_tree_tests.rs, sandbox_mode_tests.rs: `tree` and `diff`
  produce fixed, deterministic output over a fixed fixture; pinned to
  the exact string instead of a single-character substring check that
  is satisfied by nearly anything.
- overlay_tests.rs: the NoLocal+overlay construction error is one fixed
  message that already names NoLocal, overlay, and virtual all at once,
  so the exact string is pinned instead of an either-or-or that any one
  term trivially satisfies. The budget-exceeded test executed the same
  script twice to get stdout from one call and stderr from another;
  reads both from a single execution, and drops the stdout half of the
  check because a redirect failure never surfaces there.
- kernel.rs: `uname` is a builtin (tools/builtin/uname.rs) and builtins
  always win over PATH (docs/LANGUAGE.md, "External Commands"), so the
  bare-glob-no-match test's `Err` arm and the uname test's 127 branch
  were both dead — no path through the code can produce them. Tightened
  to the single reachable outcome in each case.

Kept as genuine hedges, with the reason now in a comment:
- kernel.rs `test_background_job_basic`: the fixed 100ms sleep before
  reading job status races the job's own completion, the same race
  documented for `grace_escalation_sigkills_term_trapping_child` in
  cancellation_tests.rs (11/20 failures reproduced under 4x CPU
  oversubscription) — "running" is a rare but real outcome here.
- validation_tests.rs `validation_accepts_glob_pattern_ls_star`: this
  kernel's cwd is the real host $HOME (kernel.rs default_sandbox_root),
  so whether `*.txt` exists there is uncontrolled environment state,
  not a bug; the Err arm was still dead by the same glob contract, so
  the match was collapsed to `Ok` with the exit-code hedge kept.

Found, not fixed (reported separately, per brief): cancellation_tests.rs
`kernel_cancel_kills_running_external` hedges 130-or-143 because the
foreground single-statement cancellation checkpoint in kernel.rs only
runs between statements, so a cancel that lands mid-flight on a lone
external command leaks the raw 128+signal exit code (143) instead of
the 130 every other cancellation path in this file enforces
(background_program_cancel_of_external_reports_cancellation and
background_program_cancel_reaches_running_execution both assert exactly
130). Left as-is rather than picking a value that contradicts what the
code actually does today.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@tobert @claude
test: fail a per-crate run that compiles out the process suites
6cf715e 
Five kaish-kernel test files and kernel.rs's unit tests are gated on
`subprocess` or `localfs`, and `subprocess` is not a default feature.
`cargo test -p kaish-kernel` compiled those suites to nothing and exited
0; only `cargo test --all` ran them, because kaish-repl enables `full`.

A self dev-dependency enabling the features was ruled out:
`cargo test -p kaish-kernel --no-default-features --test
cancellation_tests` then ran all 16 subprocess tests, so CI's
no-default-features leg would stop testing the hermetic kernel.
Instead an ungated test fails, naming the command, when either feature
is off. CI's reduced build sets KAISH_ALLOW_REDUCED_TESTS=1, and
CLAUDE.md lists the full kernel test command.

Three earlier assertions are made discriminating rather than kept as
hedges. `test_external_command_basic` ran `uname`, a builtin; it now
runs `/usr/bin/printf` by absolute path so it reaches external
dispatch. `test_background_job_basic` raced a 100ms sleep against the
job; it waits on job 1 and expects `done:0`. The glob validation test
read the real $HOME; it runs in an empty directory and expects exit 1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tobert @claude
docs: list the full kernel test command
42540e3 
CLAUDE.md is a link to AGENTS.md, so the previous commit staged the
unchanged link and left this line out. `cargo test -p kaish-kernel`
without the features now fails feature_coverage_tests by design; this
is the command that runs every kernel suite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tobert @claude
test: keep the feature guard and glob test green in every build
f89480d 
Clippy flagged the guard's `assert!` on `cfg!(...)` as a constant
assertion; it now panics from an `if`, with the same message. The glob
validation test made its empty directory with `tempfile`, which lives on
the host and is invisible when the localfs backend is compiled out, so
the no-default-features build failed at `cd`. The kernel now creates and
removes the directory, which works on either backend.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tobert
tobert changed the base branch from kaijutsu-async-programs to main September 13, 2026 11:10
@tobert
tobert merged commit a5ed706 into main Sep 13, 2026
3 checks passed
tobert added a commit that referenced this pull request Sep 13, 2026
@tobert @claude
Merge origin/main into fix/error-keeps-prior-output
7f04113 
Brings in #445, #447, #450, and #446. CHANGELOG.md conflicted where #446
and this branch both added Fixed bullets; both are kept, in merge order.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tobert added a commit that referenced this pull request Sep 13, 2026
@tobert @claude
Merge origin/main into fix/job-stream-routing
8758bc8 
Brings in #445, #447, #450, #446, and #448. Two conflicts, both from
additions at the same spot: CHANGELOG.md Fixed bullets (all kept, in merge
order) and scheduler/pipeline.rs, where #448's `fault_result` and this
branch's `redirects_stdout` were each added after
`finalize_scatter_gather_error`. Both helpers are kept; the dispatch site
merged cleanly with `fault_result(e)` and the stream-flag restore.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@tobert
tobert deleted the test/hedged-assertions branch September 13, 2026 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tobert