thoughtbot / thoughtbot/factory_bot
Linting `traits: true` on nested factories made from traits causes those traits to be called twice in one build
Nobody has claimed this yet.
- 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
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 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