Set up reloading after_initialize

Alternate fix for #336. We went with #347 instead because the solution
in this commit didn't work with Rails 4.2. Since we are no longer
supporting Rails 4.2, I think this is a better approach.

The original problem looked like this:

Rails runs all of the initializers
1. Run the ["factory_bot.set_factory_paths"][set_factory_paths]
initializer
2. Run the ["factory_bot.register_reloader"][register_reloader]
initializer, which sets up a [prepare callback][]
3. Run the [`:run_prepare_callbacks`][] initializer
4. This triggers the factory_bot [prepare callback][], which causes
factory\_bot to [reload][]

Rails runs `after_initialize` callbacks
1. [I18n initializes]
2. factory\_bot [reloads again][] as described in #334

Triggering the first factory_bot reload before initializing I18n could
cause an error in some cases.

We avoided the problem in #347 by adding a conditional to skip reloading
factory_bot before the application has initialized.

This commit, on the other hand, moves factory_bot reloading from a
prepare callback into an `after_initialize` callback. The initialization
process is now simplified to:

Rails runs all of the initializers
1. Run the ["factory_bot.set_factory_paths"][set_factory_paths]
2. Run the [`:run_prepare_callbacks`][] initializer, which no longer
involves factory_bot

Rails runs `after_intialize` callbacks
1. [I18n initializes]
2. factory_bot loads definitions for the first time. It then runs the
reloader to set up the prepare callback to reload factory_bot when the
application reloads (for example by calling `reload!` in the console),
and to register the reloader to trigger reloads when any factory_bot
definition files change.

[set_factory_paths]: 3815aae2b9/lib/factory_bot_rails/railtie.rb (L17-L19)
[register_reloader]: 3815aae2b9/lib/factory_bot_rails/railtie.rb (L21-L23)
[prepare callback]: https://github.com/thoughtbot/factory_bot_rails/blob/master/lib/factory_bot_rails/reloader.rb#L34-L36
[`:run_prepare_callbacks`]: https://github.com/rails/rails/blob/5-2-stable/railties/lib/rails/application/finisher.rb#L62-L64
[reload]: https://github.com/thoughtbot/factory_bot_rails/blob/master/lib/factory_bot_rails/reloader.rb#L24-L26
[I18n initializes]: 13e2102517/activesupport/lib/active_support/i18n_railtie.rb (L16-L20)
[reloads again]: https://github.com/thoughtbot/factory_bot_rails/blob/master/lib/factory_bot_rails/railtie.rb#L25-L27
This commit is contained in:
Daniel Colson 2020-06-15 10:41:04 -04:00
parent 870bf2a2f4
commit bfee5d8dcc
3 changed files with 53 additions and 29 deletions

View File

@ -18,12 +18,9 @@ module FactoryBotRails
FactoryBot.definition_file_paths = definition_file_paths
end
initializer "factory_bot.register_reloader" do |app|
Reloader.new(app, config).run
end
config.after_initialize do
FactoryBot.reload
config.after_initialize do |app|
FactoryBot.find_definitions
Reloader.new(app).run
end
private

View File

@ -4,9 +4,8 @@ require "factory_bot_rails/definition_file_paths"
module FactoryBotRails
class Reloader
def initialize(app, config)
def initialize(app)
@app = app
@config = config
@paths = DefinitionFilePaths.new(FactoryBot.definition_file_paths)
end
@ -18,7 +17,7 @@ module FactoryBotRails
private
attr_reader :app, :config
attr_reader :app
def build_reloader
reloader_class.new(@paths.files, @paths.directories) do
@ -31,12 +30,8 @@ module FactoryBotRails
end
def register_reloader(reloader)
closed_over_app = app
config.to_prepare do
if closed_over_app.initialized?
reloader.execute
end
app.reloader.to_prepare do
reloader.execute
end
app.reloaders << reloader

View File

@ -12,42 +12,74 @@ describe FactoryBotRails::Reloader do
context "when a definition file paths exist" do
it "registers a reloader" do
allow(reloader_class).to receive(:new)
file_watcher = file_watcher_double
run_reloader(["spec/fixtures/factories", "not_exist_directory"])
run_reloader(
["spec/fixtures/factories", "not_exist_directory"],
file_watcher
)
expect(reloader_class).to have_received(:new)
expect(file_watcher).to have_received(:new)
end
end
context "when a file exists but not a directory" do
it "registers a reloader" do
allow(reloader_class).to receive(:new)
file_watcher = file_watcher_double
run_reloader(["spec/fake_app", "not_exist_directory"])
run_reloader(
["spec/fake_app", "not_exist_directory"],
file_watcher
)
expect(reloader_class).to have_received(:new)
expect(file_watcher).to have_received(:new)
end
end
context "when a definition file paths NOT exist" do
it "does NOT register a reloader" do
allow(reloader_class).to receive(:new)
file_watcher = file_watcher_double
run_reloader(["not_exist_directory"])
run_reloader(["not_exist_directory"], file_watcher)
expect(reloader_class).not_to have_received(:new)
expect(file_watcher).not_to have_received(:new)
end
end
def run_reloader(definition_file_paths)
def run_reloader(definition_file_paths, file_watcher)
FactoryBot.definition_file_paths = definition_file_paths
FactoryBotRails::Reloader
.new(Rails.application, Rails.application.config).run
app = app_double(file_watcher)
FactoryBotRails::Reloader.new(app).run
end
def reloader_class
Rails.application.config.file_watcher
def file_watcher_double
class_double(
Rails.application.config.file_watcher,
new: double(:reloader, execute: nil)
)
end
def app_double(file_watcher)
instance_double(
Rails.application.class,
config: app_config_double(file_watcher),
reloader: reloader_double,
reloaders: []
)
end
def app_config_double(file_watcher)
instance_double(
Rails.application.config.class,
file_watcher: file_watcher
)
end
def reloader_double
class_double(
Rails.application.reloader,
to_prepare: nil
)
end
end
end