test: if-サブシェル条件内で無効化されていた中間アサーションを拘束化 - #173
Conversation
Bash は if の条件として実行される ( ... ) サブシェルとその内部の set -e に 対して errexit を無視し、( ... ) || rc=$? も同じ扱いになる。最後のコマンド の終了コードしか判定に使われないため、途中の [[ ]] や関数呼び出しの失敗は 無視されていた（#170 で test-native-file-tools.sh の 3 ブロックを修正済み）。 tests/unit/*.sh の該当ブロックを全数監査し、実際に非拘束だった 12 文に house style の `|| exit 1` を付けた。 - test-deploy-refactor.sh (2): build_settings_file の終了コードが捨てられ、 MDM 側は最終判定が `! jq -e` なのでビルド失敗でも空振りで pass していた - test-ghostty.sh (5): 否定テストの fixture 生成（_ghostty_test_make_app、 ln -s）と deploy_ghostty_config の終了コード。fixture が作れなくても 「拒否された」扱いで pass していた - test-mdm-install.sh (5): launcher の quiescence / quick-exit ケースは 最後のコマンドが trap - EXIT や while で _rc が常に 0 になり、PGID と group 状態のアサーションが死んでいた 他の 5 ファイル（fonts / prerequisites / update-refactor / setup-refactor / mdm-detect）は既に拘束済みで変更なし。各修正はアサーションを反転させる mutation check で FAIL に転じることを確認した。MDM ファイルは CI の run-mdm-tests.sh が実行時検証。 Claude-Session: https://claude.ai/code/session_01C9mrbbXQgV9fJ8Zy5UoEYc
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟢 Approval recommended
The changes are narrowly scoped to unit-test strictness, follow an established pattern (|| exit 1 in subshell conditions), and don’t introduce functional code-path risk.
Pull request overview
This PR hardens Bash unit tests by ensuring intermediate assertions inside if ( ... ); then subshell condition blocks are binding, avoiding false-positive passes caused by Bash ignoring errexit in conditional command contexts.
Changes:
- Enforce failure propagation in MDM launcher tests by adding
|| exit 1to key assertions inside subshell condition blocks. - Make Ghostty unit tests fail fast when fixture creation or config deployment steps fail inside
if ( ... )conditions. - Ensure
build_settings_filefailures are not silently ignored in deploy refactor unit tests.
File summaries
| File | Description |
|---|---|
| tests/unit/test-mdm-install.sh | Adds ` |
| tests/unit/test-ghostty.sh | Adds ` |
| tests/unit/test-deploy-refactor.sh | Ensures build_settings_file failures inside subshell conditions abort the condition (prevents “missing output file” from being treated as a pass). |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
概要
Bash は
ifの条件として実行されるコマンド（( ... )サブシェルを含む）に対して errexit を無視し、サブシェル内でset -eを宣言していても効かない（bash 5.3 / 3.2 の両方で実測）。( ... ) || rc=$?の形も同じ。そのためでは最後のコマンドの終了コードしか判定に使われず、途中の
[[ ]]の失敗は無視される。PR #170 のレビューでtests/unit/test-native-file-tools.shの 3 ブロックがこの形で非拘束だったことが見つかり同 PR で修正したが、同じ書き方が他の単体テストにも残っていたので全ファイルを監査した。監査結果
tests/unit/*.shのif ( ... ); then/if ! ( ... ); then/( ... ) || rc=$?ブロックをファイルごとに 1 エージェントが全数確認し、別エージェントが差分を独立検証した。&&連結、|| exit 1、rc 捕捉）で変更なしbuild_settings_fileの終了コードが捨てられていた。特に MDM 側は最終判定が! jq -e ... fileなので、ビルドが失敗してファイルが無くても空振りで pass していた（stub で再現）_ghostty_test_make_app、ln -s）とdeploy_ghostty_configの終了コード。fixture が作れなくても「拒否された」扱いで pass していた（stub で再現）( ... ) || _rc=$?11 箇所trap - EXITやwhileループで_rcが常に 0 になり、PGID・group 状態のアサーション 5 件が死んでいた修正はすべて house style（対象文に
|| exit 1を付ける）で、ブロックの再構成やset -eの追加はしていない。stub 関数本体・heredoc・jq テキストには触れていない。確認
/bin/bash -n（3.2）・/opt/homebrew/bin/bash -n・shellcheck と、対象 2 ケースだけを抽出した scratch harness で PASS を確認。5 並列負荷でも新たに拘束した PGID / group アサーションは一度も落ちていない。本番の実行は CI の unit-tests（MDM スイート含む）で検証shellcheck -S warning: 指摘 0補足
set -m下のsetpgid競合で bash 自身が stderr にchild setpgid ...: Operation not permittedを出し、既存の! -s "$_diagnostic"判定に引っかかる）。5 並列負荷でのみ再現し、単独 300 回では再現しない。MDM runner は 1 ファイル 1 プロセスで直列実行するため通常は影響しない。今回は手を付けていないhttps://claude.ai/code/session_01C9mrbbXQgV9fJ8Zy5UoEYc