https://esminc.doorkeeper.jp/events/138417 で使用したスライドです
https://github.com/colorbox/rspec-all_records_validator
バグから生まれたgemの話失敗から学ぶ技術失敗から学ぶ技術@color_box
View Slide
rspec-all_records_validatorのはなし
プロジェクトで出たバグの話プロジェクトで出たバグの話
ある日突然の問い合わせある日突然の問い合わせ
「なんか更新機能動かないよ?」「なんか更新機能動かないよ?」
なぜか特定レコードがinvalidになっていた
モデル構造モデル構造class Binder < ApplicationRecordhas_many :documentsvalidates :documents, length: {minimum: 1}endclass Document < ApplicationRecordbelongs_to :binderend
Binderからdocumentsに対する関連が消失has_many関連のlengthに対する最小値のバリデーションによりinvalidに
バリデーションをすり抜けるはなしバリデーションをすり抜けるはなし
発生原因発生原因結論から書くとバグ
問題のバリデーション問題のバリデーション関連の個数に対するバリデーションはバグですり抜けうるvalidates :documents, length: {minimum: 1}
再現コード再現コードbinderをそれが持つ書類ごとコピーしようとして下記のようなコードを書いてしまうoriginal_binder = binder.find(XXX)copied_documents = original_binder.documents.each do |documentdocument.dupendBinder.create!(documents: copied_documents)
バグバグオリジナルのbinderのdocumentsをコピーするeachを使用したためコピー元のdocumentsが移動map等を使用すべき箇所だったcopied_documents = original_binder.documents.each do |documentdocument.dupend
バリデーションがあるのにすり抜けてしまう
同じフローを実行するE2Eテストは存在検出できなかった
テスト内でoriginal_binderに対するバリデーションは実行されていれば検出できた
新規に作製されたレコードについてのみ実行され古いレコードはバリデーションされない
original_binderでバリデーションが実行されないためすり抜けが発生すり抜けが発生
自動で検出できると嬉しい
こういうすり抜けをなんとかしたいこういうすり抜けをなんとかしたいテスト終了時にDB内のすべてのレコードに対してvalidationをかけることで検出できそう
テスト終了時に全レコードのバリデーションチェック
Q.Q.特定レコードがinvalidになることを確認するテストはどうする?
A. 無視するA. 無視する
featureやsystemのようなE2Eテストのみを対象とするバグでない限りユーザー操作を模倣したテスト終了時にinvalidなデータが発生しているのはおかしい
初手、プロトタイプ、検証初手、プロトタイプ、検証ひとまずざっくりRSpecのafterフックに仕込むconfig.after doApplicationRecord.subclasses.all? {|klass| klass.all.each {|obj| expect(obj).to be_valid }}end
雑に仕事プロジェクトに導入雑に仕事プロジェクトに導入テスト用のコードなので本番環境に入らない本番には影響がなく開発/テスト環境に閉じる最悪取り外せば良い
判明した問題判明した問題default_scopeを使用していると正常なユーザー操作後でもinvalidになるレコードが大量発生する大量発生するCI実行時間が1.5倍に1.5倍に
default_scopedefault_scopeの問題の問題default_scopeを使用しているとそのモデルと関連するレコードがinvalidになるのは不可避不可避
レコード総チェックの中で特定のモデルを避けたい避けたい
実行時間の問題実行時間の問題全モデルをチェックする上で不要なモデルまで処理してそうfixtureで投入するマスターデータなどはバリデーション不要のはず
モデルのフィルタ機能追加モデルのフィルタ機能追加default_scope絡みでinvalidになるレコードを除外バリデーション不要なレコードを除外して高速化
リリースまでリリースまで概ね機能はできたのでgemとしてリリースしたいテストは絶対必要
テストどうしようテストどうしよう書きづらい
feature specもしくはsystem spec終了時のみ実行したいコードのテストRSpecのafterでのみ実行されるコードをRSpecで実行する?expectが失敗することをテストする?expect in expect問題
どうやってテストする?
分割して対処分割して対処本質でない箇所は一旦無視「全レコードに対してバリデーションを実行」という部分のみテストする
レコードを用意してバリデーションをかけるための場が必要擬似的なRailsプロジェクトが必要
先人から学ぶ先人から学ぶテストを実行する場としてのRailsアプリが必要DatabaseRewinder gemのテストを参考にする小さいRailsアプリをテスト用に追加
expect in expect 問題
テスト対象コード中にexpectがあるのは明らかな破綻破綻expectが失敗することを確認するテストはそもそも無理そもそも無理
RSpecの中で「expectが失敗」したらその時点でテストは失敗するexpectが失敗することによって成功するテストは書けなさそう
gem内のコードとしてexpectではなくvalidation!を使用する「例外が発生することを確認するテスト」は「expectが失敗することを確認するテスト」の100倍書きやすい
expect in expectを回避して対応
リリースリリースそんなこんなで機能もテストも用意できたのでリリース$ rake release
改良編改良編件のバグはhas_many関連を持つモデルでのみ発生has_manyを持つモデルのみを対象として高速化ActiveRecord::Reflection::ClassMethodsを使用することでhas_many関連を持つモデルを抽出できる
まとめまとめバグによってはバリデーションをすり抜けうる2つのパターン1. default_scope設定されたモデルの周辺モデル2. has_many関連に対する個数のバリデーション
2 のパターンのすり抜けをテストで防止するためrspec-allrecords_validatorを是非どうぞhttps://github.com/colorbox/rspec-all_records_validator