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