はじめに
こんにちは、楽天の日暮です。普段は主にJavaを使ったシステムの開発に携わっています。
日々の開発業務の中で、テストコードのレビューを したりされたりする機会は非常に多いと思います。
みなさんはテストコードの書き方はどのように勉強してきたでしょうか？
もちろん世の中にはコーディングとテストに関する名著はたくさんありますが、私が勧める最も実践的かつ読みやすいものの一つとして、Google Testing Blogをお勧めします。
Google Testing BlogはGoogleが記事投稿しているテストに関してフォーカスしたブログで、一つ一つの記事は短いので読みやすくトピックも現場レベルの疑問や課題になっており、ためになる知見が多いと思っています。
この記事では、私がテストコードをレビューする際によく指摘するポイントを、Google Testing Blogなどで言及されているベストプラクティスを交えながら、アンチパターンとして紹介したいと思います。
これらのプラクティスが、皆さんのチームのテスト品質向上の一助となれば幸いです。
なお、本記事はサンプルの実装コードはJavaで記述し、そのテストコードとしてSpockによるテストコードを例として記載します。
Spockに関する詳細はこちらの記事もぜひ読んでみてください。
1. DRY原則の呪縛
DRY (Don't Repeat Yourself) は、ソフトウェア工学における最も基本的で重要な原則の一つです。
私たちはプロダクションコードを書く際、重複をなくしコードを効率的で保守しやすくするために、この原則に従うよう教えられてきました。
この習慣が非常に強力であるため、多くの開発者は無意識のうちに、同じ原則をテストコードにも厳格に適用しようとします。
コードの重複は「悪」であり、ヘルパーメソッドや共通のセットアップ関数でそれらを抽象化することは「クリーン」な行為だと考えがちです。
しかし、この「良かれと思って」の抽象化が、テストコードの最も重要な価値、すなわち可読性とドキュメント性を損なう罠になるのです。
重要なのは、コードの種類によって「良いコード」の尺度が異なることを理解することです。
- プロダクションコードの保守性: 主に「仕様変更への対応しやすさ」を指します。ロジックが一箇所にまとまっていれば、変更は一度で済みます。
- テストコードの保守性: 主に「テストが失敗したときの原因の特定しやすさ」と「テストの意図の理解しやすさ」を指します。
テストコードにおいては、DRYを追求するあまりロジックを共通化しすぎると、個々のテストケースがどのようなデータ（Given）で何（When）をテストし、何を期待しているのか（Then）が、一見して分からなくなります。
悪い例
テストデータの生成をヘルパーメソッドに集約してしまうと、一見DRYに見えます。
しかし、ヘルパーメソッドによる過度な共通化は可読性をむしろ損ねてしまします。
// BadOrderServiceSpec.groovy
import spock.lang.Specification
class BadOrderServiceSpec extends Specification {
def orderService = new OrderService()
def "GOLDランクの顧客は送料無料になること"() {
given:
def customer = createCustomer("GOLD") // ヘルパーメソッドで顧客を生成
when:
def fee = orderService.calculateShippingFee(customer)
then:
fee == 0
}
def "一般ランクの顧客は送料が500円であること"() {
given:
def customer = createCustomer("SILVER") // こちらもヘルパーメソッド
when:
def fee = orderService.calculateShippingFee(customer)
then:
fee == 500
}
// テストデータ生成用のヘルパーメソッド
private Customer createCustomer(String rank) {
return new Customer("user-123", "Taro Yamada", rank)
}
}
このテストでは、createCustomerメソッドの内部を追わない限り、テストで使われる顧客IDや名前といった重要なコンテキストが分かりません。
テストが失敗したとき、原因を特定するために複数のファイルやメソッドを飛び回る必要が出てきます。
この程度であれば読めばわかると思うでしょうが、テストコードを仕様書として読む場合にこのような実装はコンテキストスイッチとなりえます。
良い例
DRY原則とは異なる、DAMP (Descriptive and Meaningful Phrases) という原則があります。これはテストコードなどにおいて、コードの可読性や分かりやすさを優先させる原則です。
このDAMPはデバッグ体験を最優先に考えます。各テストが自己完結し、必要な情報がすべてその場に記述されていれば、失敗したテストメソッドを読むだけで、
- どのような状況で（Given）
- 何を実行し（When）
- 何が期待と異なったのか（Then）
を即座に理解できます。これは、プロダクションコードで重視される「コードの再利用性」よりも、テストコードにおける「一読したときの明快さ」を意図的に優先する、という重要なトレードオフの選択なのです。
さらに、DAMPなテストは、それ自体が優れた生きたドキュメントとして機能します。
新しいメンバーがプロジェクトに参加したとき、このテストを読めば、複雑な仕様書を読み解かなくても、システムの具体的な振る舞いを学ぶことができるのです。
// GoodOrderServiceSpec.groovy
import spock.lang.Specification
class GoodOrderServiceSpec extends Specification {
def orderService = new OrderService()
def "GOLDランクの顧客は送料無料になること"() {
given:
def customer = new Customer("user-gold", "Gold Member", "GOLD")
when:
def fee = orderService.calculateShippingFee(customer)
then:
fee == 0
}
def "一般ランクの顧客は送料が500円であること"() {
given:
def customer = new Customer("user-silver", "Silver Member", "SILVER")
when:
def fee = orderService.calculateShippingFee(customer)
then:
fee == 500
}
}
なぜあえてコードの重複を許容するのでしょうか。
その根底にあるモチベーションは、テストコードの第一の読者は、未来の自分や他のチームメンバーであるという思想です。
テストが失敗したときのことを想像してみてください。
CI/CDパイプラインが赤くなった時はわれわれはその原因調査を求められますが、そのときにテストコードが複数のヘルパーメソッドや複雑なセットアップロジックに依存していると、何が原因で失敗したのかを理解するために、あちこちのファイルを参照し頭の中でコンテキストを再構築する必要があり、大きな認知負荷がかかります。
以下のGoogle Testing Blogでは、DRYとDAMPはトレードオフの関係にあるが、ユニットテストを書く際にDRY原則とDAMP原則のどちらかを選択しなければならない場合、DAMP原則に重点を置くべきと述べています。
過度な抽象化は、テストの意図を不明瞭にし、失敗したときのデバッグを困難にするということは意識しておくと良いと思います。
"Note that the DRY principle is still relevant in tests; for example, using a helper function for creating value objects can increase clarity by removing redundant details from the test body. Ideally, test code should be both readable and unique, but sometimes there’s a trade-off. When writing unit tests and faced with a choice between the DRY and DAMP principles, lean more heavily toward DAMP."
2. if文の罠
みなさんはついつい、1つのテストで多くを検証しすぎていませんか？
結果のアサーションをまとめて検証する際には、罠が潜んでいます。
悪い例
テストメソッド内にif/elseのような条件分岐を見つけたら、その瞬間警戒アラートが発生するコードとなりえます。
多くの場合、1つのテストが複数のシナリオを検証しようとしている兆候であり、テストの価値を大きく損ないます。
// BadOrderServiceSpec.groovy
def "顧客ランクに応じて送料が計算されること"() {
given:
def customer = new Customer("user-123", "Taro Yamada", rank)
when:
def fee = orderService.calculateShippingFee(customer)
then:
// このような条件分岐はテストの意図を曖昧にする
if (rank == "GOLD") {
fee == 0
} else {
fee == 500
}
where:
rank | _
"GOLD" | _
"SILVER" | _
}
これは簡単な例で、こんなことはしないと多くの人が思うと思いますが、テストコードでヘルパーメソッドを実装すると、if/elseでやりくりしてしまうコードは意外と多いのではないかと思います。
このアンチパターンに陥る背景には、いくつかの心理的な要因が考えられます。
-
DRY原則の呪縛、再び: ここでもDRYの考え方が顔を出します。複数のテストケース（例: GOLD会員とSILVER会員）で、
given(前提)やwhen(操作)の部分がほぼ同じだと、「この重複を一つにまとめたい」という動機が働きます。そして、then(検証)の部分だけをif文で分岐させ、1つのテストメソッドで効率的に済ませようとしてしまうのです。 -
関連ケースのグルーピングという誤解: 「正常系」と「異常系」、「境界値の上と下」といった関連する複数のケースを、1つのテストメソッドの中でまとめて検証した方が、関係性が分かりやすいと誤解してしまうこともあります。
しかし、これらのモチベーションはテストの本来の目的を見失わせる原因となります。
if文がもたらす問題点として以下の点が挙げられます。
- 可読性の著しい低下: テストの意図が「顧客ランクに応じた送料の検証」という一つの大きな塊になってしまい、「GOLD会員なら送料無料」という個別の仕様がコードの中に埋もれてしまいます。
-
デバッグの困難さ: テストが失敗したとき、テストランナーはどのメソッドで失敗したかしか教えてくれません。
if文のどの分岐で、どのアサーションが失敗したのかを特定するには、結局コードを読んでデバッガでステップ実行する必要があり、原因究明に余計な時間がかかります。 -
ドキュメント価値の喪失:
if文のロジックを読み解かなければ仕様が分からないテストは、テスト仕様書というドキュメントとして価値を損なってしまいます。
良い例
1つのテストメソッドは、1つのこと（1つのシナリオ、1つの論理的概念）だけを検証するべきです。
これにより、テストは明確で、失敗時のデバッグも容易になります。Spockフレームワークのwhereブロックのようなデータ駆動テストの仕組みは、これをエレガントに実現するための強力なツールです。
// GoodOrderServiceSpec.groovy
def "顧客ランクに応じた送料が計算されること"() {
given:
def customer = new Customer("user-123", "Taro Yamada", rank)
when:
def fee = orderService.calculateShippingFee(customer)
then:
fee == expectedFee // アサーションはシンプルに
where:
rank | expectedFee
"GOLD" | 0
"SILVER" | 500
"BRONZE" | 500
}
このアプローチなら、テストが失敗すれば、どのデータセット（どのシナリオ）で問題が起きたのかが一目瞭然です。
以下のGoogle Testing Blogでは、1つのオブジェクト全体を比較するような広範なアサーションよりも、テストしたい特定の振る舞いに焦点を当てた、複数の具体的な（Narrow）アサーションを使うことが推奨されています。
これは「1テスト1論理的概念」の原則にも通じますので常に心がけたい内容になるかと思います。
"Broad assertions should only be used for unit tests that care about all of the implicitly tested behaviors, which should be a small minority of unit tests. Prefer to have at most one such test that checks for full equality of a complex object for the common case, and use narrow assertions for all other cases."
3. テストの書きにくさが産む、過度なモックという負債
「このコード、テスト書くの難しいな…」と感じた時、それはプロダクションコードの設計に問題があるサインです。
特に、クラスが多くの依存関係を持っている場合、テストの準備は非常に煩雑になります。
そして、この「書きにくさ」から逃れるために、多くの開発者が 過度なモック (Over-mocking) というアンチパターンに陥ってしまいます。
多くの依存を持つクラスをテストする際、その依存オブジェクトをすべてモックに置き換えるのは、一見すると合理的に思えます。
しかし、それはテスト対象の「振る舞い」ではなく、依存オブジェクトとの「インタラクション（呼び出し回数や順番など）」を検証するテストにつながりがちです。
悪い例
以下のような実装コードについて考えてみたいと思います。
// 悪い設計の例: 多くのServiceやDomainに依存している
public class OrderService {
private final UserRepository userRepository;
private final ProductRepository productRepository;
private final PaymentGateway paymentGateway;
private final AnalyticsService analyticsService;
// ...コンストラクタ
public void placeOrder(String customerId, List<String> productIds) {
// 複数の依存オブジェクトを使った複雑なロジック...
}
}
このOrderServiceクラスは、複数の依存オブジェクトを使った複雑なロジックになっているだろうことが想像つくと思います。
このOrderServiceをテストするために、以下のような「脆いテスト（Brittle Test）」を書いてしまうことがあります。
// 脆いテストの例
def "注文処理で各コンポーネントが1回ずつ呼ばれること"() {
given:
def userRepo = Mock(UserRepository)
def productRepo = Mock(ProductRepository)
def paymentGw = Mock(PaymentGateway)
def analytics = Mock(AnalyticsService)
def service = new OrderService(userRepo, productRepo, paymentGw, analytics)
when:
service.placeOrder("user-1", ["prod-1"])
then:
// このテストは「何を達成したか」ではなく「どうやったか」を検証している
1 * userRepo.findById("user-1")
1 * productRepo.findAllByIds(["prod-1"])
1 * paymentGw.charge(_)
1 * analytics.sendEvent(_)
}
このテストの問題点は、placeOrderメソッドの振る舞い（例: 注文が成功し、DBに保存される）を変えずに、内部の実装（例: paymentGwとanalyticsを呼び出す順番）を入れ替えただけで、テストが失敗してしまうことです。
このようなテストはリファクタリングの妨げとなり、開発者はコードの改善を恐れるようになります。
改善策
根本的な解決策は、シンプルにテスト容易性を考慮してプロダクションコードをリファクタリングすることです。
OrderServiceのように多くの責務を持つクラスは、より小さなクラスに分割するべきです。
// リファクタリング後のサービス
class OrderPlacementService {
private final UserFinder userFinder;
private final OrderValidator orderValidator;
private final PaymentProcessor paymentProcessor;
// ...
}
この問題については、Google Testing Blogでも言及されています。
「モックの使いすぎは、実装の詳細に結合した脆いテストを生み、リファクタリングを妨げる」と言われています。
モックの代わりにFakeを使うことで、テストは「インタラクション」の検証から「状態」の検証へとシフトできます。
FakeとしてインメモリのMapでデータを保持する実装を用意すれば、「placeOrderを呼んだ後、Fakeのレポジトリに注文データが正しく保存されているか」を検証できます。
""If you're trying to read a test that uses mocks and find yourself mentally stepping through the code being tested in order to understand the test, then you're probably overusing mocks
4. 曖昧なテスト名
テストコードは、単なるバグ検出ツールではありません。それは、システムが仕様通りに動作することを示す、最も信頼性の高いエビデンスです。
理想的なテストコードは、それ自体が実行可能な仕様書(Executable Specification) となり、ビジネスサイドのステークホルダーに対しても品質を証明する公式なレポートとして機能します。
この「テスト＝エビデンス」という思想を実現する上で、根幹をなすのがテスト名です。
Jacocoのようなツールが出力するカバレッジレポートを想像してください。
そこに並ぶ一つ一つのテスト名は、レポートの「項目」です。
その項目名が「test OrderService」のような曖昧なものであれば、そのレポートは全く意味をなさず、エビデンスとしての価値を失います。
「テストが通っている」という事実だけでは不十分で、「どの仕様と振る舞いが、なぜ正常だと言えるのか」 まで示せて、初めてテストはレポートとして完成するのです。
この「テストをレポートとして完成させる」という意識が薄れると、私たちは安易な命名に流れてしまいます。
-
時間的プレッシャーと「後で直す」という罠: 開発者はしばしば納期に追われています。「とりあえず動くテスト」を確保することが最優先され、命名のような「些細なこと」は後回しにされがちです。しかし、ご存知の通り、その「後で」は決してやって来ません。技術的負債として静かに蓄積されていきます。
-
思考のショートカット:
test+メソッド名のような単純なパターンは、何も考えずに素早く名前を付けられるため、安易に採用されがちです。「何をテストしているか」ではなく、「どのメソッドをテストしているか」という視点に陥ってしまっています。 -
「テストは開発者のためのもの」という誤解: テストは自分だけが読むもの、あるいはCIをパスさせるためだけのものだと考え、他の人（未来の自分、レビュアー、新メンバー）が読むことを想定していないため、命名に時間をかけるモチベーションが湧きにくいのです。
これらの背景を理解した上で、悪い例を見てみましょう。
悪い例
以下はレポートの項目として意味をなさないテスト名の例です。
これらの名前では、テストが成功したときに「何が」正常であることを保証するのか、全く分かりません。
将来、仕様変更でこのテストが失敗した場合、コードを読まないと期待結果を思い出せず、修正に時間がかかります。
// BadNamingSpec.groovy
import spock.lang.Specification
class OrderServiceSpec extends Specification {
// 悪い例1: 具体性がない。
def "test OrderService"() {
// ...
}
// 悪い例2: 期待する結果が書かれていない。何がOKなのか不明
def "calculateShippingFee with GOLD customer"() {
// ...
}
良い例
良いテスト名とは、[状況]において[何を]したとき、[どうなる]べきか という振る舞いを明確に記述したそれ自体が仕様となるものであるものです。
Spockフレームワークでは日本語のメソッド名が使えるため、これを活用することでよりわかりやすいテストコードになります。
// GoodNamingSpec.groovy
import spock.lang.Specification
class OrderServiceSpec extends Specification {
def "calculateShippingFeeメソッドは、顧客ランクがGOLDの場合、送料0を返す"() {
given:
def customer = new Customer("user-gold", "Gold Member", "GOLD")
def orderService = new OrderService()
when:
def fee = orderService.calculateShippingFee(customer)
then:
fee == 0
}
def "calculateShippingFeeメソッドは、顧客ランクがGOLDでない場合、送料500を返す"() {
given:
def customer = new Customer("user-silver", "Silver Member", "SILVER")
def orderService = new OrderService()
when:
def fee = orderService.calculateShippingFee(customer)
then:
fee == 500
}
}
Google Testing Blogでもテスト名はシナリオと期待結果を含めることは多くのメリットがあることを話しています。
テスト名は、そのクラスが持つ「責務」や「機能」を雄弁に語るべきです。メソッド名ではなく、クラスの振る舞いに焦点を当てることで、他の開発者はコードを読まなくてもそのオブジェクトの役割を理解できるようになるはずです。
"Make sure test names contain both the scenario being tested and the expected outcome."
まとめ
以上、いくつかテストを書く際に私が気をつけていて、またレビューなどで指摘する観点を上げてみました。
良いテストコードを書くことは、健全なソフトウェア開発に不可欠です。
それは単なる品質維持活動ではなく、システムの振る舞いを記述した、信頼できるエビデンスとレポートを構築する行為そのものです。
みなさんも以下の点を意識してみてください。
- DRYよりDAMP: テストは冗長なくらいがちょうど良い。可読性を最優先する。
- 1テスト1論理的概念: 1つのテストでは1つのことだけを検証する。
- テストの書きにくさは設計のバロメーター: テストが困難なら、安易にモックに頼る前に、まず設計を疑う。
- テストは名前で意図を語る: 個々のテスト名で、何を検証しているのかを明確にし、レポートとしての価値を高める。
これらの原則をチームで共有し、信頼性の高い「生きたドキュメント」としてのテストコードベースを共に目指しましょう。
楽天のエンジニアによる技術ブログです。このブログは公式ブログではなく有志で運営しています。 記事の内容は個人の見解であり、所属する企業や組織の立場、戦略、意見を代表するものではありません。
Discussion