Skip to content

Commit

Permalink
Fix strategy tracking through associations
Browse files Browse the repository at this point in the history
What?
=====

Within FactoryBot::Evaluator, the build strategy was reported either as
a class OR symbol.

A side-effect of this is misreporting within
ActiveSupport::Notifications hooks, since the instrumentation payload
may have strategies listed as e.g. `:create` or
`FactoryBot::Strategy::Create`, even though they semantically represent
the same information.

This introduces a new instance method for all strategies, `to_sym`,
to be called rather than `class`.
  • Loading branch information
joshuaclayton committed Mar 8, 2022
1 parent 8f766ef commit 1b81d5d
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/factory_bot/evaluator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def initialize(build_strategy, overrides = {})
def association(factory_name, *traits_and_overrides)
overrides = traits_and_overrides.extract_options!
strategy_override = overrides.fetch(:strategy) {
FactoryBot.use_parent_strategy ? @build_strategy.class : :create
FactoryBot.use_parent_strategy ? @build_strategy.to_sym : :create
}

traits_and_overrides += [overrides.except(:strategy)]
Expand Down
4 changes: 4 additions & 0 deletions lib/factory_bot/strategy/attributes_for.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ def association(runner)
def result(evaluation)
evaluation.hash
end

def to_sym
:attributes_for
end
end
end
end
4 changes: 4 additions & 0 deletions lib/factory_bot/strategy/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ def result(evaluation)
evaluation.notify(:after_build, instance)
end
end

def to_sym
:build
end
end
end
end
4 changes: 4 additions & 0 deletions lib/factory_bot/strategy/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ def result(evaluation)
evaluation.notify(:after_create, instance)
end
end

def to_sym
:create
end
end
end
end
4 changes: 4 additions & 0 deletions lib/factory_bot/strategy/null.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ def association(runner)

def result(evaluation)
end

def to_sym
:null
end
end
end
end
4 changes: 4 additions & 0 deletions lib/factory_bot/strategy/stub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ def result(evaluation)
end
end

def to_sym
:stub
end

private

def next_id
Expand Down
14 changes: 14 additions & 0 deletions spec/acceptance/activesupport_instrumentation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ def subscribed(callback, *args)
let(:user_factory) { FactoryBot::Internal.factory_by_name("user") }
before do
define_model("User", email: :string)
define_model("Post", user_id: :integer) do
belongs_to :user
end

FactoryBot.define do
factory :user do
email { "[email protected]" }
Expand All @@ -24,6 +28,12 @@ def subscribed(callback, *args)
after(:build) { Kernel.sleep(0.1) }
end
end

factory :post do
trait :with_user do
user
end
end
end
end

Expand Down Expand Up @@ -55,12 +65,16 @@ def subscribed(callback, *args)
FactoryBot.build_list(:user, 5)
FactoryBot.create_list(:user, 2)
FactoryBot.attributes_for(:slow_user)
user = FactoryBot.create(:user)
FactoryBot.create(:post, user: user)
FactoryBot.create_list(:post, 2, :with_user)
end

expect(tracked_invocations[:slow_user][:build]).to eq(2)
expect(tracked_invocations[:slow_user][:attributes_for]).to eq(1)
expect(tracked_invocations[:slow_user][:factory]).to eq(slow_user_factory)
expect(tracked_invocations[:user][:build]).to eq(5)
expect(tracked_invocations[:user][:factory]).to eq(user_factory)
expect(tracked_invocations[:user][:create]).to eq(5)
end
end

0 comments on commit 1b81d5d

Please sign in to comment.