1
0
Fork 0
mirror of https://github.com/thoughtbot/factory_bot.git synced 2022-11-09 11:43:51 -05:00

Remove code for registering callbacks

Before this commit, factory_bot registered default callbacks in a global
registry, and offered a `register_callback` method for registering any
custom callbacks (which would be fired within custom strategies).

The idea here seems to have been that we could check against this global
registry and then raise an error when referring to a callback that was
not registered.

However, nobody ever saw this error because we have always had the line:
`FactoryBot::Internal.register_callback(name)`, which registers
callbacks on the fly whenever they are referred to. (I noticed this when
a [change to default callbacks] accidentally duplicated `after_create`
instead of including `before_create`, and we didn't get any tests
failures.)

We have two options here:

1. Preserve the existing behavior by deleting all the callback
registration code. (That is what this commit does)

2. Change to the code try and capture what I assume was the original
intention. This would be a breaking change, so we would want to
introduce it in a major release. (See #1379)

I prefer preserving the existing behavior because we haven't seen any
issues around this, and making a breaking change for this doesn't seem
worthwhile.

[change to default callbacks]: f82e40c8c5 (diff-c6d30be672f880311a7df0820dc4fb21R12-R14)
This commit is contained in:
Daniel Colson 2020-05-03 12:25:39 -04:00
parent b73b13ca7e
commit 7cfa233775
6 changed files with 0 additions and 36 deletions

View file

@ -89,4 +89,3 @@ module FactoryBot
end
FactoryBot::Internal.register_default_strategies
FactoryBot::Internal.register_default_callbacks

View file

@ -5,7 +5,6 @@ module FactoryBot
def initialize(name, block)
@name = name.to_sym
@block = block
ensure_valid_callback_name!
end
def run(instance, evaluator)
@ -27,13 +26,6 @@ module FactoryBot
private
def ensure_valid_callback_name!
unless FactoryBot::Internal.callback_names.include?(name)
raise InvalidCallbackNameError, "#{name} is not a valid callback name. " +
"Valid callback names are #{FactoryBot::Internal.callback_names.inspect}"
end
end
def syntax_runner
@syntax_runner ||= SyntaxRunner.new
end

View file

@ -102,7 +102,6 @@ module FactoryBot
def callback(*names, &block)
names.each do |name|
FactoryBot::Internal.register_callback(name)
add_callback(Callback.new(name, block))
end
end

View file

@ -4,7 +4,6 @@ module FactoryBot
class << self
delegate :after,
:before,
:callback_names,
:callbacks,
:constructor,
:factories,
@ -87,18 +86,6 @@ module FactoryBot
register_strategy(:build_stubbed, FactoryBot::Strategy::Stub)
register_strategy(:null, FactoryBot::Strategy::Null)
end
def register_default_callbacks
register_callback(:after_create)
register_callback(:after_build)
register_callback(:after_stub)
register_callback(:before_create)
end
def register_callback(name)
name = name.to_sym
callback_names << name
end
end
end
end

View file

@ -2,7 +2,6 @@ module FactoryBot
def self.reload
Internal.reset_configuration
Internal.register_default_strategies
Internal.register_default_callbacks
find_definitions
end
end

View file

@ -24,16 +24,4 @@ describe FactoryBot::Callback do
FactoryBot::Callback.new(:after_create, ->(one, two) { ran_with = [one, two] }).run(:one, :two)
expect(ran_with).to eq [:one, :two]
end
it "allows valid callback names to be assigned" do
FactoryBot::Internal.callback_names.each do |callback_name|
expect { FactoryBot::Callback.new(callback_name, -> {}) }.
to_not raise_error
end
end
it "raises if an invalid callback name is assigned" do
expect { FactoryBot::Callback.new(:magic_fairies, -> {}) }.
to raise_error(FactoryBot::InvalidCallbackNameError, /magic_fairies is not a valid callback name/)
end
end