なんの話?
テストを書いていることは大前提として、 そのテストが失敗した時のことまで考えて書いていますか? という話です。
具体例に関しては諸説あります。
どういう状況?
例えばユーザー一覧を取得した時に、 IDが50以下のユーザーだけ取得できていること をテストしたいとします。
(実際にこんなテストをするかどうかは今は気にしない)
ユーザーは id
を持ちます。
public class User {
private final Integer id;
// コンストラクタ・ゲッターは省略
}
allMatchを使った実装
いざ、 getUserList
というメソッドを使いユーザーを取得して、IDが50以下であることをアサートしましょう!
public class UserListTest {
@Test
public void test_getUserList() {
List<User> userList = getUserList();
assertTrue(userList.stream().allMatch(user -> user.getId() <= 50));
}
}
できました!
めちゃめちゃシンプルに書けてるし、いい感じなのでは 🤔
何が悪いの
このようなテストだと、テストが失敗した時に何が間違っていたのか把握することが難しくなります。
例えば id
= 60のユーザーがリストに含まれている時、下記のようなログが出力されます。
expected: <true> but was: <false>
これだとどんなデータがuserList
含まれていたのか分からず、なぜテストが失敗したのか良く分かりませんね。
こうしてみよう
public class UserListTest {
@Test
public void test_getUserList() {
List<User> userList = getUserList();
List<User> filtered = userList.stream()
.filter(user -> !(user.getId() <= 50))
.collect(Collectors.toList());
assertEquals(filtered, Collections.emptyList());
}
}
このようにIDが50以下のユーザーを取り除き、その結果が空のリストと等しいことをアサートします。
行数も増え条件が否定系になるので少しだけ可読性は下がるかもしれませんが、テストが失敗した時のログは分かりやすくなります。
expected: <[]> but was: <[User(id=60)]>
まとめ
- テストが失敗した時にどんなログが出力されるのか考えながらテストを書きましょう
- そのために、
assertTrue
ではなくassertEquals
を使うようにしましょう
メモ
- サンプルリポジトリ: https://github.com/sanopi/dont-use-allmatch/
- サンプルコードではListの具体的な型が異なる可能性がありますが、アサーションには問題ありません。気になる方は
assertEquals
の実装や、java.util.List#equals
のドキュメントを参照してみてください。