thoughtbot / thoughtbot/factory_bot

Traits possible race condition when creating dependent associations

Open
#1,501 8 comments 15 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

Description

When using FactoryBot.create to create a model with a dependent association validations that depend on traits will fail because they are applied too late

This can be doubly confirmed if using a custom validation method and debugging within the method to check the value of the record

Another side effect is that validations will get called twice, once when the dependent association is created and once when the main model is created. This isn't the main issue however since having 2 validations that pass is better than having one that fails.

EDIT 1: It might also be some sort of race condition since the validation should not be required anyway if the trait wasn't at least partially applied, which makes this all the more confusing to me.
EDIT 2: Added a reproduction script

Reproduction Steps

Reproduction repo

Reproduction script
require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "factory_bot", "~> 6.0"
  gem "activerecord"
  gem "sqlite3"
end

require "active_record"
require "factory_bot"
require "minitest/autorun"
require "logger"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :users, force: true do |t|
    t.integer :role, default: 0
    t.string :some_value
  end

  create_table :addresses, force: true do |t|
    t.references :user, null: false, foreign_key: true
  end
end

class User < ActiveRecord::Base
  enum role: %i[user admin]

  has_one :address

  validates_presence_of :some_value, if: :admin?
end

class Address < ActiveRecord::Base
  belongs_to :user
end

FactoryBot.define do
  factory :user do
    role { :user }

    address { association(:address, user: instance) }

    trait :admin do
      role { :admin }
      some_value { 'present' }
    end
  end

  factory :address do
    user
  end
end

class FactoryBotTest < Minitest::Test
  def test_factory_bot_stuff
    user = FactoryBot.create(:user, :admin)

    assert user.persisted?
  end
end

# Run the tests with `ruby <filename>`
Reproduction detailed steps
  1. Create a main model with a conditional presence validation
class User
  enum role: %I[user admin]
  
  validates_presence_of :some_value, if: :admin?
end
  1. Create a second model linked to the First model
class User
# ...
  has_one :address
# ...
end

class Address
  belongs_to :user
end
  1. Create a factory with a trait to validate the given condition
FactoryBot.define do
  factory :user do
    role { :user }

    address { association(:address, user: instance) }

    trait :admin do
      role { :admin }
      some_value { 'present' }
    end
  end
end
  1. Create a simple factory for the second model
FactoryBot.define do
  factory :address do
    user { association(:user, address: instance) }
  end
end
  1. Add some simple specs
require 'rails_helper'

RSpec.describe User, type: :model do
  context 'without trait' do
    it 'builds' do
      expect(build(:user)).to be_valid
    end

    it 'creates' do
      expect(create(:user)).to be_persisted
    end
  end

  context 'with admin trait' do
    it 'builds' do
      expect(build(:user, :admin)).to be_valid
    end

    # this will fail
    it 'creates' do
      expect(create(:user, :admin)).to be_persisted
    end
  end
end
  1. Run specs
rails db:migrate
bundle exec rspec
Expected behavior
bundle exec rspec
#=> ....
Actual behavior
bundle exec rspec

#=> ...F
#=> Failures:

#=>  1) User with admin trait creates
#=>     Failure/Error: address { association(:address, user: instance) }

#=>     ActiveRecord::NotNullViolation:
#=>       PG::NotNullViolation: ERROR:  null value in column "user_id" violates not-null constraint
System configuration

factory_bot version: 6.2.0
rails version: 6.1.3.2
ruby version: 2.7.3

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

Start with the reproduction script in the issue and run it with ruby <filename>, then compare the FactoryBot.create(:user, :admin) path with the passing build case. Trace the dependent address association and trait application order; done means the reproduction and the listed RSpec examples pass without the not-null validation failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
rails, ruby
Domain
testing-qa
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.