テストクラスのあり方を見直していこうじゃないか

PEACOCK MEETS UP #01 テストクラスのあり方を 見直していこうじゃないか 2022/05/27 @nemuzuka

初めましての方もそうでない方も • 片桐 一宗(かたぎり かずむね) • id: nemuzuka / @nemuzuka • Java のサーバサイド開発が長い • 個人事業主

はじめに

5年くらい1つのサービス開発に携わらせてもらっていて、 「ずっと開発し続ける」ことを経験できています。 ずっと開発し続けることで見えてくる課題もいくつかあり ます。

その中のひとつが「テスト」

このテストって意味ある？ 品質維持できてる？

まれによくある • Unit Test に mock を大量に使用してて、依存するオブジェクトのシグ ニチャを変えると Unit Test 側も直さないといけない • リファクタリングする時に Unit Test がデグレ検知の役に立たない • そもそもこのテストは意味あんの？ • このテスト他の所でもやってたような…？ • テストメソッド見ても何のテストなのかよくわからん

で、出会った

テストの章で気になったところ • 変化しないテストを目指す • 明確なテストを書く

1. 変化しないテストを目指す

1度テストを書いたら… • システムをリファクタリング • バグ修正 • 想定してなかったケースを対応する時 • 新機能を追加 する時に、既存のテストに再度触れるのは 「何かが間違っている」と思うべき。

Q. なぜ間違っていると思うのか？ A. イジる必要のないテストを修正するコ ストが余計にかかるから。 既存のテストを「修正する」ことは 「壊す」可能性もある。

「変化しないテスト」の実例

ケース1
これをテストする場合、どのように行うべきか？
public void processTransaction(Transaction transaction) {
if(isValid(transaction)) {
saveToDatabase(transaction);
}
}
private boolean isValid(Transaction t) {
return t.getAmount() < t.getSender().getBalance();
}
private saveToDatabase(Transaction transaction) {
var s = t.getSender() + "," + t.getAmount();
database.put(t.getId, s);
}

1.

private なメソッドを package private にして、そのメ
ソッドに対するテストを行う
•

2.

isValid や saveToDatabase のテストを書く
processTransaction メソッドのテストとしてテストケース
分 Transaction のデータを用意する

• テストクラスから呼び出すのは public メソッドだけ

今までは1だった。けど… 公開していなかったメソッドを引っ張り出して無理やりテストすること になる。 → リファクタリングを行うことで簡単にテストを破壊できる → package private にしたメソッドの rename やヘルパーメソッドへの括りだしでテストの修正が 発生する → この修正は無駄な時間 2の方式だとリファクタしても、処理を外出ししてもテストの修正は発生 しない

「変化しないテスト」の為に テストは public メソッド経由 で実施すべき

ケース2 ヘルパークラス単体に対するテストクラスを作らず、ヘルパークラスを使用 する箇所のテストケースとして組み込むべき。 public class FooHelper { static void isValid(Foo foo) { // 前提条件を満たしていない時は Exception を throw } ✗: FooHelperTest と FooServiceTest を作成 ○: FooServiceTest のみ作成 } public class FooService { public void executeFoo(Foo foo) { FooHelper.isValid(foo); // 処理 } これに関するテストは } →FooServiceTest の中で FooHelper#isValid で Exception を throw するテストを行う

分けたほうが効率良さそうなのですが… ヘルパークラス単体でテストして、使用する箇所でテストしていない と、当該メソッド呼び出しで期待する振る舞いをしているかのテストを していないことになる → ヘルパークラスの仕様変更に気づけない可能性がある public なメソッドの責務に対するテストを優先して 行うべき

ケース3 mock を多用したテストは脆い。mock を使用した時の検証は、 • mock の呼び出し回数が想定した回数か？ • mock のパラメータが想定したものと一緒か？ →mock するオブジェクトの振る舞いが変わると 合わせて変更する必要がある

mock は使わないようにしたい • mock を使うテスト • 関連オブジェクトを想定通りに呼び出しているか？の観点で、実装 に対するテストなので脆い • 本物のオブジェクトを使ったテスト • 仕様が変わらなければテストが壊れることはない(実装だけ変わった 時はテストが壊れない) 本物のオブジェクトを使えば変化しにくいテス トになる

2. 明確なテストを書く

明確なテストを書く • 時間の経過によってシステムを理解する人が減ったり、細かな経緯を忘 れたりする • そういうことに備えるためにテストの「明確性」が大事になってくる • あいまいなテストだと何をテストしているのか？の理解に時間がかかり すぎてしまう

メソッドでなく「挙動」をテストする 「挙動」とは、 • XXX という条件下で • YYY の場合 • ZZZ になること の組み合わせ

テスト対象のメソッドが複数の挙動を持つ時 displayResults を呼び出した時 • Transaction 名を表示 • 残高が不足している時「残高が少ない」を表示 ✗ ○ @Test @DisplayName("displayResults の正常系") @Test @DisplayName("displayResults 呼び出し時、Transaction 名を表示すること”) public void testDisplay() { sut.displayResults(new Transaction("決済1", new Amount(3000))); public void testDisplay_showItemName() { sut.displayResults(new Transaction("決済1", new Amount(3000))); assertThat(ui.getText()).contains("決済1"); assertThat(ui.getText()).contains(“決済1"); assertThat(ui.getText()).contains("残高が少ない"); } } @Test @DisplayName("displayResults 呼び出し時、” + “Transaction の残高が不足している時「残高が少ない」を表示”) public void testDisplay_showWarning() { sut.displayResults(new Transaction("決済1", new Amount(3000))); assertThat(ui.getText()).contains("残高が少ない"); }

「メソッド」にフォーカスしてテストを書くと… • 挙動を追加した時にテストの変更も行う • テスト対象メソッドの挙動が増えると、既存のテストメソッドの verify も増えていく • すべてのテストメソッドに同じような verify が増えて「結局ここは何 が確認したかったんだっけ？」となってしまう • 別の人が機能追加して、テストを書こうとしても、どこにかけば良い んだっけ？になって、結局どこにも書くことになる

テストメソッド内の verify の観点を「挙動ごと」にすると • 「変化しないテスト」になりやすい • テストメソッドで何を確認しているかがわかりやすくなる • どの機能をテストしているかがわかりやすい • 別の人が機能追加時にどこにテストを書くべきか迷子にならない

まとめ • 開発し続ける案件 == 長期案件なのですが、新規案件取ってくるとは 違う悩みがあります • 「無いよりあったほうが良い」なテストは、後から負債になりがちな ので、「そのテスト必要？」ってのを考えていきたい • テストよりライブラリのバージョン UP の方が闇深いです