fix(make): propagate test failures instead of masking via POSIX ; chaining (#550)#587
Merged
DorianZheng merged 1 commit intoMay 28, 2026
Conversation
Contributor
Author
Why this slipped — and why it matters more nowWorth calling out the practical detection asymmetry, since it explains both how this lived so long and why the fix matters more in 2026 than it would have in 2020:
As more test runs get kicked off by CI and by AI/agent-driven dev loops, the false-green signal lands on exactly the consumer that can't see the "failed" line, which is when it does the most damage. The fix's biggest payoff isn't for the developer who runs |
…ining Fixes boxlite-ai#550. Three recipes chained multiple commands inside one shell with `;` instead of `&&` / status-accumulation. POSIX evaluates `if A; then B; C; fi` to the rc of C, so: - test:unit:rust: a `-p boxlite` lib-test failure was hidden by a passing `-p boxlite-shared` run — CI saw exit 0 with 29 failed tests in the log. Now uses status-accumulation (both crates always run, final rc reflects any failure), plus `--no-tests=warn` to avoid a false-fail when FILTER scopes to a test that only lives in one crate (nextest's default --no-tests=fail exits 4 with zero matches). - dist:python Linux branch: build-guest.sh failure was masked by cibuildwheel's rc. Switched to && so cibuildwheel can't proceed against a stale guest binary. - dist:c per-platform branches: a failing dynamic-lib cp left the static-lib cp to "succeed", producing a half-staged sdks/c/dist with exit 0. Switched to && so any cp failure aborts the platform branch. Verified locally: - happy path: make test:unit:rust FILTER=container_layout → exit 0 (matches 1 in boxlite-shared, 0 in boxlite — no false-fail) - failure path: make test:unit:rust FILTER=can_create_process_in_new_user_ns → recipe exit 100, make exit 2 (the boxlite-ai#544 EACCES panic in -p boxlite correctly propagates instead of being swallowed by -p boxlite-shared finishing clean) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
b0b5c29 to
3cf3636
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three Makefile recipes chained commands with
;instead of&&/ status-accumulation. POSIX evaluatesif A; then B; C; fito the rc ofC, so an earlier failure was silently swallowed — CI sees green even when tests fail. Meta-bug: this masks every other bug, so it's the highest-leverage fix to land first.What was broken
test:unit:rust— a-p boxlitelib-test failure was hidden by a passing-p boxlite-sharedrun. Captured log in Should test:unit:rust / dist:python / dist:c chain commands with && (or accumulate $status) instead of ;? #550 shows674 passed; 29 failedfollowed by recipeexit=0.dist:pythonLinux branch —build-guest.shfailure masked bycibuildwheel's rc; wheel built against stale guest.dist:cper-platform branch — failing dynamic-libcpleft the static-libcpto "succeed", producing a half-stagedsdks/c/dist/with exit 0.Fix
test:unit:rust: status accumulation (|| rc=\$?) so both crates always run and final rc reflects any failure. Added--no-tests=warnto keepmake test:unit:rust FILTER=<test-only-in-one-crate>working (nextest's default--no-tests=failwould otherwise exit 4 from the crate with zero matches).dist:python/dist:c: switched intra-branch chains to&&so downstream commands can't run against a failed upstream..pre-commit-config.yaml: widenedlint-fixhook's files filter frommake/quality\\.mktomake/.*\\.mkso futuremake/test.mk/make/dist.mkedits go through the same autofix gate (the original narrow filter is exactly what let this class of issue land without local validation).Test plan
make test:unit:rust FILTER=container_layout(matches 1 in boxlite-shared, 0 in boxlite) → exit 0 — no false-fail from the empty crate.make test:unit:rust FILTER=can_create_process_in_new_user_nson an AppArmor-restricted host (the Question: shouldtest_can_create_process_in_new_user_nstreat AppArmor’s EACCES as an expected errno? #544 EACCES panic in-p boxlite) → recipe exit 100, make exit 2 — the failure correctly propagates instead of being swallowed by-p boxlite-sharedfinishing clean.bash -c 'rc=0; false || rc=\$?; true || rc=\$?; exit \$rc'→ exit 1.Closes #550.
🤖 Generated with Claude Code