test: テスト不足箇所の改善(hooks/eslint/commitlint) - #779
Conversation
未テストだった3つの領域にJestテストを追加する: - test/git-commitlint-config.test.js: git/commitlint.config.js を require() で 実際に実行し、型・ルール・日本語対応設定を検証(カバレッジ 0% → 100%) - test/eslint-complexity-rules.test.js: eslint/complexity-rules.mjs の ルール定義・ドキュメント・eslint.config.mjs との整合性を静的解析でテスト - test/hooks-integrity.test.js: .claude/hooks/ の12本のPythonフックについて ファイル存在確認・重要なパターン・関数定義を網羅的に検証 テスト数: 119 → 272(+153) カバレッジ: Statements 90.47% → 95.23% / Lines 89.47% → 94.73% Closes #778 Co-authored-by: keito4 <keito4@users.noreply.github.com>
テスト不足だったテンプレートワークフロー4種にコントラクトテストを追加: - claude-health-check.yml: 週次スケジュール、権限、重複Issue防止、 CLAUDE_CODE_OAUTH_TOKENの検証 - dependabot-auto-merge.yml: patch自動マージ、major手動レビュー必須、 pull_request_target トリガー、GITHUB_TOKEN使用の検証 - quality-gate-fallback.yml: テンプレートと実際のファイルの "Quality Gate"ジョブ名・CI状態チェックの一致検証 - label-sync.yml: トリガー条件、EndBug/label-sync使用、 権限・同期設定の検証 テスト数: 326 → 380(+54テスト) Co-authored-by: keito4 <keito4@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds four Jest test suites that contract-validate ESLint complexity rules, commitlint configuration, Claude Code hook scripts, and GitHub Actions workflow templates against expected file structure and content. ChangesRepository infrastructure validation test suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
test/template-workflows.test.jsOops! Something went wrong! :( ESLint: 10.4.0 ReferenceError: describe is not defined 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. Comment |
|
Claude finished @keito4's task in 2m 5s —— View job コードレビュー完了 ✅全 226 テスト合格。以下は観点別のフィードバックです。 総評テストカバレッジの大幅な拡充(hooks・ESLint・commitlint・ワークフロー)は価値が高く、コントラクトテストとして理にかなった設計です。Pythonファイルをテキスト検索する手法も正当なアプローチです。 Verdict: APPROVE(minor/recommendation のみ) Security ✅
Performance ✅
Quality — minor 指摘1.
|
| ファイル | 指摘 |
|---|---|
test/template-workflows.test.js |
'use strict'欠落 (minor)、beforeAll内expect (minor)、冗長テスト (recommendation) |
test/eslint-complexity-rules.test.js |
冗長アサーション (minor) |
test/git-commitlint-config.test.js |
型数ハードコード (minor) |
test/hooks-integrity.test.js |
問題なし ✅ |
critical/major 指摘はなし。全体としてテストカバレッジの改善に貢献する良い PR です。
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/eslint-complexity-rules.test.js (1)
41-67: ⚡ Quick winStrengthen the max assertions to validate rule key→value binding (and drop the vacuous
.mjstest).*
- The current checks only match substrings (
max: 15,max: 100,max: 500), so they can still pass if those maxima are accidentally assigned to the wrong rule keys; assert the actualcomplexityRules['<rule-key>']mapping (or parse/require the config and verify key→value).- The
.mjstest that only relies on a hardcoded.mjssuffix doesn’t add signal—remove it or replace it with a check that the ESM config/module is actually loadable.- If you switch to importing
.mjsmodules in Jest 30 tests viaimport(), run Jest withNODE_OPTIONS="$NODE_OPTIONS --experimental-vm-modules".🤖 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/eslint-complexity-rules.test.js` around lines 41 - 67, Replace the fragile substring assertions in the test file by loading/parsing the actual ESLint config and asserting exact key→value mappings (e.g., require or import the module and assert complexityRules['complexity'] === 15, complexityRules['max-lines-per-function'][1].max === 100, complexityRules['max-lines'][1].max === 500) instead of using expect(content).toContain('max: 15') etc.; remove the vacuous `.mjs` suffix test and replace it with a loadability check that uses dynamic import() to load the ESM config (when using import() in Jest 30 ensure tests run with NODE_OPTIONS="$NODE_OPTIONS --experimental-vm-modules"), and update the tests to reference the actual exported object (e.g., complexityRules or the module default export) rather than the string variable content.
🤖 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.
Nitpick comments:
In `@test/eslint-complexity-rules.test.js`:
- Around line 41-67: Replace the fragile substring assertions in the test file
by loading/parsing the actual ESLint config and asserting exact key→value
mappings (e.g., require or import the module and assert
complexityRules['complexity'] === 15,
complexityRules['max-lines-per-function'][1].max === 100,
complexityRules['max-lines'][1].max === 500) instead of using
expect(content).toContain('max: 15') etc.; remove the vacuous `.mjs` suffix test
and replace it with a loadability check that uses dynamic import() to load the
ESM config (when using import() in Jest 30 ensure tests run with
NODE_OPTIONS="$NODE_OPTIONS --experimental-vm-modules"), and update the tests to
reference the actual exported object (e.g., complexityRules or the module
default export) rather than the string variable content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 089317b7-f46a-46d9-a638-1b474616b947
📒 Files selected for processing (4)
test/eslint-complexity-rules.test.jstest/git-commitlint-config.test.jstest/hooks-integrity.test.jstest/template-workflows.test.js
|
(aside) |
|
Claude finished @keito4's task in 13m 14s —— View job CI調査・修正完了 ✅タスクリスト:
調査結果PR上のCI(GitHub Actions)は全て通過していることを確認しました。
PR状態: 実施した修正CodeRabbitが指摘した問題(Docstring Coverage 0%)を修正し、コードの一貫性を改善しました。
+'use strict';
+
const fs = require('fs');
const path = require('path');
+/**
+ * Read a workflow file from the repository.
+ * @param {string} relativePath - Relative path from the repository root.
+ * @returns {string} The workflow file content.
+ */
function readWorkflow(relativePath) { ... }
+/**
+ * Check if a workflow file exists in the repository.
+ * @param {string} relativePath - Relative path from the repository root.
+ * @returns {boolean} True if the file exists.
+ */
function workflowExists(relativePath) { ... }修正後の全テスト: 326テスト 全通過 ✅ 新しいCIが完了すればコードがクリーンであることが確認されます。 補足
|
Co-authored-by: keito4 <keito4@users.noreply.github.com>
|
🎉 This PR is included in version 1.112.12 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Closes #778
Summary by CodeRabbit