この記事は Merpay Tech Openness Month 2022 の15日目の記事です。
はじめに
こんにちは。Credit Design Teamでバックエンドエンジニアをしている@tanaka0325です。主にメルペイスマート払いの開発をしています。
この記事では、先日私のチームで作成したユニットテストのガイドラインについて紹介します。
課題
現在私が担当している「メルペイスマート払い」のマイクロサービスは、もともと「メルカリ月イチ払い」として提供されていたコードを流用し、新規要件となる機能を追加して作られたマイクロサービスです。
マイクロサービス化するにあたり、「メルカリ月イチ払い」にあったデータはマイクロサービスリリース後に随時マイグレーションをする方針になったので、既存のデータをマイグレーションしつつ、定額払いなどの新規機能を追加してきました。メルペイスマート払いのマイクロサービスが作られた経緯については下記の記事が詳しいです。
【書き起こし】メルペイスマート払いにおけるマイクロサービス化の軌跡 – 吉田 拓矢【Merpay Tech Fest 2021】
さまざまな新規機能を追加してきた結果としてコードだけでなくテストも肥大化し、今となっては不要なテストが残っていたり、テストから意図を汲み取ることが難しいケースが存在している状態になってしまっていました。これによりコード理解の妨げや、機能追加時の工数への影響も出始めていました。
この状態を解決するべく、テストのリファクタリングをすることに決めました。ただしコードの設計に正解が無いのと同じ様に、テストの書き方にも正解はありません。まずはチームメンバーみんなが納得できるようなガイドラインを作成することにしました。
大事にしたこと
今回ガイドラインを作成する上で、下記のことを大事にしました。
1つ目は、現実的に実現可能なものにする、ということです。今回は新規でテストを書くのではなく、既存のテストに適用していくことになるので、実装コード側を大きく改修する必要があるルールにはしませんでした。理想的だが実現可能性が低いルールを作ったとしても、結局適用できなければ意味がありません。
2つ目は、巷でよく聞くルールだとしても鵜呑みにせず自分たちの状況に合うかどうかを検討する、ということです。たとえ一般的に良いと言われているルールだとしても、必ずしも今の状況に適しているとは限りません。こういったルールも無条件に組み込むのではなく、しっかり議論していきました。
3つ目は、もし今後ガイドラインの項目で違和感を感じるものがあれば、随時チーム内で相談し、アップデートをしていくということです。テスト対象となるコードは設計から書き方まで日々変わっていきます。今時点で最適だと思えるルールも明日には変わっているかもしれません。また、メンバー自身の考えも移り変わっていくので、より良いアイデアが生まれてくるかもしれません。変化に柔軟に対応できるよう、メンバーには議論の初期の段階から、その都度相談して変えていきましょう、と話していました。
ガイドラインの項目
実際にガイドラインの項目を紹介する前に事前に強調しておきたいことがあります。それは今回のガイドラインは、あくまで今のチームメンバーが自分たちが今運用しているサービスでやるなら、ということを前提としている点です。チームやテスト対象となるコードなど、状況が変われば内容も変わるべきだと思います。大事なことはちゃんとチーム内で議論した上で、自分たちに合ったガイドラインを作ることです。
ここからはいくつかの項目を紹介します。
ビジネスロジックのテストはそのロジックが書かれたパッケージに書く
例えば、あるパッケージ（以下 A とする）が別のパッケージ（以下 B とする）を import している構造のコードがあるとします。テストによっては B のロジックに関するテストが B だけでなく、 A にも書かれているケースがありました。こうなると B の機能改修時に B のテストだけでなく、 A のテストにも影響を与えます。また、 A のテストを読んでいる時に B に関するテストが目に入ることになるので、純粋に A のテストを把握する難易度が上がります。パッケージをまたぐようなテストは別途 e2e やシナリオテストが存在しているので、ユニットテストでは特定のパッケージのテストはなるべくそのパッケージにのみ書くことにしました。
重複・不要なテストケースは削除する
前述のように、同じようなテストが複数のパッケージに書かれているテストは重複を削除するようにしました。また、機能改修などで不要になったテストも存在していたので、それらも削除するようにしました。テストは多ければ多いほど良いわけでもないので、無駄なものは積極的に削除していきたいです。
1つのテスト関数内で条件分岐による複数条件のテストをしない
肥大化してしまったテストでは、ものによってはテスト内に条件分岐が書かれていることがありました。テスト内に条件分岐などのロジックが存在するとテスト内容を把握する難易度が上がります。チーム内では、条件別にテスト関数を分けて、個々のテスト関数内での条件分岐を減らす方が理解しやすい、という結論になったので、そのようにテスト関数を分けるようにしました。
テスト関数名のルール
テスト関数を分ける際の関数名のルールを作りました。
- テスト関数名は下記のルールで命名する
Test{*テスト対象*}_{条件}_{*想定結果*}- ** は must で記載
- 例（名前は適当です）
- 条件がある場合
Test_CreateUser_AdminUser_OK
- 条件がない場合
Test_CreateUser_OKTest_CreateUser_ValidationError
- 条件がある場合
テストのパッケージ名は分けても分けなくても良いものとする
ガイドライン作成以前までは、テスト対象のコードとテストは別のパッケージにすることが推奨されておりました。と言ってもおそらくこのルールも途中から作られたもので、現在ではどちらも混在している状態でした。今回議論するにあたり、本当に別パッケージに分けたほうが良いのかを議論しました。結果として、どちらにもメリット・デメリットがあり、必要に応じて使い分ける運用にしました。
ちなみに分けた際のメリットや非公開な機能を使ったテストについては下記の記事が参考になるので、もし興味があれば一読してみると良いかもしれません。
Go Fridayこぼれ話：非公開（unexported）な機能を使ったテスト
テスト対象の公開/非公開関数は気にせず必要に応じてテストを書く
一般的にテストは公開関数を通じてのみ行われるのが良いと言われています。仮に、今から新規で実装を開始するなら公開関数のみをテストするようにしたかもしれません。しかし今現在の実装コード/テストコードを考慮した場合、非公開関数のテストを書いても良いことにしました。理由はいくつかありますが、既存のテストで非公開関数のテストがかなりの数あったこと、今の実装コードの作り上、1つの関数が大きくなりがちで必ずしもテストが書きやすい作りになっていないが、前述の通り今回は実装コード側を大きく改修することはしないこと、などです。
テストデータの作成では共通のfixture生成関数を使う
各テストでは事前準備としてテストデータを作成しているのですが、テストによってはこの部分だけでかなりの行数になっており、依存関係なども分かりづらく可読性を下げていました。行数を抑えつつ依存関係を把握しやすい方法を検討した結果、新しくツールを作成することにしました。ここに関しては先日記事が公開されましたので、詳しい説明はそちらを参照してください。
障害起因のテストを書く場合は必ず紐づくチケットURLを記載する
どれほど頑張って開発していても障害を 0 にすることは難しいです。障害が起こってしまった後にその障害の再現テストを追加します。障害が発生する条件は、通常の動作とは違う場合が多いため、テストコードを読んだだけでは、完全に理解することはできない場合があります。そのため、障害の再現テストとして追加されたテストコードに関しては、必ず、その障害を詳しく説明しているチケットへの URL をテストコードにコメントとして記載することにしました。
最後に
今回は私のチームで作成したユニットテストのガイドラインについて紹介しました。テストのリファクタリングはサイドプロジェクト的に進めていることもあり、実際に書き換えが済んでいるテストはまだまだ多くありません。今後書き換えが進むにつれてガイドラインを見直す機会も増えてくるかと思いますが、その都度チーム内で相談しながらより良いガイドラインを作っていければと思っています。
どんな状況にもマッチする最強のガイドラインは存在しません。みなさんもぜひチーム内で自分たちに合ったテストについて話し合ってみてください。この記事がみなさんのチーム内での議論のきっかけになれば幸いです。
