「単体テストの考え方/使い方」で得た知見をレガシーコードの多いプロジェクトで実践してみた感想

本書の感想だが、Railsでの開発で最初にRSpecの使い方を学んだきりテストについて全く学んでこなかった身としては、テストコードを書く際の指針、プラクティスがはっきりし、非常によい内容だった。 その上で、学んだ内容を実際にプロジェクトで実践しようとした際、レガシーコードまみれのプロジェクトであることが起因して色々と問題があった。

私が関わっているプロジェクトで単体テストを書けるコードはほぼ無い

私がこの本を読んで得た一番の学びの1つがこれだった。 単体テストの定義をおさらいすると

  • 「単体」と呼ばれる少量のコードを検証する
  • 実行時間が短い
  • 隔離された状態で実行される

の3つを満たすテストコードが単体テストとなる。 この内1つ目と3つ目の定義には解釈の余地があり、その解釈をめぐってデトロイト学派とロンドン学派という2つのスタイルが存在している。

この本の著者は古典学派であり、古典学派の立場では「単体」とはビジネス上の単体のふるまいを指し、「隔離」というのはそれぞれのテストケースが他のテストケースから隔離された状態で実行されるということを意味する。

ここで私が現在携わっているプロジェクトについて話をしなければならないのだが、以下のような残念な状態になっている。

  • アプリケーション外部への副作用を持つ依存関係が何個もあるクラスに複雑なロジックが書かれている
    • 酷いときにはControllerからRepositoryを呼び出してそのままロジックが始まったりする
  • domainというパッケージの中には、DBのテーブル構造を写しただけの何のメソッドも持たない構造体のようなものが入ってる
  • DBのテーブル1つに対しRepositoryが作られており、集約はほとんど活用されていない

この本では、それぞれのクラスを「依存関係が多い/少ない」, 「コードが複雑/複雑でない」という2つの軸で評価できるとしており、それぞれのクラスは以下のように名前が付けられている。

依存関係が少ない 依存関係が多い
コードが複雑 ドメインモデル/アルゴリズム 過度に複雑なコード
コードが複雑でない 取るに足らないコード コントローラ

この内、過度に複雑なコードはテストしないと危険だが、依存関係が多いために簡単にはテストができないという厄介なものだ。 このようなクラスはコードの複雑さが高いが依存関係をほとんど持たない「ドメインモデル/アルゴリズム」と、依存関係を多く持つがコードの複雑さがほとんどない「コントローラ」に分離する必要がある。 、依存関係が多くロジックも複雑なクラスはテストしないと危険だがテストが困難であるため、依存関係が多いが複雑なロジックを持たないクラスと、ロジックが複雑だがアプリケーション外部への副作用を持つ依存関係は持たないクラスに分ける必要があると説いている。 そして、依存関係が少なくテストしやすい「ドメインモデル/アルゴリズム」を単体テストで、依存関係が多いが複雑なロジックを持たない「コントローラ」は必要があれば統合テストによりカバーしていく。

ここで私のプロジェクトを振り返ってみると、「ドメインモデル/アルゴリズム」に書かれるべきだったビジネスロジックの大半が「コントローラ」に流れ出てしまい、「過度に複雑なコード」が大量に生まれてしまっている状態だ。 ビジネスロジックが漏れ出てしまったドメインモデルのなり損ないについては「取るに足らないコード」に分類できる。 このような状況では、実行時間が短く他のテストケースから隔離された状態で実行される単体テストは書きようがない。 リファクタリングの前に追加されるテストは統合テストにならざるを得ない。

レガシーコードまみれのプロジェクトではリファクタ耐性を意識して統合テストを追加していくしかない

ではこの本を読んだのは無駄だったのかと言えば、「単体テストの考え方/使い方」というタイトルに反して統合テストのプラクティスについても書いてあったので全く無駄ではなかった。 本書では、質のいい単体テストの性質として以下の4つを挙げている。

残念ながら、これら4つの要素の内最初の3つは互いにトレードオフの関係になってしまう。 実際に単体テストを作成する際はリファクタリング耐性を重要視しつつも、退行への保護と迅速なフィードバックへの間でバランスを取る必要がある。 統合テストは、これの内3, 4つ目の性質を犠牲にし、代わりにより強い対抗への保護, リファクタリング耐性を獲得したテストだと考えられる。 どうしても実行時間がかさむので迅速なフィードバックは得られなくなるし、セットアップなども長くなりがちなのでテストコードは単体テストより長くなり、その分保守性は低くなってしまう。

レガシーコードは必然的にテストで保護された後でリファクタリングを受けるので、退行への保護はもちろんリファクタリング耐性が極めて重要になってくる。 リファクタリング耐性を落とす要因として、モックすべきでないクラスのモック化が挙げられる。 リファクタリングされていく内に、もともとモックしていたクラスの呼び出しが行われなくなり、モックの呼び出しをテストしていたテストケースが落ちる。 このようなテストの失敗は、テストが落ちた原因の調査に時間をとられる分、コンパイルエラーになるようなテストの壊れ方よりだいぶ性質が悪い。

ではどのようなクラスをモックすべきかとこの本は説いているのかというと「アプリケーションの管理下にないプロセス外依存だけをモックせよ」と書いている。 順に説明していくと、まず「プロセス外依存」とは、DBや外部APIファイルシステムなどプロセスに割り当てられたメモリの外の存在にアクセスすることを指す。 次に「アプリケーション管理下にない」とは、アプリケーションを経由する以外にアクセスする方法が用意されていることを意味する。 たとえばDBなら、そのアプリケーションからしか直接読み書きされないならアプリケーションの管理下にあるプロセス外依存だが、もしそのDBを他のアプリケーションも利用するのであれば、アプリケーションの管理下にないプロセス外依存となる。 外から利用されないアプリケーションの管理下にあるプロセス外依存は実装の詳細であると考えられ、実装の詳細をテストしてしまっているテストは壊れやすくなる。

これまで私は統合テストの実行時間削減のためにRepositoryをモックするか、そもそも前術の「DBのテーブル1つに対しRepositoryが作られており、集約はほとんど活用されていない」という事情があるためにRepositoryの設計自体がイケてなく作り直すことになるのだからDBのレコードを確認するテストを書くべきなのか悩んでいたが、この辺の話を読んで、統合テストは迅速なフィードバックなど考えずDBをそのまま使って、退行への保護とリタクタリング耐性に全振りすべきなのだと理解できた。

今後解決したい悩み

ただ、この本に書いてある内容は正常なテストピラミッドが構築されていることが前提になる。 テストの割合は単体テストが最も多く、統合テストはそれより少なく、最後にごくわずかなE2Eテストがあるべきというあれだ。 単体テストが書けずひたすら統合テストを追加しているのだから、当然歪なテストピラミッドになる。 するとテストの実行時間がどんどん膨らんでいく。

このままではCIでテストを回す際に困ってしまうというのもあるのだが、1回の実行に30秒くらいかかるテストスイートを頼りにリファクタリングを行うというのは実際やっていてあまり開発体験がよくない。 リファクタ後は統合テストをやめて単体テストできる部分は単体テストに切り替えていきたいが、なかなかそこまで手が回らないのが現状だ。 この点については今後プラクティスを探っていきたい。