前回まではspecの構造の話でした。今回は検証の中身です。
テストが通っていても、守られているのはexpectに書いた範囲だけです。書いていない部分は、変わっても落ちません。
カバレッジでは測れません。その行を通ってはいるので、カバレッジは上がります。通っただけで、何を確認したかは別の話です。
- have_attributesは書いた属性しか見ない
- JSONレスポンスもハッシュ全体を比較する
- super_diff(gem)がないと完全比較は現実的でない
- 消されたものも検知できない
- withとonceはチェーンしない
- まとめ
have_attributesは書いた属性しか見ない
スペースを作成するrequest specで、こう書いたとします。
expect(last_space).to have_attributes(
name: valid_attributes[:name],
description: valid_attributes[:description],
private: valid_attributes[:private],
created_user_id: user.id
)
一見きちんと検証しているように見えますが、列挙した属性しか見ていません。
後からカラムが増えて、そこに値がセットされるようになっても落ちません。実装した人は「カラムを1つ足した」だけで、specを直す理由がない。レビューでも、追加されたカラムとspecを突き合わせない限り気づけません。
「メンテを忘れなければいい」という話ではなくて、直す理由が発生しないのが問題です。
全カラムを対象にする
Space.new.attributesを土台にします。
let!(:start_time) { Time.current }
expect(last_space).to have_attributes(
Space.new.attributes.symbolize_keys.merge(
id: kind_of(Integer),
code: kind_of(String),
name: valid_attributes[:name],
description: valid_attributes[:description],
private: valid_attributes[:private],
created_user_id: user.id,
created_at: be_between(start_time.floor, Time.current.ceil),
updated_at: be_between(start_time.floor, Time.current.ceil)
)
)
土台が全カラムなので、mergeで上書きしていないカラムは「デフォルト値のまま」が期待値になります。
idは土台ではnilなので、必ず上書きすることになります。codeのように事前に値が分からないものも同じで、kind_ofで型だけ見ます。
code: last_space.codeのように検証対象自身の値を書くと常に通ってしまうので、そこは避けます。
形式まで検証する方法もありますが、生成規則をここに書くと、仕様変更のたびにこのspecも直すことになります。生成自体は別のspecの責務です。
カラムが追加された時、
- 値がセットされないなら、デフォルトのままなので通る
- 値がセットされるなら、落ちる
つまり「実装が新しく何かを書くようになった」瞬間に気づけます。DBのデフォルト値定義を二重管理せずに済むのもいいところです。
created_atとupdated_atも検証しています。Railsが勝手にセットするので不要に見えますが、暗黙の挙動こそ検証したいと思っています。update_columnやinsert_all、upsert_allのようにタイムスタンプを通さない書き方に変えた時、ここで落ちます。
更新の場合は更新前のレコードが土台
let!(:start_time) { Time.current }
let(:current_space) { Space.find(space.id) }
expect(current_space).to have_attributes(
space.attributes.symbolize_keys.merge(
name: valid_attributes[:name],
description: valid_attributes[:description],
private: valid_attributes[:private],
last_updated_user_id: user.id,
updated_at: be_between(start_time.floor, Time.current.ceil)
)
)
土台が更新前のレコードなので、変更していない属性が変わっていないことまで検証できます。意図しない副作用を検知できる。
updated_atの検証もここでは意味があります。Railsは値の変更がないとUPDATEを発行しないので、updated_atが更新されていることは実際に書き込みが発生した証拠になります。
start_timeはlet!で取る
be_betweenの下限に使っているstart_timeは、subjectの実行前に評価されている必要があります。
let!(:start_time) { Time.current }
letだとsubject実行後に評価されてしまうので、ここはlet!です。let_it_beを導入していればlet!を使う場面は多くありませんが、評価のタイミングそのものが検証内容になるケースだけは必要になります。
JSONレスポンスもハッシュ全体を比較する
同じ話がAPIのレスポンスにも当てはまります。
let(:response_parsed_body_sym) { response.parsed_body.deep_symbolize_keys }
expect(response_parsed_body_sym).to eq(
success: true,
notice: I18n.t('notice.space.update'),
space: {
code: current_space.code,
name: current_space.name,
description: current_space.description,
private: current_space.private,
member_count: 1,
destroy_requested_at: nil,
destroy_schedule_at: nil,
created_at: I18n.l(current_space.created_at, format: :json)
}
)
includeや個別キーの検証ではなくeqで全体を比較します。
- キーが増えた → 落ちる
- キーが減った → 落ちる
- ネストの構造が変わった → 落ちる
APIレスポンスは仕様書に書かれた外部との約束なので、増えることも破壊的変更になりえます。内部IDやメールアドレスが意図せず載るようなケース。includeだとこれを一切検知できません。
エラーメッセージも同じ
let(:messages) { { code: [I18n.t('activerecord.errors.models.space.attributes.code.taken')] } }
expect(model.errors.messages).to eq(messages)
includeだと、他のバリデーションも一緒に落ちていることに気づけません。「codeがtaken」を検証したつもりで、実は文字数制限にも引っかかっていた、という状態が通ってしまう。
eqなら、検証対象のバリデーションが単独で落ちていることまで保証できます。
文言は直接書かない
期待値に日本語を直書きすると、文言を変えただけでspecが落ちます。
notice: I18n.t('notice.space.update')
文言は仕様ではなく表示なので、そこで落ちてほしくない。キーで比較すれば、文言変更では落ちず、キーの変更では落ちます。多言語対応していれば、そもそもベタ書きは成立しません。
I18n.tを使うと実装とspecが同じ辞書を見るので、辞書の値そのものは検証していないことになります。ただ、そこはテストで守る範囲ではないと思っています。文言の間違いはレビューや動作確認で気づきますし、値をspecにコピーすれば二重管理になります。
super_diff(gem)がないと完全比較は現実的でない
ここまで書いておいて何ですが、完全比較には障壁があります。
落ちた時に、どこが違うのか分からない。
RSpecの標準の差分表示は、大きなハッシュだと実質読めません。expected:とgot:に長大なハッシュが並ぶだけで、どのキーが違うかを目で探すことになります。ネストしていると絶望的です。
super_diffがなかった現場で、既存のspecの比較が長すぎて、どこが落ちているのか確認するのに苦労したことがあります。
gem 'super_diff'
これを入れると差分だけが色付きで表示されます。ネストしていても、どの階層のどのキーが違うかが直接出る。
これを導入してから、書き方を変えました。 完全比較を避けてincludeや個別検証に逃げていたのは、読めなかったからです。検証の弱さが道具の制約から来ていた。
今なら長いdiffをAIに貼って違いを聞くこともできますが、CIのログは先に人間の目に入りますし、書いている最中は何度も落とします。毎回貼るのは現実的ではないので、gemを入れる方が先だと思います。
消されたものも検知できない
ここまでは「増えたものを検知する」話でしたが、逆方向もあります。
validates :name, presence: true
このバリデーションを消したとします。nameに値を入れているspecは、全部通ります。落ちるのは「nameがない」ケースを書いている場合だけです。
同じことがDBの制約や、権限チェックのifにも言えます。守っているものを消しても、そのケースを書いていなければ気づけません。
だからmodel specでは各バリデーションを網羅します。「そのバリデーションが効いていること」を確認するケースがないと、消えたことも変わったことも検知できない。
完全比較は「書いた範囲」を広げますが、そもそもケースがなければ守りようがありません。検証の深さとケースの網羅は、どちらも必要です。
withとonceはチェーンしない
非同期処理の呼び出しを検証する時の話です。
expect(SendWorker).to have_received(:perform_async).with('hoge').once
一見「'hoge'で1回呼ばれた」を検証しているように見えます。実際はwith('hoge')に一致する呼び出しが1回という意味です。
別の引数で呼ばれていても素通りします。
def call
SendWorker.perform_async('hoge')
SendWorker.perform_async('fuga') # これを検知できない
end
分けて書くと、perform_async全体が1回であることを担保できます。
expect(SendWorker).to have_received(:perform_async).with('hoge')
expect(SendWorker).to have_received(:perform_async).once
これで、意図しない呼び出しが増えた時に落ちます。
チェーンしていた時は緑でした。分けた瞬間に落ちて、そこで初めて気づいた。コードは変わっていないのにです。緑が保証していた内容が、書いた本人の理解とズレていました。
matcherは英語の文のように読めますが、修飾の係り方が英語の直感と違うことがあります。チェーンした時に何を検証しているのか、一度確認しておくといいと思います。
まとめ
同じ原則がいくつかの形で出てきました。部分検証は、追加も削除も検知できない。
have_attributesはModel.new.attributesを土台にして全カラムを対象にする- 更新の検証は更新前のレコードを土台にする。変えていない属性が変わっていないことまで見る
created_at/updated_atのような暗黙の挙動も検証する- JSONレスポンスとエラーメッセージは
eqでハッシュ全体を比較する - 文言はlocaleのキーで比較する。辞書の値そのものはテストで守る範囲ではない
- 守っているものを消しても、そのケースがなければ検知できない。検証の深さとケースの網羅は別
- 完全比較はsuper_diffとセット。入れないと落ちた時に読めない
withとonceはチェーンせず分ける。チェーンすると絞り込みになる
次回は、matcherの選び方を書きます。be_truthyの曖昧さや、HTTPステータスをシンボルで書くか数字で書くかの話です。
