| name | review-test |
| description | テストコードのレビューを行う。公開契約（入出力・エラー契約・副作用保証）を起点に、テストが振る舞いの仕様として機能しているかを評価し、改善点を優先度付きで報告する。「テストをレビューして」「テストのレビューをお願い」「test review」「review the tests」などテストそのもののレビューを依頼された時、または /review-test コマンドで起動する。 |
| effort | max |
| allowed-tools | Bash, Read, Glob, Grep |
テストコードをレビューし、改善点を優先度(P1/P2/P3)付きで報告する。
出力言語: 全て日本語で記述すること。 コード片やファイルパスはそのまま記載してよい。
このスキルの境界: 読み取りとテスト実行のみ。 ファイルの変更や git 操作（add / commit / push など）は一切行わない。Bash はテスト／カバレッジコマンドの実行と、レビューに必要な情報収集（設定ファイルの確認など）に限って使い、書き込み・破壊的操作には使わない。レビュー結果を受けての修正は別の依頼として扱う。
レビューの手順
以下の5段階で進める。順序を守ること。
Phase 1: 公開契約の分析
レビュー対象モジュールの公開契約を特定する。テストは振る舞いの仕様である。起点は「この実装が内部でどう動くか」ではなく「このモジュールは呼び出し元に何を約束しているか」である。
- 公開インターフェースの特定: export された関数・クラス・resolver・endpoint の型定義（入力型・出力型・エラー型）
- 契約の明文化: 型・JSDoc から読み取れる明示的な契約（正常系の入出力、エラー条件、副作用）
- 暗黙の契約の推定: 型では表現できない保証（null 非返却、配列順序、冪等性など）を実装から推定。内部分岐の網羅ではなく「呼び出し元が依存しうる振る舞い」に絞る
- 境界の特定: 入力ドメインの境界（空配列、null/undefined、上限下限）を型と契約から導出
実装の内部ロジック（private メソッドの分岐など）はテストケース設計の起点にしない。
Phase 2: あるべきテストケースの設計
Phase 1 の公開契約をもとに、呼び出し元が依存する振る舞いの観点でテストケースを設計する。
設計の原則: テストケースは「内部の分岐」ではなく「呼び出し元が期待する振る舞い」から導出する。良いテストは内部リファクタリングで壊れない。
導出手順:
- Phase 1 の各契約（正常系の入出力、エラー契約、副作用保証）に対してテストケースを1つ以上設計する
- Phase 1 で特定した境界を、対応する契約のバリエーションとして紐づける
- Phase 1 の暗黙の契約のうち、呼び出し元が実際に依存しうるものについてテストケースを設計する
必須のテストケース種別:
- 正常系: 主要な成功シナリオ（入力 → 期待出力）
- 異常系: 契約で定義されたエラー条件（バリデーション、権限、外部依存の失敗）
- 境界値: 入力ドメインの境界（空配列、null/undefined、上限下限）
- 副作用: 契約に含まれる副作用（DB書き込み、外部API、イベント発行）が期待通り発生すること
テストスコープ × テストサイズの選択は、プロジェクトのテストガイドラインがあればそれに従う。
Phase 3: 既存テストとのギャップ分析
Phase 2 のテストケースと実際のテストコードを突き合わせる。
網羅性: Phase 2 で挙げたケースのうち、欠けているものを特定する。正常系の主要パスが欠けていれば即 P1。
品質チェック:
- テスト名: 期待動作を明確に記述しているか
- AAA パターン: Arrange → Act → Assert に従っているか
- 独立性: テスト間の実行順序依存がないか
- 粒度: 1テスト1検証になっているか
- スコープ×サイズ: Unit が DB アクセスしていないか、Integration のモックが統合の本質を損なっていないか
- アサーションの意味: 公開契約の振る舞いを検証しているか。存在確認だけ（JS の
toBeDefined() など）で済ませていないか。内部の実装詳細をアサートしていないか
- モックの適切性: モックライブラリの正しい使用。過剰モックで実動作と乖離していないか
- 実装結合度: private メソッドのテスト、内部呼び出し順序の検証、リファクタリングで壊れる構造、実装の分岐条件をそのままテスト分割基準にしていないか
- Flaky の兆候: 時刻依存（
Date.now() 未モック）、ランダム値、タイミング/競合状態、共有状態変更、未モックのネットワーク呼び出し
- テストデータ: プロジェクトにテストデータ生成の仕組み（factory, fixture, template 等）があれば、それを活用しているか
- クリーンアップ: DB を使う結合テストでデータの後片付けが適切に行われているか
- 環境変数・グローバル状態の不変性: テスト内で直接変更していないか（JS の
process.env など）
実装側の問題検出:
テストコードのレビューが主目的だが、Phase 1 で公開契約を分析する過程や Phase 3 で実装結合度を評価する過程でバグの可能性や明らかにおかしい箇所を発見した場合は記録しておく。P1/P2/P3 とは別の「実装への指摘」セクションで報告する。
Phase 4: カバレッジ情報の取得
テストを実行してカバレッジ情報を収集し、Phase 3 のギャップと照合する。
プロジェクトのカバレッジ取得コマンド（例: yarn test:coverage, npm run test:coverage, go test -cover 等）を使用する。コマンドが不明な場合は package.json やプロジェクト設定から特定すること。
カバレッジは参考情報。100% が目標ではなく、重要なパスがカバーされているかが本質。
Phase 5: レビュー結果の出力
以下のフォーマットで出力する。
## テストレビュー: [対象の説明]
### 概要
- 対象: [ファイルパスまたはモジュール名]
- テストファイル数: N
- テストケース数: N
- カバレッジ: N% (lines)
### P1 (Critical)
- [具体的な指摘内容と理由]
- [ ] ...
### P2 (Important)
- [具体的な指摘内容と理由]
- [ ] ...
### P3 (Nice to have)
- [具体的な指摘内容と理由]
- [ ] ...
### 実装への指摘
- [テストレビュー中に発見した実装側のバグや問題]
該当する指摘がない優先度セクション・実装への指摘セクションは省略してよい。
優先度の判断基準
P1/P2/P3 は絶対的な基準で判断する。均等に振り分けるのではなく、基準に該当するかで分類する。品質の高いテストに対して無理に P1 を探す必要はない。
P1 (Critical) — マージ前に対処すべき問題。放置するとバグの見逃しやリグレッションに直結する。
以下のいずれかに該当する場合のみ P1 とする:
- 公開契約で約束された主要な振る舞いに対するテストが存在しない
- アサーションがない、または存在確認だけ（JS の
toBeDefined() など）で振る舞いを検証していない（偽の安心を生む）
- テストが公開契約の振る舞いを一切検証せず、内部実装の詳細（private メソッドの呼び出し回数、内部状態の中間値など）のみをアサートしている（偽のカバレッジを生み、リファクタリングを阻害する）
- テストスコープ×サイズの選択が不適切（Unit Test が外部リソースに依存、etc.）
- テスト間に暗黙の実行順序依存があり、並列実行やランダム実行で壊れる
- グローバル状態・環境変数をテスト内で直接変更している（JS の
process.env など）
- エラーハンドリングの主要パス（公開契約で定義されたエラー条件）がテストされていない（副次的なエラーパスの不足は P2）
P2 (Important) — 品質向上のために対処すべき問題。テストの信頼性や保守性に影響する。
以下のいずれかに該当する場合に P2 とする:
- 境界値テストの不足
- テスト名が振る舞いを適切に記述していない
- AAA パターンに従っていない
- 過剰なモックにより実動作との乖離が生じている
- テストが内部実装の詳細に過度に結合しており、振る舞いを変えないリファクタリングで壊れやすい構造になっている
- Flaky の兆候がある（時刻・ランダム値・タイミング・共有状態・ネットワーク依存）
- カバレッジレポートで変更行のテスト漏れがある
- 副次的なエラーパスのテストが不足している
P3 (Nice to have) — 改善すると良いが、必須ではない。
- テストファイルの構造・整理の改善
- 冗長なアサーションの整理
- テストセットアップの簡素化
- テストのグルーピング階層の改善（JS の
describe ブロックなど）
- テストケースの分割が契約/振る舞いではなく実装の分岐構造に基づいている（機能的には問題ないが、保守性の観点で改善の余地がある）
