神コントローラーにテスト駆動開発で機能追加

TDDBC オンラインの基調講演の録画を見た。とても面白かった。和田さんのセッションやスライドは何度も見て勉強してるので、それをひとつずつ再確認しながら見ることができて、とても良かった。話の流れが分かりやすくてすごいなー。

最近実際にやったテスト駆動開発

その録画を見ながら、最近自分がテスト駆動開発で機能追加をしたことを思い出してた。今日は、それをだらだらと書いてみようと思う。プライベートメソッドに対してテストを書いてたりするのが、なんか泥臭くて面白いかなと思って。

背景

  • 自動テストが全くないプロダクト
  • God コントローラー(1000行以上ある)
  • 既にあるアクションに1つ機能を付け加える

こんな感じ

f:id:bufferings:20200809142428p:plain

ほえー

色々やる前に

さすがに神アクションに対して「Code & Fix (コードを書いてみて、動作を確認して修正すること)」はつらいので、PHPUnit を入れたいなと思った。

そう、対象のプロダクトは PHP で書かれてるんだけど、僕自身 PHP はあまり詳しくないので、PHP 自体やそのプロダクトが採用してるフレームワークの動きを確認しながら進めたいという気持ちもある。

実は、ちょっと前から PHPUnit の導入を企んでいて、ある程度調査してたから、ちょうどいいタイミングだし投入してみることにした。

なので、まずは PHPUnit をプロジェクトに追加して、Hello World 的なテストが動くことを確認しといた。ちなみにここが一番苦労した。既存のコードやデプロイフローに影響を与えない形で導入しておいた。

あとは楽しいだけ。

集中

さて、機能追加をどんな風に実装しようかな。やりたいことを考えてみる。いくつかの変数の値を元に(入力)、ごにょごにょ処理をして、結果として次の処理に渡す配列を生成すること(出力)ができれば良さそう。

本来なら、ビジネスロジックとしてコントローラーから切り離したいところだけど、そこまでやっちゃうと変化としてちょっと大きそう。だから、今回は「PHPUnit の導入」だけに集中する。これだけでも十分大きな変化だろう。

だから、新たにビジネスロジックに切り出すのではなくて、これまでのやり方に合わせて GodController の中のプライベートメソッドとして実装することにしよう。

プライベートメソッドA

じゃ、そのプライベートメソッドAに対するテストを考えてみる。ちなみに、アクションに対するテストは、現時点では難しそうだから今回は考えない。

んー。メソッドAに対するテストかぁ・・・。でかいな。Aの中には、B・C・D・Eみたいなロジックが入りそう。それぞれ別のメソッドに切り出したいな。メソッドEが一番簡単だし重要だから、そこから始めるとシンプルで良さそう。

f:id:bufferings:20200809153049p:plain:w300

機能に対するテストクラス

メソッドEのテストを書いてみよう。渡したオブジェクト2つの特定のフィールドが同じ値を持ってたら true を返すメソッドを作りたい。

GodControllerTest を作って・・・いや、ちょっとこれ名前がでかいな。んー、普通はテスト対象のクラスと1対1でテストクラスを作るところだけど・・・テスト対象のクラスがでかいからな・・・今回は機能を表すクラスにしとこ。GodControllerFeatureATest クラスを作った。よし、テストメソッドを書こ。

プライベートメソッドのテスト

んだけども、メソッドEってコントローラーの中のプライベートメソッドだから、どうしようかな。

リフレクションで呼び出すのも一つの手だけど。。。ちょっと本質的なところから遠いから、とりあえずもっと簡単な方法ないかな?

コントローラーにパブリックメソッドとして実装するのもいいけど、もっと気楽に、まずはこのテストクラスの中に実装を書いてしまおっと。こんなイメージ↓

f:id:bufferings:20200809153533p:plain:w300

動作するテスト

やっと最初のテストを書けそう。2つのオブジェクトを渡すと false が返される。。。と。

で、まだ実装がなくてエラーが出るから、クイックフィックスからメソッドを生成して。

とりあえず true を返すようにして、実行。よし。 予定通りレッド。

次は false を返すようにしてグリーンになった。

テストはちゃんと動いてるみたいだな。良かった。

ここまでで「テストが動く」ことが確認できた。

f:id:bufferings:20200809153924p:plain:w300

実装

特定のフィールドが同じ値を持ってるオブジェクト2つを渡すと true が返される。。。と。実行してレッド。

じゃ、比較の実装を入れよっと。はい、グリーン。

不安をなくす

特定のフィールドの中で1つでも異なる値を持つオブジェクトを渡すと false が返される。。。と。実行してグリーン。

ふむ。ちょっと順調すぎるけど、シンプルだからそんなもんかな。いくつかパターンを書いて全部グリーン。

え?ちょっとレッドになるか、フィールドの値を変えてみよっと。お、よかった。レッドになった。

じゃ、メソッドEには不安はもうないな。

f:id:bufferings:20200809155159p:plain:w300

他のメソッド

他のメソッドB・C・Dもそんな風にテストと実装を書いた。まだメソッドAのテストと実装は書いてない。ここでB・C・Dの実装はイケてないなぁ、もっと良い実装方法がありそうだよなぁ、と思いつつ、ただ動きはするから、まずはキレイにするよりは前に進めることに集中することにする。

リフレクションユーティリティの導入

じゃ、今テストクラスに実装したメソッドB・C・D・Eをコントローラー側に移動しよっと。

まずはリフレクション用のユーティリティメソッドを作って・・・と。で、コントローラー側にメソッドEを置いて、リフレクションユーティリティ経由で呼び出して、テスト実行・・・OK全部グリーンだ。

念の為、ちょっとメソッドEの中身を変えてみよっと。お、良かった。テストがレッドになったや。

他のメソッドも移動して、テスト実行。全部グリーンね。安心。

f:id:bufferings:20200809155845p:plain

メソッドAを作成

メソッドAのテストについて考える。全然関係ないけどここでペアプロを始めた。

メソッドAはその中でB-Eを呼び出して、結果を返す。

メソッドAは、いくつかの変数の値を元に(入力)、ごにょごにょ処理をして、結果として次の処理に渡す配列を生成する(出力)

じゃ、まずは空の配列を返すテストを書いてレッド→仮実装(単に空の配列を返す)→グリーン。で、メソッドAの実装を書く。単にB-Eを呼び出すだけだから簡単。はい、グリーンのまま。

メソッドAの他のテストを書く

既にあんまり不安はないけど、メソッドAに対するパターンを考えて、テストを一個ずつ実行しながらペアと交互に書いていく。

全部グリーンになったや。これで動作する実装は終わりだなー。

f:id:bufferings:20200809160631p:plain

メソッドAのテストをリファクタリング

メソッドAのテストは、パターンとしては大体網羅されてるけど、まずは実装することを重視したから、テストコードをキレイにして、意図が伝わるようにしよう。重複を取り除いたり、テストメソッドの名前を変えたり。都度グリーンをキープしながら。

よし。終わった。・・・ん?俯瞰して見ると、こういうパターンもあった方がいいな。足しとこ。逆に、このケースは冗長だな。消しとこ。

これでメソッドAのテストは「動作するキレイなテストコード」になった。良い感じ。

f:id:bufferings:20200809160925p:plain

要らないテストを削除

ここまで、メソッドEから始めて、一歩ずつテストを書いて、動作を確認しながらだったから、安心して最終的にメソッドAを実装することができた。

でも、メソッドAのテストがあれば、メソッドB-Eのテストって、要らないし、あると逆に邪魔になる。だから、削除しよっと。

ただ、その中でも、メソッドEのパターンは、メソッドAのテストだけだとカバーしづらいから、キレイに書き直して残しておこう。

最終的に、メソッドAのテストと、メソッドEのテストが残った。3ヶ月後の自分に意図が伝わる最小限のテストになったかな。これで安心して忘れられる。どっちもプライベートメソッドに対するテストだけどね。

f:id:bufferings:20200809161254p:plain

本実装のリファクタリング

さて、メソッドAのテストが揃ったから、本実装をリファクタリングしちゃおう。

B・C・Dの実装がイマイチだとずーっと気になってたから、メソッドFを作ってそっちにまとめてしまおっと。やっぱりこの方が読みやすいな。

メソッドAとEのテストはグリーンのままだから大丈夫。

f:id:bufferings:20200809161756p:plain

ということで

楽しかったー。安心して眠れる。みたいな気持ち。