前回は「書いた範囲しか守られない」という話でした。今回はもう少し細かく、どのmatcherを使うかという話です。
同じ検証でも、matcherの選び方で通る範囲の広さが変わります。広すぎると、壊れていても通ってしまう。

• be_truthy / be_falseyは広すぎる
  • nilを返す述語メソッド
  • 使い分けではなく、使わない
  • be_presentも広い
  • be true より be(true)
• 否定はmatcher側に寄せる
• HTTPステータスは数字で書く
  • :successはもっと広い
  • 可読性の反論について
• 順序を見るか、見ないか
• 検証する項目を絞る場合
• 件数は増減で見る
  • 複数の変化をまとめて見る
  • is_expectedはブロックの中に入れる
• まとめ

be_truthy / be_falseyは広すぎる

expect(user.admin?).to be_truthy

be_truthyはnilとfalse以外の全てを通します。trueでも1でも"false"という文字列でも通る。

実装がtrueを返す仕様なのか、truthyな何かを返す仕様なのかが、specから読めません。

nilを返す述語メソッド

be_falseyの方がもっと問題になります。nilもfalseも通すので、nilが返っていることに気づけません。

def admin?
  role&.admin?
end

roleがnilだとnilが返ります。書いた人は真偽値のつもりですが、?が付いたメソッドはtrueかfalseを返す、というのがRubyの約束です。

&.で書いた時や、present?・include?の結果を素通しにした時に混入します。

そして問題はspecの外にあります。nilを返すメソッドがJSONに載るとnullになる。falseを期待しているクライアントが壊れます。||の分岐も変わる。受け取る側がnilを許容しているかどうかは、そのメソッドを見ても分かりません。

be(false)にしておけば、nilが返った瞬間に落ちます。そこで実装を直すことになります。

# これも避けたい
def admin?
  !!role&.admin?
end

# こう書く
def admin?
  role&.admin? || false
end

!!でも真偽値にはなりますが、元の値が何であれ潰してしまうので、何を想定しているのか読めません。|| falseなら「nilのときはfalse」と書いてあります。

使い分けではなく、使わない

述語メソッドの検証はbe(true) / be(false)にしています。be_truthy / be_falseyは使いません。

「値の有無」を見たい場合はbe_present / be_nilと書けばいい。用途ごとに専用のmatcherがあるので、truthy / falseyに頼る理由がありません。

両方を通したい場面があるとすれば、falseが返るケースとnilが返るケースが混ざっているということです。それならcontextを分けて、それぞれbe(false)とbe_nilで書く方が正確です。

判断が要らなくなるのが大きいと思っています。述語メソッドならbe(true) / be(false)、値の有無ならbe_present / be_nil。迷う場面がありません。

be_presentも広い

be_truthyほどではありませんが、be_presentも広い部類です。

expect(space.name).to be_present

nilでも空文字でもなければ通るので、値が何であっても通ります。名前が入れ替わっていても、意図しない値が入っていても気づけない。

値が分かっているならeqで書きます。使うとすれば、値が事前に分からない場合くらいですが、そこはkind_ofの方が意図が明確です。

他で担保しているから「あればいい」で済ませる、という判断はありえます。ただそれは後述するhash_includingと同じで、どこかで完全比較していることが前提になります。

be true より be(true)

細かい話ですが、括弧を付けています。

# こう書く
expect(model.active?).to be(true)

# 避けたい
expect(model.active?).to be true

動作は同じです。Rubyが括弧を省略できるだけで、beというmatcherにtrueを渡している構造は変わりません。

括弧があると、matcherに引数を渡している構造が字面に出ます。be_truthyは引数を取らないmatcher、be(true)は引数を取るmatcher。この違いが見えた方が読み間違えにくい。

eq(200)やhave_attributes(...)、be_between(a, b)と書き方が揃うのもいいところです。

否定はmatcher側に寄せる

同じ検証でも、否定をどちらに置くかで変わります。

# こう書く
expect(model).to be_invalid

# 避けたい
expect(model).not_to be_valid

not_to be_validは「validでなければ何でもいい」に読めます。to be_invalidなら、invalidという状態を直接指しています。

読み手の視点でも、toとnot_toは字面が近くて見逃しやすい。be_validとbe_invalidなら、matcher名で区別がつきます。

状態を表すmatcherがあるなら、そちらを使う。後述するnot_changeも同じ考え方です。

なおnot_toとto_notはエイリアスで動作は同じですが、RuboCopのRSpec/NotToNotがデフォルトでnot_toを推すので、それに従っています。

HTTPステータスは数字で書く

# こう書く
is_expected.to eq(403)

# 避けたい
expect(response).to have_http_status(:forbidden)

実装側がこう書いているとします。

return head :forbidden

specも:forbiddenで書くと、実装とspecが同じものを参照していることになります。:forbiddenが403に対応するのはRackが決めていて、コードには書かれていません。

もしシンボルの対応が変わったら、実装とspecが同時に変わるのでテストは通り続けます。クライアントが受け取るステータスが変わっても気づけない。

数字で書けば、シンボルから数値への変換結果を検証していることになります。実装をhead :unauthorizedに書き換えたら403が401になって落ちる。シンボルの取り違えも検知できます。

API仕様書に書かれているのも、クライアントが見るのも403です。specの期待値は仕様書に合わせる。実装がどう表現しているかは実装の自由です。

it 'HTTPステータスが403'のような説明文とも揃います。シンボルで書くと、説明文の数字とコードのシンボルがズレることになる。

:successはもっと広い

シンボルの中でも:successは特に広い。

expect(response).to have_http_status(:success)

これは200番台なら全部通ります。200も201も204も区別しない。createで201を返すべきところが200になっていても気づけません。

eq(200)と書けば、そこがズレた時に落ちます。

なお、シンボルには2種類あります。:ok（200）や:created（201）は個別のコードを指しますが、:success / :redirect / :missing / :errorはグループ指定です。前者はRack::Utils::SYMBOL_TO_STATUS_CODE、後者は2xx・3xx・404・5xxに対応しています。

グループ指定を使う場面は、ほとんどないと思っています。201と204、301と302、500と503はそれぞれ意味が違うので、どれが返るべきかは仕様として決まっているはずです。

可読性の反論について

:forbiddenの方が読みやすい、という意見はあります。ただ、API開発者は403を読めます。

むしろ:unprocessable_entityが422だと即答できる人の方が少ない。シンボルの方が可読性が高いというのは、常に成立するわけではありません。

前回の「文言はlocaleのキーで比較する」と逆のことを言っているようですが、基準は同じです。仕様書に書かれている方で書く。文言は表示なのでキー、ステータスは仕様書に載るので数字になります。

順序を見るか、見ないか

配列の比較にはeqとmatch_arrayがあります。違いは順序を見るかどうかです。

expect(spaces).to eq([a, b, c])          # 順序も一致すること
expect(spaces).to match_array([a, b, c]) # 順序は問わない

一覧のAPIで並び順が仕様ならeqです。更新日時の降順で返すことが決まっているなら、順序が変わったら落ちてほしい。

逆に、絞り込みの検証で「この3件が返る」だけを見たいならmatch_arrayになります。並び順は別の観点なので、そちらのdescribeで見る。

その検証で順序が仕様かどうかで選びます。何となくmatch_arrayにしておくと、並び替えが壊れても気づけません。

検証する項目を絞る場合

前回「ハッシュはeqで全体を比較する」と書きましたが、常にそうするわけではありません。

絞り込みの検証では、返ってくるスペースを識別できれば十分です。

expect(response_parsed_body_sym[:spaces]).to match_array(
  expect_spaces.map { |space| hash_including(code: space.code) }
)

hash_includingでcodeだけを見ています。名前や説明文がレスポンスに含まれるかは、ここでの関心事ではありません。

これは手を抜いているのではなく、この観点が守るべき範囲を決めているということです。レスポンスの項目が正しいかは別のdescribeで完全比較していて、そちらで担保されています。同じことを2箇所で検証すると、1つの変更で両方落ちます。

逆に言うと、他のどこかで完全比較していることが前提です。どこでも完全比較していないのにhash_includingで済ませていると、単に検証が弱いだけになります。

件数は増減で見る

レコードが作られたことを検証する時の話です。

expect(Space.count).to eq(1)

これは実行前が0件だった前提に依存しています。テストデータが増えたり、let_it_beで共通データを作るようになったりすると落ちます。実装は何も壊れていないのに。

expect { subject }.to change(Space, :count).by(1)

増減で見れば前提に依存しません。

by(1)まで書くのも大事です。change(Space, :count)だけだと、増えても減っても通ります。1件作るつもりが2件作られていても気づけない。

fromとtoで開始値と終了値を書く方法もありますが、件数には向きません。from(0)はテストデータが増えれば崩れるので、eq(1)と同じ問題を抱えます。

ステータスの遷移のように、数値でない属性を見るときはtoを使います。

expect { subject }.to change(download, :status).to('success')

fromは開始値の確認なので、自分は書きません。それは前提であって、このケースで検証したいことではない。ただ、書いてあるものをレビューで指摘するほどではないと思っています。

件数はby、属性の遷移はto と使い分けています。

「変わらないこと」もby(0)で書けば形が揃うのですが、RuboCopのRSpec/ChangeByZeroでnot_to changeに直されます。ここは素直に従っています。

複数の変化をまとめて見る

ただ、not_toには制約があります。「Aは増えるがBは増えない」を1つのexpectで書こうとしても、andの後ろにnot_toは置けません。andはpositiveなmatcherしか受け取らないためです。

そこでrails_helper.rbに否定版を定義しました。

RSpec::Matchers.define_negated_matcher :not_change, :change

expect { subject }.to change(Space, :count).by(1)
                  .and not_change(Member, :count)

単独で「変わらないこと」を見るならnot_to changeで足ります。andで繋げたい時だけ、この否定版が必要になります。

itを分ければnot_toで書けますが、subjectが2回実行されます。

is_expectedはブロックの中に入れる

expect { subject }で囲むと、subjectの実行がchangeのブロックの中になります。is_expectedを先に書くとそこでsubjectが評価されてしまうので、changeが変化を検出できません。

ブロックの中に入れれば両方書けます。

it '作成される' do
  expect do
    is_expected.to eq(200)
  end.to change(Space, :count).by(1)
end

ただし、これが使えるのはsubjectが正常に値を返す場合だけです。raise_errorのように例外を検証するときはsubjectが値を返さないので、is_expectedは使えません。

it 'エラーになる' do
  expect { subject }.to raise_error(SomeError)
end

件数も見るなら、前述のnot_changeと組み合わせます。

it 'エラーになる' do
  expect { subject }.to raise_error(SomeError)
    .and not_change(Space, :count)
end

まとめ

• be_truthy / be_falseyは使わない。nilを通すので、述語メソッドがnilを返していても気づけない
• 述語メソッドはbe(true) / be(false)。値の有無はbe_present / be_nil
• be_presentも値は見ていない。値が分かっているならeq
• be trueではなくbe(true)。matcherに引数を渡している構造を字面に出す
• 否定はmatcher側に寄せる。not_to be_validよりto be_invalid
• HTTPステータスは数字。実装がシンボルなら、specは変換結果を検証する形になる
• :successは200番台を全部通す。201や204との違いが消える
• 配列は順序が仕様ならeq、そうでなければmatch_array
• hash_includingで項目を絞るのは、他で完全比較している場合だけ
• 件数はchangeで増減を見る。eqは実行前の状態に依存する
• byまで書く。changeだけだと増えても減っても通る
• 複数の変化をまとめるにはdefine_negated_matcherでnot_changeを定義する
• changeとis_expectedを併用するなら、is_expectedをブロックの中に入れる

次回は、テストデータの話を書きます。buildとcreateの使い分け、let / let! / let_it_beです。