thoughtbot / thoughtbot/factory_bot
Object associations behave inconsistently inside factories
Nobody has claimed this yet.
- 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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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