はじめに

この記事の内容

この記事は上記記事で作成したデータベースに接続するテストの仕組みを運用した際に発生した問題点とそれに対する分析、解決策の案をまとめた記事です。
データベースに接続するテストの詳細な内容は上記記事を参照していただければと思うのですが、作った仕組みの概要としては データベースに接続するJUnitTestをCIで実行するしくみ です。

これによって私が開発しているプロダクトのテストの仕組みの全体像は以下のようになりました。

CIで実行されるJUnitTest（Javaプロセスのみ）の実行基盤
CIで実行されるJUnitTest（データベースにアクセスする）の実行基盤
機能開発時に開発者が作成し協力会社の打鍵者の方に依頼するファンクショナルテスト
これは自動E2Eテスト等でカバーできるものも含まれており、そういった仕組みも整備しなければいけないという課題認識があります。
QAチームが作成しているブラックボックスE2Eシナリオテスト
受入テストのようなイメージのものです。

組織としては既存システムの保守を行う組織で50名ほどの開発者がいました。
開発者のスキルレベルや経験にはばらつきがあり、プロダクト経験10年を超える開発者から新卒入社1年目の開発者までさまざまな人が所属しているという状況でした。
（現在もだいたいそのような状況です）

反省したこと（結論的なもの）

データベースと結合したテストの仕組みを運用していく時は以下が重要

データベースがどういう状態であれば正しいのかを明確にして示す
正しいデータの状態を具体例を出して伝え続ける

データベースと結合したテストに限らず、新しいテストの仕組みを作り運用していく時には以下が重要

それぞれのテストの仕組みの責務を明確にして示す
テストの仕組みの責務にあった実装の方法を具体例を出して伝え続ける

運用時に起きた問題

実は以下に書いた以外にもテスト用のConnectionの取得ロジックにConnectionPoolを使い忘れていてConnection取得のコストが高くなってしまっていたとかいうしょぼいミスもあったのですが、こういった話は共通側のロジックを修正すれば良い話なのでひとまず ~~隠ぺい~~ 省略します。

テストデータの投入ルールが定まっておらず混乱した問題

発生した問題

データベースに接続するテストはCIの中で一つの初期出荷状態のデータベースを立ち上げてすべてのテストで共用するという実装でした。たくさんのテストが実装されていくと、同じテーブルにアクセスするテストも増えてきました。特に共通で利用されるテーブルは多くのテストでアクセスされることになりました。

そうなった時に以下のような問題が発生してCIでのテスト実行が失敗するということが起こり始めました。

あるテストが初期状態のデータベースに存在するデータを消してしまい、そのあとに実行するテストが失敗する
あるテストが投入したデータと別のテストが投入するデータのプライマリキーが重複し、あとで実行されるテストが失敗する

しくじりポイント

テストに必要なデータの投入はテストのセットアップで行うことにしていました。 ¹

ただし初期状態のデータベースに存在するテーブルやデータはそのまま利用して良いものとしていました。開発者はDockerで立てた初期状態のデータベースで動くようにテストを作成しているため、最初から入っているデータの投入はほとんどの場合行っていませんでした。
また投入したデータを初期状態に戻すことは必須とはしていませんでした。

そのためテストケースが増えてくると前述のようなテストどうしのデータの扱いがバッティングしてしまう事態となったのです。

どうすれば良かったのか？

関係する開発者が迷わないように、少なくともはっきりとしたルールを決めるべきでした。

例えば テストで実行したデータの変更を全てもとに戻すこと でも テーブルのCREATEを含めてテストに必要なデータは実行するテストが投入すること でも構わないと思います。

いずれにしてもはっきりとしたルールを定め、できれば特に新人の開発者が迷わないように具体的な事例を添えて明文化して、誰でも確認できる場所に置き、繰り返し伝えて誰もが当たり前に守れるようにする必要がありました。

我々が定めたルール

我々は以下のようにルールを定めました。

初期状態のデータベースを前提にテストの実装をする
- 初期状態のデータベースのテーブルをドロップしない
- 初期状態のデータベースに入っているレコードを消さない
- ただし通常の操作で行われる変更は行って良いし、他のテストが行った変更を吸収できるように実装する

また具体的な良い行動、悪い行動を示して明記しました。

共通の設定テーブルをCLEAN_INSERTして自分のテストに必要なレコードだけが入っている状態にして、放っておく
× 共通の設定テーブルに出荷されているレコードがない状態は通常ありえない
共通の設定テーブルのテストで利用するレコードをUPDATEして使いたい値に変更して、放っておく
〇共通の設定テーブルに出荷されているレコードから設定値が変更されることはある（つまり後で実行するテストが責任をもつべき）
初期状態が空のユーザーデータが入るテーブルにテストのために通常利用で入るレコードを投入し、放っておく
〇初期出荷状態が空のテーブルにレコードが入っていることはある
初期状態が空のユーザーデータが入るテーブルで他のテストが投入したデータと被って一意制約違反にならないように一旦空にしてからデータを入れる
〇初期状態が空のテーブルにレコードが入っていることはある

やや難しめのルールにはなってしまったのですが、理解すれば実装量が少なくてすむところに決められたと思っています。
現状ある程度定着して機能していると思っていますが、新たに人が入ってきた場合にきちんと機能するのかは引き続き観察する必要があると思っています。

テストサイズの戦略を示せていなかった問題

発生したこと

データベースに接続するJUnitTestの実行基盤ではテストのサイズは小さくしましょうという提言はしていました。²しかし特段実装の制約や強いルールは設けていませんでした。

理由は以下２つです。

まずはやりたいことを実現するためのツールとして定着してほしかったため
外側から包み込むテストを書くことができるようにするため

外側から包み込むテストを書く必要がある理由は我々のプロダクトのコードがレガシーコードであるためです。
レガシーコードはテストを書くことを前提に設計されていないため、データベースにアクセスするコードがクラス分離されていなかったり、分離されている場合も容易にインジェクションできないような作りになっている場合が多くあります。
よくある例として責務がきちんと分割されていない神Serviceクラスに不具合が見つかった場合をあげてみます。

まずは不具合動作を含めた仕様化テストを書きます。

この後

不具合修正した後のふるまいを神ServiceTestに反映しFailさせる
神Serviceの不具合を修正する -> 神ServiceTestがSuccessになる
テストで保護できているのでついでにSQL発行箇所を分離

このように進めることで安全に不具合修正ができます。
そうするとクラスの構成としてはこのような形になります。

このコードの改善やテスト整備の活動自体はうまくUnitTestを利用することができており、悪いことではないと思っています。
またこのステップで改善を進めた上で、この状態で実装を止めるということも悪いことではないと思っています。
（一度書き上げたものを変更するにはコストが発生するので）

しかし０から実装したり、気が済むまでリファクタリングするのであればこのような実装にすると思います。

これがみんなに伝わっているだろうか？
という引っかかりを実装が盛んになってからは感じていました。

しくじりポイント

テストサイズとサイズダウン戦略に関してもっと丁寧な説明が必要だった
仕組みを作った時の説明ではテストサイズに関する定義が不明瞭であったり、（結合するクラスの数なのか？プロセスやマシンの数なのか？）誤解を生んでしまうような表現を含んでしまっていました。また具体的なサイズダウン戦略も示せていませんでした。
制約がある中で外側から包むテストを書く場合と制約なくテストが書ける場合のお手本がそれぞれ必要だった

どうすれば良かったのか？

テストサイズとサイズダウン戦略に関してもっと丁寧な説明が必要だった

テストのサイズとサイズダウン戦略については、開発生産性カンファレンスでのt_wadaさんの発表が大変分かりやすく説明してくださっています。

これをデータベースにアクセスするテストの仕組みに当てはめると、
このテスト構成よりも

このテスト構成の方が、テストダブルを使うことでサイズを小さくできている。

ということが分かります。

テストサイズの定義を明確にし、具体的な実装をあげてどちらのテストサイズが小さいのか、といったことを示すべきだったのではないかと考えています。

制約がある中で外側から包むテストを書く場合と制約なくテストが書ける場合のお手本がそれぞれ必要だった

利用してもらうイメージが湧くように、レガシーコードに対して外側から包むテストを書いて不具合修正をする前述の流れのようなPRのお手本は用意していました。しかし０からコードを書いた場合のお手本を用意していなかったため、外側から包むテストの実装に寄りすぎてしまったのではないかと考えています。
レガシーコードの場合はサイズが大きな状態からテストを書いていく必要があり、分割していない構成で留めることは悪ではない。しかしより改善できる状態であることも認識はするべきで、０から書くのであればより良い状態を目指すべき。
そういった考えを示し共有した上で、０から書いたパターンでのお手本となるケースも書いて、参照してもらえるようにするべきだったなと考えています。

勉強会の実施とお手本になるコードの準備にこれから取り組んでいきたいと思います。

まとめ

テストの仕組みに限らず多くの開発者が関わる仕組みを使ってもらい、開発に活かしてもらうためには、具体的なお手本の用意とその背景にある設計思想が定着するように繰り返し伝えるという活動が重要であることを感じています。
またそういった活動の中で得たフィードバックを仕組みに反映し、メンテナンスし続けていく必要があるということも感じています。
これからE2EテストやAPIテストの仕組みも作っていきたいと考えているので、今回得た反省を活かしてがんばっていきたいです。

おわり。

詳細は前回記事のテストクラスとテストデータの距離を近く保つ仕組みにするに記載。 ↩
詳細は前回記事の実装者にテストのサイジングを意識してもらうに記載 ↩