前回はspec側でのデータの使い分けでした。今回はfactory側、データそのものをどう作るかです。
何を埋めて何を埋めないか。そしてもう1つ、データが揃いすぎていると検出できないバグがあります
以前、FactoryBot作成のベストプラクティス!? という記事を書きましたが、今の考えで整理し直します。

値を埋める

必須項目は全て埋めます。 埋まっていないと、レスポンスの検証でnil同士の比較になって何も守りません。

expect(response_parsed_body_sym[:space][:name]).to eq(space.name)

space.namenilなら、レスポンスのnamenilでも通ってしまう。何を検証したのか分かりません。

値はできるだけ散らします。 固定値だと全レコードで同じになるので、取り違えに気づけません。namedescriptionが同じ値だったら、入れ替わっていても分からない。

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が減るので、ケース数が多いほど効いてきます。

非正規化した関連は整合を取る

保守のために外部キーを重複して持たせることがあります。spacestaskstask_cyclesという階層で、task_cyclesspace_idも持たせるような形です。

この場合、factoryで作るとspacetask.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_stubbedafter(: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_createcreate戦略にしか効きません。また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_atcreated_atより後のランダムな時刻にする。 これが意図的で、更新は作成順とは限りません。作成順と更新順をわざとずらすことで、並び順のバグが検出できます。 ここでFakerを使っているのは値をばらけさせるためです。特定の値が必要なら、呼び出し側で明示的に渡します。

どちらも過去にする。 「今」との比較が常に成立します。

specを1行も足さずに、既存の並び順の検証が効くようになります。

偶然の一致が見逃しを生む

一般化すると、テストデータが「たまたま揃っている」と、区別すべきものが区別できなくなります。

  • created_atupdated_atが同じ → 取り違えが通る
  • id順と作成順が同じ → 同上
  • 全レコードで同じ値の属性 → その属性での絞り込みミスが通る

factoryは「妥当なデータを作る」だけでなく、区別できるデータを作る必要があります。値をばらけさせるのも、一意性が要るところでsequenceを使うのも、根っこは同じです。

なお、このバグで実際に困ったことはありません。並び順の検証を書いている時に気づいて、先に潰した形です。予防なので効果が観測できないのは当然ですが、コストもほとんどかかりません。

まとめ

  • 必須項目も任意項目も埋める。nilだと検証にならない
  • 値はできるだけ散らす。ユニーク制約があるところはsequenceやハッシュで
  • 任意項目をtraitで足す方式は、付け忘れても落ちないので使わない
  • 必須のリレーションはassociation。任意のリレーションは自動で作らない
  • 非正規化した関連は、コールバックで整合を取る。build_stubbed用にafter(:stub)も要る
  • traitはベースからの差分を表現する。デフォルト値と同じでも明示する
  • 共通のtraitはFactoryBot.define直下、モデル固有のものはfactory内
  • バリデーションを通らないレコードはto_createで作れるようにしておく
  • factoryに複雑な状態を作り込まない。複雑な前提はspec側のshared_contextで
  • created_atupdated_atをずらしておくと、並び順のバグが検出できる
  • テストデータが「たまたま揃っている」と、区別すべきものが区別できなくなる

次回は、テストケースの減らし方を書きます。責務の分け方と、実装構造を根拠にケースを絞る話です。

コメントを残す

メールアドレスが公開されることはありません。 が付いている欄は必須項目です