バグから生まれたgem太郎

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

View Slide

rspec-all_records_validatorのはなし

View Slide

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

View Slide

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

View Slide

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

View Slide

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

View Slide

モデル構造
モデル構造
class Binder < ApplicationRecord

has_many :documents

validates :documents, length: {minimum: 1}

end

class Document < ApplicationRecord

belongs_to :binder

end

View Slide

Binderからdocumentsに対する関連が消失
has_many関連のlengthに対する

最小値のバリデーションによりinvalidに

View Slide

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

View Slide

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

View Slide

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

View Slide

再現コード
再現コード
binderをそれが持つ書類ごとコピーしようとして

下記のようなコードを書いてしまう
original_binder = binder.find(XXX)

copied_documents = original_binder.documents.each do |document
document.dup

end

Binder.create!(documents: copied_documents)

View Slide

バグ
バグ
オリジナルのbinderのdocumentsをコピーする

eachを使用したため
コピー元のdocumentsが移動
map等を使用すべき箇所だった
copied_documents = original_binder.documents.each do |document
document.dup

end

View Slide

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

View Slide

同じフローを実行するE2Eテストは存在

検出できなかった

View Slide

テスト内でoriginal_binderに対する

バリデーションは実行されていれば検出できた

View Slide

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

View Slide

original_binderでバリデーションが

実行されないため
すり抜けが発生
すり抜けが発生

View Slide

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

View Slide

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

View Slide

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

View Slide

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

View Slide

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

View Slide

featureやsystemのような

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

View Slide

初手、プロトタイプ、検証
初手、プロトタイプ、検証
ひとまずざっくりRSpecのafterフックに仕込む
config.after do

ApplicationRecord.subclasses.all? {|klass|

klass.all.each {|obj|

expect(obj).to be_valid

}

}

end

View Slide

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

View Slide

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

View Slide

default_scope
default_scopeの問題
の問題
default_scopeを使用していると

そのモデルと関連するレコードがinvalidになるのは
不可避
不可避

View Slide

レコード総チェックの中で特定のモデルを
避けたい
避けたい

View Slide

実行時間の問題
実行時間の問題
全モデルをチェックする上で
不要なモデルまで処理してそう
fixtureで投入するマスターデータなどは

バリデーション不要のはず

View Slide

レコード総チェックの中で特定のモデルを
避けたい
避けたい

View Slide

モデルのフィルタ機能追加
モデルのフィルタ機能追加
default_scope絡みでinvalidになるレコードを除外

バリデーション不要なレコードを除外して高速化

View Slide

リリースまで
リリースまで
概ね機能はできたのでgemとしてリリースしたい

テストは絶対必要

View Slide

テストどうしよう
テストどうしよう
書きづらい

View Slide

feature specもしくはsystem spec終了時のみ

実行したいコードのテスト
RSpecのafterでのみ実行される

コードをRSpecで実行する？
expectが失敗することをテストする？
expect in expect問題

View Slide

どうやってテストする？

View Slide

分割して対処
分割して対処
本質でない箇所は一旦無視
「全レコードに対してバリデーションを実行」
という部分のみテストする

View Slide

レコードを用意して
バリデーションをかけるための場が必要
擬似的なRailsプロジェクトが必要

View Slide

先人から学ぶ
先人から学ぶ
テストを実行する場としてのRailsアプリが必要
DatabaseRewinder gemのテストを参考にする
小さいRailsアプリをテスト用に追加

View Slide

expect in expect 問題

View Slide

テスト対象コード中にexpectがあるのは明らかな
破綻
破綻
expectが失敗することを確認するテストは
そもそも無理
そもそも無理

View Slide

RSpecの中で「expectが失敗」したら

その時点でテストは失敗する
expectが失敗することによって
成功するテストは書けなさそう

View Slide

gem内のコードとしてexpectではなく

validation!を使用する
「例外が発生することを確認するテスト」は
「expectが失敗することを確認するテスト」
の100倍書きやすい

View Slide

expect in expectを回避して対応

View Slide

リリース
リリース
そんなこんなで機能もテストも用意できたので
リリース
$ rake release

View Slide

改良編
改良編
件のバグはhas_many関連を持つモデルでのみ発生
has_manyを持つモデルのみを対象として高速化
ActiveRecord::Reflection::ClassMethods
を使用することで
has_many関連を持つモデルを抽出できる

View Slide

まとめ
まとめ
バグによってはバリデーションをすり抜けうる
2つのパターン
1. default_scope設定されたモデルの周辺モデル
2. has_many関連に対する個数のバリデーション

View Slide

2 のパターンのすり抜けをテストで防止するため

rspec-allrecords_validatorを是非どうぞ

https://github.com/colorbox/rspec-
all_records_validator

View Slide

バグから生まれたgem太郎

バグから生まれたgem太郎

color_box

More Decks by color_box

Other Decks in Technology

Featured

Transcript