🎃

RSpecでallow_any_instance_ofをinstance_doubleに置換したらテストカバレッジが下がった

に公開

背景

RSpecでテストを高速化する一環で、allow_any_instance_ofを駆逐すべく、instance_doubleに置換したら、テストカバレッジが下がってあたふたした話です。

allow_any_instance_ofが非推奨な理由

まず前提として、allow_any_instance_ofは以下の理由から使用が非推奨とされています。
公式ドキュメントによれば、

This feature is sometimes useful when working with legacy code, though in general we discourage its use for a number of reasons:

The rspec-mocks API is designed for individual object instances, but this feature operates on entire classes of objects. As a result there are some semantically confusing edge cases. For example, in expect_any_instance_of(Widget).to receive(:name).twice it isn’t clear whether a specific instance is expected to receive name twice, or if two receives total are expected. (It’s the former.)
Using this feature is often a design smell. It may be that your test is trying to do too much or that the object under test is too complex.
It is the most complicated feature of rspec-mocks, and has historically received the most bug reports. (None of the core team actively use it, which doesn’t help.)

要は、「allow_any_instance_ofはレガシーコードを扱う時には便利なこともあるけど、

  • クラス全体のオブジェクトをモックすることで、設計を不明瞭にしたり
  • そもそもこの機能を使わないといけないテストを書いてるってことは、設計に不備があったり
  • モックする機能の中でも複雑で、バグを多発させたりする

だから、使うのはオススメしないよ」ということです。

テストの種類によるカバレッジの変化

今回、SQS(アップロードされたデータを一時的に保存するAWSのサービスです)へのメッセージ送信を行うクラスを例に、3つのパターンで比較しました。

対象のコード

class SqsExportService
  def initialize(user_id:, file_name:, export_type:)
    @user_id     = user_id
    @file_name   = file_name
    @export_type = export_type
    # 初期化時に複雑な処理がある想定
  end

  def call
    # 外部通信を伴う処理
    send_message
  end

  private

  def send_message
    # AWS SDKなどを用いた処理
    true
  end
end

1. allow_any_instance_ofを使った場合

# 本物のインスタンスが生成され、initializeが実行される
allow_any_instance_of(SqsExportService).to receive(:send_message).and_return(true)
  • カバレッジ:高
  • 理由:SqsExportServiceを呼び出した際に、初期化処理を行うinitialize内のコードが実行され、send_messageのみがモックに差し代わる(必ずtrueを返す)。initializeを通過するため、テストカバレッジは維持される。

2. instance_doubleを使った場合

sqs_stub = instance_double(SqsExportService)
allow(SqsExportService).to receive(:new).and_return(sqs_stub)
allow(sqs_stub).to receive(:send_message).and_return(true)
  • カバレッジ:低
  • 理由:SqsExportService.newを呼び出すと、本物のインスタンスを作らず、偽(double)のインスタンスを返す。つまり、initializeメソッドの中身は一切実行されず、テストカバレッジは下がる。
  • 一方で、外部通信や重い計算を伴うinitizalizeをスキップでき、テストが高速化するメリットもある

3. and_wrap_originalを使った場合
最終的にand_wrap_originalを使う方向で落ち着きました。

allow(SqsExportService).to receive(:new).and_wrap_original do |original_method, *args, **kwargs|
  # 本物のinitializeを呼び出してインスタンスを生成
  instance = original_method.call(*args, **kwargs)
  # 特定のメソッドだけをスタブ化
  allow(instance).to receive(:send_message).and_return(true)
  instance
end
  • カバレッジ:高
  • 理由:original_method.callで、initializeメソッドを呼び出してインスタンスが生成される。この場合はinitializeを通過するため、テストカバレッジは維持される。
  • 生成されたインスタンスに対してのみallowでスタブ可能なため、他のテストにもモックが漏れる心配がない

まとめ: 3つのテスト手法の比較と使い分け

手法 推奨度 カバレッジ 特徴 最適なユースケース
allow_any_instance_of 全インスタンスを書き換える。副作用が強く、Flaky Testの原因。 既存のレガシーコードで、リファクタリングが困難な時のみ。
instance_double 完全に「偽物」に置き換える。高速で、インターフェースの検証も可能 純粋なユニットテスト。初期化に重い通信やDB処理があり、そこを切り離したい時。
and_wrap_original 本物の初期化を動かしつつ、一部だけスタブ化する 結合テスト寄り。初期化の引数代入なども含めて検証したい時。

今回はテストカバレッジの観点でしか見ていないですが、本来テストコードはエンジニアが安心して開発できるようにするためのものです。例えば今回であれば、

  • インスタンスの中身に関係なく行える単体テストはinstance_doubleを使って、高速に回す
  • サービス/結合テスト層はand_wrap_original を使い、オブジェクト同士の繋がり(初期化〜実行)を本物に近い形で保証する

といったこと踏まえて、テストカバレッジを「ゴール」とせずに良いテストを書いていきたいものです。

Atrae Tech

Discussion