仕様化テスト作成の弊害
仕様化テストを書く際に唯一避けて通れないデメリットがあります。
それはソースコードの修正量が増えることです。
テストコードが増えるのは当然ですが、プロダクトコードへの修正も増加します。
- クラスやメソッドのスコープを広げる
- 既存ロジックをクラスやメソッドに切り出す
- 変数名、メソッド名、クラス名を変更する
- final修飾子を除去する
- 不要であることが判明したロジックを削除する
などなど、仕様化テストを書いたりさらにその後にリファクタリングを行うと
プロダクトコードにもたくさんの修正が入ります。
コードの改善活動は積極的に行うべきことですが、
コードを変更するということはそこに不具合が発生するリスクが生じます。
また変更をレビューするレビュアーの負担も増加します。
コミット履歴を工夫する
不具合発生のリスクやレビュアーの負担を低減するために
私が実行しているコミット履歴の作成方法の工夫を紹介します。
基本原則
- リファクタリングは可能な限りIDEの機能を利用する
- 小さい作業でコミットを分ける
- 作業の種類でコミットを分ける
- 作業内容をコミットコメントに書く
レビュアーが重点的に確認したい箇所をコミット差分から選択できるようにします。
作業例
実際にレガシープロダクトに不具合修正を行った際のコミット履歴を見ながら説明します。
（時系列は下から上）
以下の手順で作業しています。
- テスト追加をしやすいようにプロダクトコードにリスクの低い修正をする
- 仕様化テストを書く
- 不具合修正とそれに合わせたテストの修正
- テストで保護した箇所への大胆なリファクタリング
- メソッド名の変更など微調整
それぞれ作業内容とコミット履歴を作る際の工夫を説明します。
テスト追加をしやすいようにプロダクトコードにリスクの低い修正をする
テストを書くための下準備として細心の注意を払いながらリスクの低い修正を行います。
- 読みづらいコードの修正（無駄にスコープが広い、変数名・メソッド名が嘘など）
⇒スコープの変更、変数名・メソッド名の修正 - Mockにしたいメソッドがstaticになっている
⇒ staticをやめる、instanceメソッドでラップする - Mockにするのが面倒な構造体が引数で渡ってきている
⇒Mockにしやすい引数に変更する
これらの作業を以下のような基準で分けるようにしています。
- IDEによる修正（変数名・メソッド名の変更、メソッドの抽出など）
- 手動の修正だがほぼリスクなし（スコープの変更、利用していない引数の削除など）
- 手動の修正で少しリスクあり（引数の変更など）
今回はIDEでの修正はなかったので手動の修正でほぼリスクのないものを一つ目のコミットにまとめ、手動の修正で若干リスクがあるものを二つ目のコミットに入れています。
コメントに内容も記載しているので、レビュアーは一つ目のコミットはリスクが無い内容なので軽く見て、二つ目のコミットは少しリスクがあるのでじっくり見る、というような見方ができるようになります。
仕様化テストを書く
三つ目のコミットで実施しています。
今回は対象のメソッドが小さかったので一つのコミットで書ききっています。
テストコードが大きくなる場合は区切りの良いところでコミットを分けて書きます。
その際プロダクトコードにも修正を入れている場合はそれを明示するようにしています。
プロダクトコードにも修正が入っていればそこで不具合が発生する可能性があるためです。
不具合修正とそれに合わせたテストの修正
四つ目のコミットで書いています。
アプリケーションの挙動を意図して変える部分なので必ずコミットは分けるようにしています。
仕様化テストを書かずに不具合修正を行う場合、コードの修正はこのコミットのみになります。
テストで保護した箇所への大胆なリファクタリング
五つ目のコミットです。
仕様化テストを書いてコードを読み切ると、リファクタリングをしたい箇所が少なからず出てくると思います。
仕様化テストでメソッドの分岐網羅ができていれば、大胆な修正を行ってもある程度JUnitで動作を担保できます。
また仕様化テストがかけていれば、リファクタリングの内容に問題が無いかを仕様と照らし合わせて判断することもできます。
そういった理由から大胆なリファクタリングはこのフェーズまで我慢するようにしています。
メソッド名の変更など微調整
六つ目のコミットです。
ここではレビューでクラス名が適切ではないと指摘を受けて修正しています。
レビュー指摘の修正を行う場合も
- IDEによる修正（変数名・メソッド名の変更、メソッドの抽出など）
- 手動の修正だがほぼリスクなし（スコープの変更、利用していない引数の削除など）
- 手動の修正で少しリスクあり（引数の変更など）
- 追加の挙動修正（不具合が直りきっていないと指摘を受けた時）
といったところを意識してコミットを分けると良いと思います。
※この記事はzennで書いてた記事の転記です