Rspec 3.5についての記載となります。

documentにallow-any-instance-of/expect-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.)

working-with-legacy-code/any-instance

特に最後の「rspec-mocksの中で最も複雑で、過去最もバグ報告を受けた機能で、開発チームも積極的には使ってない」って書いてあるくらいにほっとかれている機能です。そもそも下位互換のために残されただけのようですね。

使うな、とは言ってもどーしようもない時もありますし、大抵知っているとそれを使える前提で考えてしまうもの。 本来は上記にも書かれている通り、テスト設計がマズいんでしょうけど…(Using this feature is often a design smell.) あと、テストファーストの話ではなく既存案件にテストコードを追加した時の話です。

ということで、allow-any-instance-ofを利用して遭遇した不可思議な現象と、別の方法で試してみました。

ちなみにallowexpectは全く別のもの。違和感なく使えるように機能やインタフェースは共通化されていますが(allowをexpectに変えても大抵はうまくいく)、複雑なことをやろうとすると大抵失敗します(そんなこと試すってことはテストが too complex って証拠だろうけど)

現象

require './spec/spec_helper'
class A
attr_reader :status
def start
_method({type: 1})
return 1 unless @status
_method({type: 2})
return 2 unless @status
3
end
def _method(params)
puts 'called!'
# 何かしらの処理の結果をセットする。例えばHTTPのPOSTリクエストに成功した場合など
do_something_result = true
@status = do_something_result ? true : false
end
end
class B
def self.start
# 実際は起動するまでにいろいろやってる
A.new.start
end
end

classAとclassBがあり、classAのstartメソッドのテストを実施したい。 classAの_methodメソッドは、例えばHTTPのPOSTリクエストを行うようなメソッドで、その処理結果を@statusにセットする仕様とする。 一方テスト対象のclassAのstartメソッドは、引数を変えて_methodを呼び出しその都度@statusの結果をチェックしてreturnする仕様とする。

_methodメソッドの引数と処理結果によってstartメソッドの動作が変わるので_methodメソッドをmockに置き換えてテストしたいが、_methodの処理結果は戻り値を利用せずインスタンス変数にメソッド内でセットされるのでそのままallowでメソッドを置き換えることができない。

とりあえず、戻り値のテストをどう実装するかは置いておいて、引数のチェックだけを行う場合に限ると普通にallow使えば下記のようにチェックできます。

describe A do
context 'allowを使う' do
let(:target) { A.new }
before :each do
# 同じメソッドを引数を変えてテストするときには、とりあえず受け入れられるように引数指定なしでallowする必要がある
allow(target).to receive(:_method).and_call_original # 実際はand_call_originalは不要ですがこれがこの後の不具合の要因に。
end
it '引数のチェック' do
expect(target).to receive(:_method).with({type: 1})
expect(target).to receive(:_method).with({type: 2})
#expect(target).to receive(:_method).with({type: 3}) # => 予想通りテスト失敗する
expect(target.start).to eq 3 # => これも成功する
end
end
end

ただ、実際のテスト対象クラスでは、例えばincludeしたmoduleのメソッドを継承元で利用して処理の判定に利用したり・・・とテストしたい対象クラスを起動するまでが複雑で、本クラスだけ取り出して全部mock化するにはかなり時間がかかりそうとか、もう設計がおかしいしテスト設計もおかしいんだけど。

今回は目の前の問題を早くクリアすることが大正義だったので、最短ルートを突き進むこととします。
いつもの泥沼パターン

調べてみるとclassBで起動していることが判明したので起動自体はB.startで行うこととし、allow, expectしていた箇所を軒並み allow_any_instance_of, expect_any_instance_ofに書き換えます。

describe A do
context 'allow_any_instance_ofを使う' do
before :each do
allow_any_instance_of(A).to receive(:_method).and_call_original
end
it '引数のチェック(and_call_originalを呼ばない)' do
expect_any_instance_of(A).to receive(:_method).with({type: 1}) # => ★wrong number of arguments (given 2, expected 1)
expect_any_instance_of(A).to receive(:_method).with({type: 2})
#expect_any_instance_of(A).to receive(:_method).with({type: 3}) # => 前のエラーで実行されない
expect(B.start).to eq 3
end
it '引数のチェック(and_call_originalを呼ぶ)' do
expect_any_instance_of(A).to receive(:_method).with({type: 1}).and_call_original
expect_any_instance_of(A).to receive(:_method).with({type: 2}).and_call_original
#expect_any_instance_of(A).to receive(:_method).with({type: 3}).and_call_original # => 予想通りテスト失敗する
expect(B.start).to eq 3
end
end
end

理論上これで動くはず・・・が、実際は上記★の部分で、想定外の引数の数のエラーが出てしまいます。

一応 it '引数のチェック(and_call_originalを呼ぶ)' doのようにand_call_originalをつけると上手く動くことは確認できたのですがなぜ動いたのかはrspec/mockのソースとにらめっこしてもわかりませんでした・・・。 そこでタイトルの“rspecでallow-any-instance-ofは使わない方がよい、が身に沁みた”わけです。

allow-any-instance-of/expect_any_instance_ofを使わないとすると、別の方法を考える必要があります。 ググったりしたけど、newを置き換えてテスト用のmockに差し替えるのが一番しっくりきました。 あと、ついでに_methodの処理結果問題も素直に解決できました。

describe A do
let(:mock) { A.new }
before :each do
# ここがポイント
allow(A).to receive(:new).and_return(mock)
end
it 'expectを使う' do
allow(mock).to receive(:_method).and_call_original
expect(mock).to receive(:_method).with({type: 1})
expect(mock).to receive(:_method).with({type: 2})
expect(B.start).to eq 3
end
it 'type: 1の時は成功して, type: 2の時は失敗する' do
allow(mock).to receive(:_method) do |param|
mock.instance_eval do
@status = param[:type] == 1
end
end
expect(B.start).to eq 2
end
end

引用ページにも書かれているように「rspec-mocks APIは独立したオブジェクトインスタンスで動作するように設計されている」(The rspec-mocks API is designed for individual object instances)わけなので、その用途で使うときちんと動くわけですね。

最初からテスト意識して実装しないといけないなぁ+既存案件に後付けでテスト実装する場合は予想外に時間がかかる、ということを身を持って感じたのでした・・・