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