thoughtbot / thoughtbot/factory_bot

Object associations behave inconsistently inside factories

Open
#1,373 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Ruby
Stars
8.2k
Forks
2.6k
PR merge metrics
No merged PRs in 30d

Description

In our rails app, we had some code like this:

class Account < ApplicationRecord
  belongs_to :unit

  validates_presence_of :number
  validates_uniqueness_of :number, scope: :unit_id, message: "Number has to be unique for this unit"
end

class Unit < ApplicationRecord
  has_many :accounts, dependent: :destroy
end

# factories/accounts.rb
FactoryBot.define do
  factory :account do
    association :unit
    sequence(:name) { |n| "Fake account #{n}" }
    number {
      generate_account_number(
        unit.accounts.pluck(:number).to_set
      )
    }
  end
end

# factory_bot_helpers.rb
def generate_account_number exclude_list
  l = -> { "%04d" % rand(10**4) } # generate a 4-digit random number
  number = 1
  attempt = 0
  loop do
    attempt += 1
    number = l.call
    break unless exclude_list.include?(number.to_s)
    raise "We've run out of account numbers for this unit" if attempt > 1000
  end
  number.to_s
end

# some_spec.rb
some_unit = FactoryBot.create :unit

# ... pretend we create ~10 accounts here

# this one fails randomly because the association unit.accounts returns an empty collection
FactoryBot.create :account, unit_id: some_unit.id

# this one always works because the association returns the accounts already created
FactoryBot.create :account, unit: some_unit

Our problem lies inside the number-block (where we call generate_account_number)
When passing unit_id to the factory unit.accounts returns []
When passing a unit object to the factory unit.accounts returns actual accounts.

Our current solution looks like this: Account.where(unit_id: unit_id).pluck(:number).to_set
There shouldn't be a difference between these two.

The problem is that subtleties like this cause failing tests and weird unintuitive behavior.

There are a couple of things I would like to be able to do about it:
If I could configure this particular factory to simply raise if passed any {attribute}_id (or an arbitrary attribute), we could prevent ourselves from accidentally building test data incorrectly. We would also want to configure this factory to be unbuildable (raise on calling :build), as it would have to run a query against the database to ensure that the data generated is valid.

(yes I know we should use a sequence)

System configuration

factory_bot version: 4.11.1
rails version: 5.2.2 (activerecord 5.2.2)
ruby version: 2.4.2

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the difference in some_spec.rb using FactoryBot.create with unit_id and with unit, then inspect the association behavior exercised by the number block in factories/accounts.rb. Review factory_bot_helpers.rb and the factory build/create entry points for the requested validation options. Done means the association behavior is consistent and the proposed factory restrictions have defined behavior for invalid attributes and build calls.

Written by the indexing model from the issue text.

Assessment

Tech stack
rails, ruby
Domain
testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.