バグから生まれたgem太郎

バグから生まれたgemの話失敗から学ぶ技術失敗から学ぶ技術 @color_box

rspec-all_records_validatorのはなし

プロジェクトで出たバグの話プロジェクトで出たバグの話

ある日突然の問い合わせある日突然の問い合わせ

「なんか更新機能動かないよ？」「なんか更新機能動かないよ？」

なぜか特定レコードがinvalidになっていた

モデル構造モデル構造 class Binder < ApplicationRecord has_many :documents validates :documents,
length: {minimum: 1} end class Document < ApplicationRecord belongs_to :binder end

Binderからdocumentsに対する関連が消失 has_many関連のlengthに対する最小値のバリデーションによりinvalidに

バリデーションをすり抜けるはなしバリデーションをすり抜けるはなし

発生原因発生原因結論から書くとバグ

問題のバリデーション問題のバリデーション関連の個数に対するバリデーションはバグですり抜けうる validates :documents, length: {minimum: 1}

再現コード再現コード binderをそれが持つ書類ごとコピーしようとして下記のようなコードを書いてしまう original_binder = binder.find(XXX) copied_documents = original_binder.documents.each
do |document document.dup end Binder.create!(documents: copied_documents)

バグバグオリジナルのbinderのdocumentsをコピーする eachを使用したためコピー元のdocumentsが移動 map等を使用すべき箇所だった copied_documents = original_binder.documents.each do
|document document.dup end

バリデーションがあるのにすり抜けてしまう

同じフローを実行するE2Eテストは存在検出できなかった

テスト内でoriginal_binderに対するバリデーションは実行されていれば検出できた

新規に作製されたレコードについてのみ実行され古いレコードはバリデーションされない

original_binderでバリデーションが実行されないためすり抜けが発生すり抜けが発生

自動で検出できると嬉しい

こういうすり抜けをなんとかしたいこういうすり抜けをなんとかしたいテスト終了時にDB内のすべてのレコードに対して validationをかけることで検出できそう

テスト終了時に全レコードのバリデーションチェック

Q. Q. 特定レコードが invalidになることを確認するテストはどうする？

A. 無視する A. 無視する

featureやsystemのような E2Eテストのみを対象とするバグでない限りユーザー操作を模倣したテスト終了時に invalidなデータが発生しているのはおかしい

初手、プロトタイプ、検証初手、プロトタイプ、検証ひとまずざっくりRSpecのafterフックに仕込む config.after do ApplicationRecord.subclasses.all? {|klass| klass.all.each {|obj| expect(obj).to
be_valid } } end

雑に仕事プロジェクトに導入雑に仕事プロジェクトに導入テスト用のコードなので本番環境に入らない本番には影響がなく開発/テスト環境に閉じる最悪取り外せば良い

判明した問題判明した問題 default_scopeを使用していると正常なユーザー操作後でもinvalidになるレコードが大量発生する大量発生する CI実行時間が 1.5倍に 1.5倍に

default_scope default_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

バグから生まれたgem太郎

バグから生まれたgem太郎

More Decks by color_box

Other Decks in Technology

Featured

Transcript