review-tests

実装の変更にテスト (と FE なら Story) が追随しているかを点検する。追加した実装にテストがあるか、削除した実装でテストを減らしすぎていないかの両方を見る。コミット・PR 作成前のセルフチェックや、「テスト足りてる?」「Story は過不足ない?」と確認したいときに使う。

SKILL.md

GitHub

$ARGUMENTS で指定された対象 (省略時は git diff HEAD の対象ファイル) について、実装の変更にテストが追随しているかを点検してください。

観点

1. 追加・変更した実装にテストがあるか

  • 変更した実装ファイルに対応するテストファイルが存在するか。目視ではなく1件ずつ機械的に確かめる(変更ファイルを列挙し、対応するテストファイルの存在を照合する)
  • 新しく足した分岐・関数・コンポーネントがテストで通っているか
  • プロジェクトの testing.md 等にカバレッジ目標があれば、それを下回っていないか

「対応するファイルが無いから不足」ではない。 挙げる前に次の2つを確かめる。

確かめること 例
免除規定に当たらないか リポジトリの規約が生成物・型定義・DI・ルーティング等を免除していることがある。規約の実ファイルを読んで判定する
上位のテストが実質カバーしていないか 変換関数に単体テストが無くても、それを通す結合テストが出力そのものを検証していれば足りている

後者で足りているものに単体テストを足させない。 同じことを2箇所で検証する形になり、片方だけ古くなる。ただしその判断はどこにも残らないので、報告に「どのテストが代わりに担保しているか」を1行添える。

逆に、対応するファイルがあっても足りているとは限らない。 見るのはファイルの有無ではなく、変えた行へ届く条件をテストの入力が満たしているか。

見ること 確かめ方
変えた行が条件の中にあるか 差分の行から外側の if や && や .filter を辿り、描かれる条件を書き出す
固定データがその条件を満たすか テストの固定データ・モックの戻り値を開き、条件に当てはめる。空配列や null やフラグの既定値で落ちていることが多い

実際に、部品の import を遅延読み込みの版へ差し替えた PR で、呼び出し側のテストは全件通っていた。ところが固定データの添付が [] で、差し替えた部品を1度も描いていなかった。差し替えを壊しても落ちるテストが無い状態だった。

通ったテストの一覧だけでは、届いたかどうかは分からない。入力を読んでも決めきれないなら、報告に「この行を壊して落ちるテストがあるか確かめる」を1行添える( verify_the_check_worked.md )。このスキルは編集も実行もしないので、確かめるのは受け取った側。

状態を切り替える実装を触ったら、逆方向もテストがあるか見る。 片方向だけ直すともう片方は「今まで通らなかった経路」として残る。

触ったもの 一緒に見る
保存・追加・有効化 解除・削除・無効化
開く・進む 閉じる・戻す・取り消す

表示の不具合を直したときほど役に立つ。 表示が出ないせいでボタンが押されず、その先の操作が一度も通っていないことがある。直した時点で初めて通るので、そこにテストが無ければ壊れたまま出る。

実際に一覧の表示を直したところ、その表示が出て初めて押せるボタンの側にサーバーの状態を壊す経路が残っていた。表示だけの不具合として報告されていたもので、逆方向を通すまで見つからなかった。

2. 削除した実装でテストを減らしすぎていないか

テストが減ること自体は正しい。 見るのは「消した機能の分だけ減っているか」。

確認 方法
減った件数の内訳 git show <base>:<file> と現在を比べ、減ったケースが削除した機能に対応するか1件ずつ照合する
残る機能の網羅 型・分岐・項目を N 個から M 個へ減らしたなら、残る M 個すべてにテストがあるか数える
網羅テストの追随 「全種類を列挙して突き合わせる」型のテストが、減った側の定義と揃っているか

3. Story (frontend のみ)

リポジトリが frontend でなければこの節はスキップする。判定はリポジトリ単位で行う (gh repo view --json name -q .name、取得できなければリポジトリルートに package.json があり go.mod が無いか)。

  • 本数ではなく、検証したい状態が網羅されているかで見る。削除に伴って状態が消えたなら減ってよいが、状態は残っているのに Story だけ消えているなら不足
  • 消すのではなく別のデータへ置き換えれば足りることが多い (例: カスタム項目の Story を既定項目に差し替える)
  • プロジェクトの Story 品質基準 (自明な状態・視覚差のない Story は作らない等) があればそれに従う

進め方

  1. 対象の特定(種別判定・差分取得・gh pr checks は独立なので同一メッセージ内で並列実行する)

    • $ARGUMENTS がファイルパス / ディレクトリならそれを対象に
    • 省略時は git diff --name-only HEAD。PR 全体を見るなら git diff <base>...HEAD --name-only
    • 実装ファイルとテスト / Story に仕分ける。テストの拡張子は *.test.* / *_test.go 等、プロジェクトの慣習に合わせる
  2. 追随の確認

    • 実装だけ変わってテストが動いていないファイルを洗い出す
    • 逆に、テストだけ変わって実装が動いていないものも見る (テストの意図が変わっている可能性)
    • 判定の土台は pstack の principle-test-behavior-not-implementation (振る舞いを壊すと落ち、内部を変えても通るか)
    • 不具合の修正に足したテストは、修正前のコードで落ちるかを見る。修正前でも通るテストは直ったことの根拠にならない。確かめ方は pstack の tdd の手順4(直す前に新しいテストを流し、狙った理由で落ちることを見る)
    • テストが確かめているのが、変更が乗る流れの結果かを見る。部品の挙動(何ms待つか)だけを固定したテストは、流れの結果(ログイン後に記事が開く)が壊れても通る。流れの辿り方は /review-code の手順3が持つ
  3. 削除分の照合 (差分に削除が含まれる場合)

    • git show <base>:<file> | grep -c "^\s*\(it\|test\)(" 等で before / after のケース数を数える
    • 減った分が削除した機能に対応するか確認する
    • 残る機能の網羅を数える (観点2の表を参照)
  4. Story の確認 (frontend のみ): 前後の Story 名を並べ、消えた状態が実装の削除に対応するか見る

  5. 報告

    • 過不足なしなら「✅ 過不足なし」と、確認した件数を添えて簡潔に
    • 不足: ファイル / 何のテストが無いか / なぜ要るか の表
    • 減らしすぎ: 消えたケース / 対応する実装が残っているか の表

出力の目安

  • 過不足なしのとき: 3〜5行 (テストの増減とその理由、残る機能の網羅を数えた結果)
  • 不足ありのとき: 表形式で1件1行。テストを書くのではなく、不足の指摘に留める

してはいけないこと

  • 自動でテストを書き足すこと(不足の指摘までに留め、ユーザーの明示的な指示を待つ)
  • テストが減っていることを、それだけで問題視すること。削除 PR では減るのが正しい
  • 差分のテストファイルだけを見て、追随していると判定すること。テストが通っても、変更が乗る流れの結果を確かめていなければ、不具合が戻っても気づけない
  • 本数だけで判定すること。件数が同じでも中身が抜けていることがあり、逆に減っていても正しいことがある
  • 実装の妥当性・コメントの粒度まで指摘すること(別のスキルの責務)
  • カバレッジを測るために重いコマンドを回すこと。CI にカバレッジジョブがあるなら gh pr checks の結果で足りる
  • テストを書く慣習が無いリポジトリで無理に不足を挙げること(インフラ定義・スキーマ定義のみのリポジトリ等)。テストが存在しないなら、その旨を1行報告して終わればよい