前回はspec側でのデータの使い分けでした。今回はfactory側、データそのものをどう作るかです。
何を埋めて何を埋めないか。そしてもう1つ、データが揃いすぎていると検出できないバグがあります。
以前、FactoryBot作成のベストプラクティス!? という記事を書きましたが、今の考えで整理し直します。
値を埋める
必須項目は全て埋めます。 埋まっていないと、レスポンスの検証でnil同士の比較になって何も守りません。
expect(response_parsed_body_sym[:space][:name]).to eq(space.name)
space.nameがnilなら、レスポンスのnameがnilでも通ってしまう。何を検証したのか分かりません。
値はできるだけ散らします。 固定値だと全レコードで同じになるので、取り違えに気づけません。nameとdescriptionが同じ値だったら、入れ替わっていても分からない。
sequence(:name) { |n| "space(#{n})" }
code { Digest::MD5.hexdigest(SecureRandom.uuid) }
model { Download.models.keys.sample }
description { Faker::Lorem.sentence }
enumのように取りうる値が決まっているものは、一覧からsampleで選べば散ります。
Fakerで散らすだけでも効果はあります。たまたま一致する確率は低い。ただしユニーク制約がある項目には使いません。名前や単語はいずれ重複するので、sequenceやハッシュで作ります。
任意項目もできるだけ埋めます。 理由は同じで、nilだと検証にならないからです。nilのケースを検証したい時だけ、明示的にnilを渡します。
任意項目をtraitで足す方式もありますが、付け忘れても落ちないので使っていません。埋まっているのを既定にして、外したい時だけ明示する方が、判断が要らなくて確実です。
リレーションの扱い
必須のリレーションはassociationで作ります。
association :created_user, factory: :user
呼び出し側で指定すれば、そちらが優先されます。指定しなければ自動で作られる。
前回書いた通り、関連先の作られ方は呼び出し側の戦略に従います。build(:space)なら関連先もbuildされるので、INSERTは走りません。
任意のリレーションは自動で作りません。 実装上は必須に見えても、構造上は存在しない可能性があるものがあります。
- 「最後に更新したユーザー」— 一度も更新されていなければ存在しない
- 「作成したユーザー」— そのユーザーが退会して削除されていれば
nilになる - 後から追加したカラム — 既存レコードは
nilのまま
バリデーションで必須にしていても、DBにNOT NULL制約がなければnilのレコードは存在しえます。バリデーションを通さない更新やデータ移行でも起こります。
factoryが常に作ってしまうと、その状態を再現できなくなります。nilで落ちるコードを書いてもspecが通ってしまう。
再現性が本題ですが、副次的に速度も上がります。関連先を作らない分だけINSERTが減るので、ケース数が多いほど効いてきます。
非正規化した関連は整合を取る
保守のために外部キーを重複して持たせることがあります。spaces → tasks → task_cyclesという階層で、task_cyclesにspace_idも持たせるような形です。
この場合、factoryで作るとspaceとtask.spaceがズレる可能性があります。コールバックで揃えておきます。
after(:build) do |task_cycle|
if task_cycle.task.blank?
task_cycle.space = build(:space, :public) if task_cycle.space.blank?
task_cycle.task = build(:task, space: task_cycle.space)
else
task_cycle.space = task_cycle.task.space
end
end
after(:stub) do |task_cycle|
if task_cycle.task.blank?
task_cycle.space = build_stubbed(:space, :public) if task_cycle.space.blank?
task_cycle.task = build_stubbed(:task, space: task_cycle.space)
else
task_cycle.space = task_cycle.task.space
end
end
build_stubbedはafter(:build)を実行しないので、after(:stub)も書く必要があります。中身はほぼ同じですが、コールバックの中で戦略を判定する手段がないので、両方定義することになります。
ありえないデータが作られないようにする、という役割です。
traitで差分を表現する
ベースのfactoryは「一番普通の状態」にして、そこからの差分をtraitで表現します。
factory :download do
status { :waiting }
trait :waiting do
status { :waiting }
end
trait :processing do
status { :processing }
end
trait :success do
status { :success }
completed_at { Faker::Time.backward(days: 365) }
end
end
呼び出し側はcreate(:download, :success)と書くだけで済みます。属性を直接渡すより、何の状態かが名前で読めます。関連する属性をまとめて変えられるのも利点です。
デフォルト値に期待しない
上の例で:waitingは、ベースの値と同じです。書かなくても動きます。
それでも書いているのは、明示するためです。
create(:download, :waiting)と書いた人は「待機中のレコードを作った」つもりです。でもtraitが空だと、実際に効いているのはベースやmigrationのデフォルト値です。デフォルトが変わったら、黙って挙動が変わります。
traitは共有できる
FactoryBot.defineの直下に書いたtraitは、全factoryから使えます。
FactoryBot.define do
trait :destroy_reserved do
destroy_requested_at { Faker::Time.backward(days: 30) }
destroy_schedule_at { destroy_requested_at + Settings.space_destroy_schedule_days.days }
end
end
複数のモデルが同じ「削除予約済み」の状態を持つなら、1箇所で定義できます。
一方、factory内に書いたtraitはそのfactory専用です。スコープが違うので、どこに書くかで使える範囲が決まります。共通のものはグローバル、そのモデル固有のものはfactory内。
バリデーションを通らないレコードを作る
異常系の検証や、過去データの再現で、バリデーションを通らないレコードが必要になることがあります。
trait :skip_validate do
to_create { |instance| instance.save(validate: false) }
end
create(:user, :skip_validate, email: nil)
これがないと、buildしてから属性を変えてsave(validate: false)する、という手順をspec側に書くことになります。1行で済む方がいい。
なおto_createはcreate戦略にしか効きません。またsave(validate: false)はバリデーションをスキップするだけで、コールバックは走ります。DBにNOT NULL制約がある項目は、そもそも保存できないのでnilにはできません。
factoryで頑張りすぎない
ここまで書いておいて何ですが、factoryに複雑なロジックを持たせるのは避けたいと思っています。
「このtraitを指定すると関連レコードが3件作られて、そのうち1件は削除予約済みで……」のような状態を作り込むと、呼び出し側から何が起きるか読めなくなります。2回目に書いた「呼び出し側から中身を見に行かずに済むか」の基準を満たさない。
複雑な前提はspec側のshared_contextで組み立てる方が、何が用意されているかが見えます。factoryは「1レコードを妥当に作る」までに留めるくらいでちょうどいい。
created_atとupdated_atを自動でずらす(おすすめ)
並び順の検証を書いていた時に気づいたことがあります。
created_at順とupdated_at順が一致していると、間違った方でソートしていても通ります。更新日時順のつもりで作成日時順に並べていても、期待値と一致してしまう。
同じ時刻のレコードが複数あると、DBが返す順序が不定になるという問題もあります。たまたま通るテストになる。
そこでrails_helper.rbで、全factoryにタイムスタンプを差し込んでいます。
RSpec.configure do |config|
# NOTE: created_at/updated_atを過去にして、テストの信頼性を向上させる
config.before(:suite) do
FactoryBot.factories.each do |factory|
definition = factory.instance_variable_get(:@definition)
definition.after(:build) do |record|
record.created_at = FactoryBotHelper.make_created_at if record.has_attribute?(:created_at) && record.created_at.blank?
record.updated_at = FactoryBotHelper.make_updated_at(record.created_at) if record.has_attribute?(:updated_at) && record.updated_at.blank?
end
end
end
end
module FactoryBotHelper
class << self
def make_created_at
@base_time = base_time + 1.second
end
def make_updated_at(created_at)
return created_at if created_at > max_time
Faker::Time.between(from: created_at, to: max_time).floor
end
private
def base_time
@base_time ||= 1.year.ago.floor
end
def max_time
@max_time ||= 1.hour.ago.floor
end
end
end
やっていることは3つです。
created_atを1秒ずつずらす。 全レコードで一意になるので、ソート順が確定します。作成順とも一致します。
updated_atをcreated_atより後のランダムな時刻にする。 これが意図的で、更新は作成順とは限りません。作成順と更新順をわざとずらすことで、並び順のバグが検出できます。 ここでFakerを使っているのは値をばらけさせるためです。特定の値が必要なら、呼び出し側で明示的に渡します。
どちらも過去にする。 「今」との比較が常に成立します。
specを1行も足さずに、既存の並び順の検証が効くようになります。
偶然の一致が見逃しを生む
一般化すると、テストデータが「たまたま揃っている」と、区別すべきものが区別できなくなります。
created_atとupdated_atが同じ → 取り違えが通る- id順と作成順が同じ → 同上
- 全レコードで同じ値の属性 → その属性での絞り込みミスが通る
factoryは「妥当なデータを作る」だけでなく、区別できるデータを作る必要があります。値をばらけさせるのも、一意性が要るところでsequenceを使うのも、根っこは同じです。
なお、このバグで実際に困ったことはありません。並び順の検証を書いている時に気づいて、先に潰した形です。予防なので効果が観測できないのは当然ですが、コストもほとんどかかりません。
まとめ
- 必須項目も任意項目も埋める。
nilだと検証にならない - 値はできるだけ散らす。ユニーク制約があるところは
sequenceやハッシュで - 任意項目をtraitで足す方式は、付け忘れても落ちないので使わない
- 必須のリレーションは
association。任意のリレーションは自動で作らない - 非正規化した関連は、コールバックで整合を取る。
build_stubbed用にafter(:stub)も要る - traitはベースからの差分を表現する。デフォルト値と同じでも明示する
- 共通のtraitは
FactoryBot.define直下、モデル固有のものはfactory内 - バリデーションを通らないレコードは
to_createで作れるようにしておく - factoryに複雑な状態を作り込まない。複雑な前提はspec側のshared_contextで
created_atとupdated_atをずらしておくと、並び順のバグが検出できる- テストデータが「たまたま揃っている」と、区別すべきものが区別できなくなる
次回は、テストケースの減らし方を書きます。責務の分け方と、実装構造を根拠にケースを絞る話です。
