thoughtbot / thoughtbot/factory_bot

Linting `traits: true` on nested factories made from traits causes those traits to be called twice in one build

Open
#1,560 0 comments 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

Description

Calling FactoryBot.lint([my_factory], traits: true) will create a factory with every trait it has defined.

For most factories and traits, this is fine. However, when a nested factory uses a trait, the trait is run twice: once in the definition of the factory, and again because the trait is passed in to the strategy call.

Reproduction Steps
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 :post_authors, force: true do |t|
    t.string :name
    t.integer :post_id
    t.index ["name", "post_id"], name: "index_post_author_unique", unique: true
  end
  create_table :posts, force: true do |t|
    t.string :title
    t.string :body
    t.boolean :approved
  end
  add_foreign_key "post_authors", "posts"
end

class PostAuthor < ActiveRecord::Base
  belongs_to :post
end

class Post < ActiveRecord::Base
  has_many :post_authors
end

FactoryBot.define do
  factory :post_author # has a unique index on post_id and name

  factory :post do
    title { "A title" }

    trait :with_authors do
      transient do
        author_names { ['Björn', 'Benny', 'Agnetha', 'Frida'] }
      end

      after(:create) do |post, evaluator|
        # Running this twice will cause an index violation.
        evaluator.author_names.each { |name| create(:post_author, post: post, name: name) }
      end
    end

    factory :approved_post do
      with_authors

      approved { true }
    end
  end
end

# Override strategy method so we can see what happens under the hood.
module FactoryBot
  def self.create(*args, **kwargs)
    puts "calling create with args: #{args} and kwargs: #{kwargs}"
    super
  end
end

class FactoryBotTest < Minitest::Test
  def test_factory_bot_lint
    FactoryBot.factories.each do |factory|
      puts "Lint #{factory.name}"
      ActiveRecord::Base.transaction do
        FactoryBot.lint([factory]) # passes
        raise ActiveRecord::Rollback
      end
    end
  end

  def test_factory_bot_lint_with_traits
    FactoryBot.factories.each do |factory|
      puts "Lint #{factory.name} with traits"
      ActiveRecord::Base.transaction do
        FactoryBot.lint([factory], traits: true) # fails for approved_post
        raise ActiveRecord::Rollback
      end
    end
  end
end

# Run the tests with `ruby <filename>`
Expected behavior

Factories defined with traits from parent factories should not be linted with those traits. This is the same as saying the following:

create(:post, :with_author_names) # this is fine
create(:approved_post) # this is also fine

# One would refrain from doing this, but calling it shouldn't double up the `:with_author_names` trait.
create(:post, :with_author_names, :with_author_names)

# Knowledge of the app being developed would cause one to refrain against this, but calling it shouldn't
# double up the `:with_author_names` trait.
create(:approved_post, :with_author_names)

All tests pass. The only create calls that are logged are:

calling create with args: [:post_author] and kwargs: {}
calling create with args: [:post] and kwargs: {}
calling create with args: [:post, "with_authors"] and kwargs: {}
calling create with args: [:approved_post] and kwargs: {}
Actual behavior

One test fails:

Finished in 0.009972s, 200.5616 runs/s, 0.0000 assertions/s.

  1) Error:
FactoryBotTest#test_factory_bot_lint_with_traits:
FactoryBot::InvalidFactoryError: The following factories are invalid:

* approved_post+with_authors - SQLite3::ConstraintException: UNIQUE constraint failed: post_authors.name, post_authors.post_id (ActiveRecord::RecordNotUnique)
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/factory_bot-6.2.1/lib/factory_bot/linter.rb:13:in `lint!'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/factory_bot-6.2.1/lib/factory_bot.rb:70:in `lint'
    factory_bot_test.rb:89:in `block (2 levels) in test_factory_bot_lint_with_traits'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activerecord-6.1.7/lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `block in transaction'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activerecord-6.1.7/lib/active_record/connection_adapters/abstract/transaction.rb:319:in `block in within_new_transaction'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activesupport-6.1.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activesupport-6.1.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activesupport-6.1.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activesupport-6.1.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activesupport-6.1.7/lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activerecord-6.1.7/lib/active_record/connection_adapters/abstract/transaction.rb:317:in `within_new_transaction'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activerecord-6.1.7/lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `transaction'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/activerecord-6.1.7/lib/active_record/transactions.rb:209:in `transaction'
    factory_bot_test.rb:88:in `block in test_factory_bot_lint_with_traits'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/factory_bot-6.2.1/lib/factory_bot/registry.rb:19:in `each'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/factory_bot-6.2.1/lib/factory_bot/registry.rb:19:in `each'
    /Users/henry/.rbenv/versions/2.6.10/gemsets/bookwhen_admin/gems/factory_bot-6.2.1/lib/factory_bot/decorator.rb:21:in `method_missing'
    factory_bot_test.rb:86:in `test_factory_bot_lint_with_traits'

2 runs, 0 assertions, 0 failures, 1 errors, 0 skips

The incorrect create call that was logged is:

calling create with args: [:approved_post, "with_authors"] and kwargs: {}
System configuration

factory_bot version: 6.0.2 (tested on latest 6.2.1 as bundler inline downloaded "~6.0")
rails version: 6.0.5.1
ruby version: 2.6.10

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 failure with the inline script and start at FactoryBot.lint, following the linter.rb lint! entry point shown in the stack trace. Inspect how traits are passed to nested factories such as approved_post. Done means linting with traits: true no longer invokes a parent trait twice and the reproduction tests pass.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.