test: テスト不足箇所の改善 (#821)

[keito4]

Closes #821

変更内容

• test/settings-hooks.test.js (新規）31テスト）: .claude/settings.jsonのhooks設定構造を検証
• test/eslint-runtime.test.js (新規）13テスト）: ESLintサブプロセス実行テスト
• test/integration/lib_functions.bats (12テスト追加）: platform.sh実行動作テスト

Jest: 469 → 512 tests (+43), BATS: 20 → 32 (+12)

Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added ESLint runtime behavior tests to validate linting configuration and rule compliance
  • Added integration tests for platform detection and OS-specific functionality
  • Added validation tests for settings configuration structure and hook command integrity

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds three independent test suites covering ESLint runtime validation, platform.sh shell utility functions, and Claude settings hooks and permission structure. All changes are test-only additions with no code modifications.

Changes

Test Coverage Expansion

Layer / File(s)	Summary
ESLint runtime and configuration validation test/eslint-runtime.test.js	Validates ESLint availability, configuration loading via --print-config, JSON parseability, required rules (no-console: off, complexity: max 15, max-lines: max 500), test-file-specific overrides (max-lines-per-function disabled, max-nested-callbacks: 5), and that project files (jest.config.js, commitlint.config.js, git/commitlint.config.js) lint cleanly.
Platform.sh utility function integration tests test/integration/lib_functions.bats	Verifies script/lib/platform.sh exists and is executable, platform::detect_os returns valid OS identifiers, sourcing sets PLATFORM_OS, OS detection functions (is_linux, is_darwin) are mutually exclusive and OS-correct, is_devcontainer and is_windows are callable, run_task exits cleanly for undefined tasks, and assert_supported succeeds on supported environments.
Claude settings hooks and permissions validation test/settings-hooks.test.js	Validates .claude/settings.json structure ($schema, permissions, hooks objects), event type arrays (PreToolUse, PostToolUse, Stop), hook entry shape (string matcher, non-empty hooks array with type/command fields), presence of required hook scripts in event types, extraction and on-disk verification of referenced .claude/hooks/*.py files, and permissions structure (non-empty allow/deny arrays, force-push/git reset --hard/.env protections, secret token prevention).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• keito4/config#268: Adds hook scripts (pre_git_quality_gates.py) that are validated by the new test/settings-hooks.test.js suite.
• keito4/config#602: Introduces .claude/hooks/{block_config_edit.py, post_edit_auto_lint.py, stop_test_verification.py} and settings entries that are now verified by the settings-hooks test.
• keito4/config#779: Adds shared ESLint rule configuration validation that overlaps with the new ESLint runtime behavior tests.

Suggested labels

released

Poem

🐰 Three test suites hop into view,
ESLint checks and platform too,
Settings hooks validated with care,
Coverage expanded everywhere!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	Title is in Japanese and uses vague phrasing (テスト不足箇所の改善 = test improvement) without clearly specifying what tests were added.	Consider using a more descriptive title in English or Japanese that explicitly mentions the new test files (e.g., 'Add tests for settings-hooks, eslint-runtime, and platform.sh').

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description check	✅ Passed	Description provides specific test counts and file changes but follows a simplified format that diverges from the repository's template structure significantly.
Linked Issues check	✅ Passed	The PR adds 43 Jest tests and 12 BATS tests across untested areas (settings hooks, ESLint runtime, platform.sh), directly addressing #821's goal of improving test coverage in risky/frequently-changed areas.
Out of Scope Changes check	✅ Passed	All changes are test-only additions focused on validating configuration and runtime behavior of existing code without modifying any production functionality.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/issue-821-20260613-0302

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude finished @keito4's task in 2m 21s —— View job

---

PRレビュー: test: テスト不足箇所の改善 (#821)

• コンテキスト収集
• 変更ファイルのレビュー (test/eslint-runtime.test.js, test/settings-hooks.test.js, test/integration/lib_functions.bats)
• フィードバック作成

---

総評

テストカバレッジを 469→512（Jest +43）、BATS 20→32（+12）に拡張する意義のある変更です。テストのロジックは全体的に正確で、実際の設定・スクリプトを参照して検証する設計は妥当です。以下いくつかの minor 指摘と recommendation を挙げます。CI ブロッカー相当（critical/major）は存在しません。

---

test/eslint-runtime.test.js

[minor] 複数行 JSDoc コメント (lines 8–12)

/**
 * Run ESLint via npx and return the result.
 * @param {string[]} args - Arguments to pass to eslint
 * @returns {{ status: number, stdout: string, stderr: string }}
 */

リポジトリガイドライン（"Never write multi-paragraph docstrings or multi-line comment blocks — one short line max"）に違反しています。関数名・引数名・戻り値の型は一目で分かるため、このブロック全体を削除するか 1 行に圧縮することを推奨します。

Fix this →

[minor] --print-config サブプロセスの重複呼び出し (lines 47–104)

runEslint(['--print-config', 'commitlint.config.js']) が "Configuration loading" ブロック内で 5 回独立して呼ばれています（各呼び出しに 30 秒タイムアウト）。beforeAll で 1 回実行して変数にキャッシュすると、テスト速度が改善されます。

describe('Configuration loading', () => {
  let config;
  beforeAll(() => {
    const result = runEslint(['--print-config', 'commitlint.config.js']);
    config = JSON.parse(result.stdout);
  });

  test('ESLint should load eslint.config.mjs without errors', () => { ... });
  test('config should include no-unused-vars rule', () => {
    expect(config.rules).toHaveProperty('no-unused-vars');
  });
  // ...
});

---

test/settings-hooks.test.js

[recommendation] force push ブロックのチェックに論理的な抜け (line 215)

const hasForceBlockPush = deniedCommands.some((cmd) => cmd.includes('--force') || cmd.includes('-f:'));

実際の deny リストには "Bash(git push -f)" (コロンなし) も含まれており、このエントリは -f: にマッチしません。テストは --force マッチで true を返すため現状は通りますが、-f パターンのカバレッジを明示するなら -f も含めるとより堅牢です。

const hasForceBlockPush = deniedCommands.some(
  (cmd) => cmd.includes('--force') || cmd.includes(' -f')
);

---

test/integration/lib_functions.bats

[minor] ! grep -q パターン（BATS の慣用表現に非準拠）(lines 112–113)

! grep -q "NPM_TOKEN" "$test_output"
! grep -q "BUNDLE_RUBYGEMS" "$test_output"

BATS では ! 単独で否定すると、ファイルが存在しない場合（grep exit 2）でも ! が 0 に変換されサイレントパスになります。BATS の慣用表現は run + ステータス確認です：

run grep -q "NPM_TOKEN" "$test_output"
[ "$status" -ne 0 ]

run grep -q "BUNDLE_RUBYGEMS" "$test_output"
[ "$status" -ne 0 ]

Fix this →

[recommendation] output::require_command の失敗ケースが未テスト

@test "output::require_command succeeds for existing command" { ... }

成功パスのみカバーしています。存在しないコマンドを渡したとき非ゼロで終了することを確認するテストを追加するとカバレッジが向上します。

---

確認済み

• settings.json の Stop フック "matcher": "" は空文字列 → typeof entry.matcher === 'string' は true となり、テストは正しく動作しています ✓
• hook スクリプト参照（block_dangerous_commands.py 等）は実ファイルとの整合性チェック済み ✓
• platform::run_task / platform::assert_supported テストは実装の振る舞いと一致 ✓

---

判定: ✅ APPROVE

critical/major 相当の問題は見つかりませんでした。minor 指摘（JSDoc コメント、! grep -q パターン）は対応推奨ですが、マージブロッカーではありません。

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/settings-hooks.test.js`:
- Around line 122-126: The test currently uses substring checks on
settings.hooks.PreToolUse.matcher which can match unintended strings; update the
matcher assertion in the test 'should have a Write|Edit|MultiEdit matcher for
config edit blocking' to check for exact equality to the canonical matcher
string "Write|Edit|MultiEdit" (i.e., replace the includes(...) checks with
e.matcher === 'Write|Edit|MultiEdit'), keeping the rest of the expectation
(expect at least one matching hook) unchanged.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 40de06de-78f7-4b30-a0fa-f6ee50fd29b1

📥 Commits

Reviewing files that changed from the base of the PR and between f60901f and 42f2143.

📒 Files selected for processing (3)

• test/eslint-runtime.test.js
• test/integration/lib_functions.bats
• test/settings-hooks.test.js

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Strengthen matcher assertion to avoid false positives.

At Line 123, substring checks (includes('Write') / includes('Edit')) can pass unintended matcher values (e.g., any random string containing Edit). If the intent is the canonical config matcher, assert exact equality with Write|Edit|MultiEdit.

Suggested patch

-      const editHooks = settings.hooks.PreToolUse.filter(
-        (e) => e.matcher.includes('Write') || e.matcher.includes('Edit'),
-      );
+      const editHooks = settings.hooks.PreToolUse.filter(
+        (e) => e.matcher === 'Write|Edit|MultiEdit',
+      );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/settings-hooks.test.js` around lines 122 - 126, The test currently uses
substring checks on settings.hooks.PreToolUse.matcher which can match unintended
strings; update the matcher assertion in the test 'should have a
Write|Edit|MultiEdit matcher for config edit blocking' to check for exact
equality to the canonical matcher string "Write|Edit|MultiEdit" (i.e., replace
the includes(...) checks with e.matcher === 'Write|Edit|MultiEdit'), keeping the
rest of the expectation (expect at least one matching hook) unchanged.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 42f2143255

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Add CI paths for Claude settings hook tests

Because this suite validates .claude/settings.json and the hook scripts under .claude/hooks, it currently does not run when those are the only files changed: I checked .github/workflows/ci.yml, and the dorny paths-filter only marks code for JS/TS/MJS/CJS/test files and tsconfig.json (lines 64-71), while .py hooks are not included in the workflow trigger paths at all. A PR that only edits the Claude settings or removes/renames a hook script can therefore skip the unit-test job/no workflow entirely, so these assertions won’t protect the files they are meant to cover unless those paths are added to the CI filters.

Useful? React with 👍 / 👎.

[github-actions]

🎉 This PR is included in version 1.116.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀