ここまでは、テストで何をどう守るかの話でした。今回は書き忘れても落ちないものです。
書き忘れると、落ちずに通ってしまうものがあります。緑なので誰も気づかない。気をつけるしかないように見えますが、多くは落ちる仕組みを埋め込むことで解決できます。

• mockは実装をなぞる
  • mockを使う立場もある
  • controller specがrequest specに変わったのも同じ流れ
• mockが必要な場面
  • WebMockは手書きmockと性質が違う
  • mockする場所を減らす
  • any_instance_ofは使わない
  • 境界の把握を間違えると破綻する
• ジョブの呼び出しは落ちるようにする（おすすめ）
  • 呼び出し側とジョブ側で分かれる
• Mailer Previewの追加漏れを検知する（おすすめ）
• Mailer Previewの壊れを検知する（おすすめ）
• 想定外の分岐でraiseする
  • 増える見込みがあるならcase
• カバレッジの位置づけ
• まとめ

mockは実装をなぞる

先にmockの話をします。前回「テストしやすいコードを書くのが原則」と書きましたが、mockはそれを覆い隠す道具として使われがちです。

allow(service).to receive(:send_message).with(user, body)

自分のクラスのメソッドにmockを張ると、実装の呼び出し方を固定することになります。メソッド名を変えたら? 引数の順序を変えたら? mock側も一緒に直せば緑のまま通ります。本物が呼ばれないので、実際に壊れていても検知できない。

verify_partial_doublesが有効なら、実在しないメソッドへのstubは落ちます。rspec --initで生成される設定に含まれているので、通常は有効になっているはずです。無効にしていると、削除済みのメソッドを相手に検証が通ってしまうので、確認しておいた方がいい。

ただし守れるのは存在の有無までで、引数が意味的に正しいかは分かりません。メソッド名も引数も合っているが、渡している値が間違っている、というケースは通ります。

「実装をなぞるテストは意味がない」の典型で、リファクタの瞬間に落ちるか、落ちないまま嘘になるかのどちらかです。どちらに転んでも目的に反します。

mockを使う立場もある

単体テストの原則として、「テスト対象以外は隔離する」という考え方があります。依存先の実装に引きずられないよう境界を切る。テストが速くなり、失敗の原因も絞られる。

この立場からすると、自分のクラスにmockを張るのは正しい設計判断です。何を「単体」とみなすかが違うだけだと思っています。

• クラス単位を単体と見るなら、他のクラスはmockで隔離する
• 機能単位（リクエストからレスポンスまで）を単体と見るなら、mockは境界だけでいい

自分は後者です。理由は1回目に書いた目的で、手動テストの代替として書いているからです。手動で確認していたのは画面の表示や保存されたデータで、誰も内部のメソッド呼び出しは確認していません。

controller specがrequest specに変わったのも同じ流れ

かつてのcontroller specは、assignsでインスタンス変数を覗いたり、render_templateでどのテンプレートが呼ばれたかを見たりしていました。コントローラの内部を直接検証していたわけです。

Rails 5でこれらは標準から外れ、rails-controller-testing gemに切り出されました。合わせてrequest specが推奨されるようになります。内部を覗くのをやめて、レスポンスという外形で見るようになった。

Rails自体が同じ判断をしている、という傍証だと思っています。

この考え方はrequest spec固有のものではありません。rake taskなら標準出力やDBの最終状態、ログの件数。jobなら処理の結果と副作用。内部を覗かず、外に出たもので見るという点は共通です。

mockが必要な場面

本物を呼ぶと副作用が出るものは、mockを避けられません。

• 外部APIの呼び出し（課金、レート制限、データの更新）
• 非同期処理のキュー投入

specから外部にリクエストが飛ぶのは、そもそも踏んではいけない線です。更新系が飛べばデータが壊れますし、従量課金のAPIをCIが並列で叩けばコストになる。接続元が制限されれば、テストと無関係に本番が止まることもある。

WebMockを入れれば、デフォルトで外部への接続は例外になります。VCRを使っている場合は、記録のために一度実リクエストが飛ぶので注意が要ります。

WebMockは手書きmockと性質が違う

allow(client).to receive(:post)

これは自分のコードが何を渡そうと素通りします。

WebMockはHTTPのレイヤで待ち構えています。gemが実際にリクエストを組み立て、URL・ヘッダ・bodyができあがったところで照合される。だからgemのバージョンアップで送信内容が変わったら落ちます。

stub_request(...).with(body: ...)まで書いていれば、gemが付けるパラメータが増減した時点で検知できる。手書きmockでは絶対に得られない性質です。

記述量は増えますが、それは弱点ではなく検知能力の裏返しです。細かく書くほど落ちる条件が増える。前回書いた「gemはブラックボックスで押さえる」と同じで、記述量が安全を買っています。

withにどこまで書くかは、それが仕様かどうかで決めます。APIに送る認証ヘッダや必須パラメータは仕様なので書く。gemが付けるUser-Agentは仕様ではないので書かない。4回目のonceの話と同じ判断です。

落ちた時にどちらなのかは、自分で判断することになります。

• gemが正当な変更をした（追従する）
• 自分の使い方が壊れた（直す）

差分を見れば分かる話ですし、気づけること自体が目的です。検知できなければ、本番で初めて分かることになります。

mockする場所を減らす

外部との通信が散らばっていると、その数だけmockを書くことになります。lib配下にクライアントクラスを切り出せば、

• WebMockを丁寧に書く対象はそのクラスのspecだけ
• 呼び出し側は自分のクラスを相手にするので、mockせずに済むことが多い

mockの記述量は設計で決まります。これも「テストしやすいコードを書く」の一例です。

ただし責務で分けるのが先で、テストしやすさは結果です。逆にするとテストのために設計を歪めることになります。

any_instance_ofは使わない

クラスの全インスタンスにmockを張る書き方があります。

allow_any_instance_of(User).to receive(:admin?).and_return(true)

テスト対象の内部でUser.findのようにインスタンスが作られると、外から掴めません。そこで全部に張ってしまう、という使い方です。

RSpec公式も非推奨としていますが、問題は対象を特定できないことです。同じ処理の中で別のUserを扱っていても、全部がadmin?でtrueを返します。意図していないところまで置き換わる。

have_receivedで確認しようとしても、どのインスタンスが呼ばれたのかは区別できません。

そして、そもそもインスタンスを外から渡せないこと自体が設計の問題です。引数で受け取るようにするか、メソッドを分けるかで解決できることが多い。any_instance_ofは、それを隠してしまいます。

境界の把握を間違えると破綻する

mockは接続点を正しく把握できていることが前提です。

境界だと思って張った場所が、実は境界でなかった場合。そこから先の処理は一切検証されないまま通ります。しかもmockが期待通りの値を返すので、緑になる。

外部APIやジョブの投入なら境界は明確です。プロセスの外に出るところなので、間違えようがない。

危ないのは自分のコードの中に境界を引く時です。「このクラスは十分に検証されているから」と思ってmockを張ると、その判断が間違っていた時に検知できません。境界の判断そのものは、どのspecも検証してくれない。

ジョブの呼び出しは落ちるようにする（おすすめ）

Sidekiqを使っている場合、perform_asyncにmockを張り忘れると、developmentのキューに実際に積まれます。Redisに書かれるので、workerが動いていれば実行される。しかもspecは緑のまま通ります。

設定で積まれないようにすることもできますが、それだけだとmockの書き忘れは検知できません。

そこで、mockを張り忘れた時点で落ちるようにしています。「気をつける」を「落ちる」に変えるということです。

RSpec.configure do |config|
  config.before(:suite) do
    Sidekiq::Job::ClassMethods.prepend(SidekiqStrictMode)
  end
  config.before do
    Sidekiq::Job.clear_all
  end
end

module SidekiqStrictMode
  def perform_async(*)
    raise_mock_required("#{name}.perform_async")
  end

  def perform_in(*)
    raise_mock_required("#{name}.perform_in")
  end

  def perform_at(*)
    raise_mock_required("#{name}.perform_at")
  end

  private

  def raise_mock_required(method_name)
    message = "#{method_name} が呼び出されています。Mockを使用してください。"
    p "[WARNING] #{message}" # NOTE: 例外を握り潰していると気付けない為
    raise message
  end
end

fakeモードにすればRedisには積まれませんが、それだとmockを書き忘れても通ってしまいます。副作用は防げても、規約違反は検知できない。塞ぐことと気づかせることは別です。

呼び出し側とジョブ側で分かれる

非同期処理は、呼び出し側の責務がジョブ投入までで切れます。

• 呼び出し側のspec — enqueueされたことだけ見る
• ジョブ側のspec — 実際の処理を見る

前回書いた移譲と同じ形です。境界が実装によって決まっているので、テストの分割は設計の反映であって、テスト都合の分割ではありません。

呼び出し側では、mockを張った上でhave_receivedで確認します。

expect(SendWorker).to have_received(:perform_async).with('hoge')
expect(SendWorker).to have_received(:perform_async).once

withとonceを分けて書くのは、チェーンすると「with('hoge')に一致する呼び出しが1回」になって、別の引数で呼ばれていても素通りするからです。

Mailer Previewの追加漏れを検知する（おすすめ）

Mailer Previewは開発時に見るものなので、無くても本番には影響しません。ただ、メールを修正した時にPMやCSが文面を確認するのに使います。本番に影響しない分、Mailerにメソッドを足して、Previewを書き忘れても気づけません。AIに実装させても、Previewまでは作ってくれないことが多いです。

Mailer側から回して、対応するPreviewがあるかを確認します。

# spec/mailers/previews_spec.rb
require 'rails_helper'

RSpec.describe ActionMailer::Preview, type: :mailer do
  # NOTE: test 環境は show_previews が無効で preview_paths に登録されず、Preview.all が空になる為
  Dir[Rails.root.join('spec/mailers/previews/**/*_preview.rb')].each { |file| require file }

  describe 'Preview の網羅' do
    let_it_be(:previewed_actions) do
      described_class.all.flat_map do |preview_class|
        preview_class.emails.map do |email|
          delivery = preview_class.call(email)
          [delivery.mailer_class, delivery.action.to_s]
        end
      end
    end

    mailer_classes = Dir[Rails.root.join('app/mailers/**/*.rb')].map do |file|
      Pathname.new(file).relative_path_from(Rails.root.join('app/mailers')).to_s.delete_suffix('.rb').camelize.constantize
    end
    mailer_classes.each do |mailer_class|
      mailer_class.action_methods.each do |action|
        context "#{mailer_class}##{action}" do
          it 'Preview が存在する' do
            expect(previewed_actions).to include([mailer_class, action])
          end
        end
      end
    end
  end

  describe 'Preview のレンダリング' do
    # 次の節で説明
  end
end

ポイントが3つあります。

test環境ではPreview.allが空になります。 show_previewsが無効なので、Previewのファイルが読み込まれません。requireしないと一覧が空のまま比較することになり、全部落ちるか、次の節のspecではケースが0件のまま緑で通ります。この記事の主題そのものの罠です。

Mailer側から列挙しています。 Previewから回すと、存在するPreviewしか対象になりません。Mailerの全アクションを起点にすれば、書き忘れた分が落ちます。

previewed_actionsはlet_it_beにしています。 全Previewを呼んで一覧を作るので、letだとMailerのアクションの数だけ作り直すことになります。

Mailer Previewの壊れを検知する（おすすめ）

Previewがあっても、壊れていたら意味がありません。メール本文の実装を変えて、Previewが例外を投げるようになっていても、確認しようとした時まで気づけません。

同じファイルで、全Previewを実際に呼び出します。

describe 'Preview のレンダリング' do
  described_class.all.each do |preview_class|
    preview_class.emails.each do |email|
      context "#{preview_class}##{email}" do
        it '例外が発生しない' do
          expect { preview_class.call(email).encoded }.not_to raise_error
        end
      end
    end
  end
end

.encodedまで呼んでいます。 メール本文のレンダリングは遅延されるので、生成しただけではテンプレートのエラーを拾えないことがあります。.encodedで本文を組み立てるところまで確認します。

どちらもケースを動的に生成しているので、MailerやPreviewを足しても書き足す必要はありません。1回目の記事に書いた「網羅を確認する」を、構造そのものに任せている形です。

想定外の分岐でraiseする

caseにelseがないと、どれにも当てはまらなかった時にnilが返ります。静かに素通りする。

case download.model.to_sym
when :member
  count = expect_space_basic_json(data, download.space)
  expect(data.count).to eq(count)
else
  # :nocov:
  raise "model not found.(#{download.model})"
  # :nocov:
end

elseでraiseしておけば、モデルが増えて分岐を足し忘れた時に落ちます。

なお、この形はAIには書いてもらえません。指示された分岐だけを書いて、elseは省くか正常系で埋めてしまいます。

到達しないコードなのでカバレッジ上は赤くなりますが、# :nocov:というマジックコメントで対象外にできます。「ここは検証しない」という意思を明示しているわけです。カバレッジの数字を下げずに、書かない判断を残せます。

増える見込みがあるならcase

分岐が2つだとifとelseで書きたくなります。ただそうすると、elseが正常系で埋まります。

3つ目が増えた時、elseに落ちて間違った処理が静かに走ることになる。raiseを置く場所がありません。

caseにして正常系を全てwhenに書けば、elseが想定外用として空いたままになります。

上の例は分岐が1つですがcaseにしています。ベースアプリのmodelなので、種類が増える可能性が高いからです。1つしか取らないことが確定しているなら、早期にraiseする書き方でも構いません。

カバレッジの位置づけ

カバレッジ100%を目指すと、前節のelseを通すためだけに不正なデータを作ることになります。そのテストが守るのは「raiseすること」だけで、仕様ではありません。カバレッジの数字とテストすべき範囲は別です。

とはいえカバレッジ自体は必要だと思っています。業務だとコストがかかって難しいこともありますが、感覚的には95%くらいは欲しい。gemやRailsのバージョンアップを安全にできるからです。通っていない行が多いと、全specが緑でも上げていいか判断できません。

ただし、カバレッジ100%が意味するのは「全部の行が通った」ことだけです。仕様通りに動くことの指標ではありません。 4回目に書いた通り、have_attributesで1項目しか見ていなくてもその行は通ったことになる。

• カバレッジ = 通った範囲
• 検証の深さ = 1ケースの中で何を見ているか

両方必要で、片方では代替できません。

まとめ

• 自分のクラスにmockを張ると実装をなぞることになる。リファクタで落ちるか、嘘になるか
• 対立点は「何を単体とみなすか」。手動テストの代替として書くなら、機能単位で見る
• controller specがrequest specに変わったのも、内部を覗くのをやめて外形で見る流れ
• 外形で見るのはtaskやjobでも同じ
• mockが要るのは副作用が出るもの。外部APIと非同期処理
• specから外部にリクエストは出さない。WebMockを入れればデフォルトでそうなる
• WebMockはHTTPレイヤで照合するので、gemの変更を検知できる。記述量が安全を買っている
• withに書くのは仕様だけ。gemが付けるものは書かない
• mockの記述量は設計で決まる。境界を1箇所に寄せる
• any_instance_ofは対象を特定できない。設計の問題を隠してしまう
• 境界の判断を間違えると、その先が検証されないまま通る
• ジョブの投入はraiseさせて、mock忘れを落ちるようにする
• Mailer側から回して、Previewの追加漏れを検知する
• 全Previewを呼び出して、壊れを検知する。ケースは動的に生成できる
• 呼び出し側はenqueueされたことだけ、ジョブ側は処理を見る
• caseのelseでraiseする。増える見込みがあるなら分岐が少なくてもcaseにする
• # :nocov:で「ここは検証しない」という意思を明示する
• カバレッジは通った範囲、検証の深さは別。両方必要

次回が最終回です。AIがspecを書く時代に、何をレビューして何を仕組みに落とすかを書きます。