Skip to content

Commit

Permalink
Set up reloading after_initialize
Browse files Browse the repository at this point in the history
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]: https://github.com/thoughtbot/factory_bot_rails/blob/3815aae2b9e4a5c5c3027a2ee8851f44b7c6a4da/lib/factory_bot_rails/railtie.rb#L17-L19
[register_reloader]: https://github.com/thoughtbot/factory_bot_rails/blob/3815aae2b9e4a5c5c3027a2ee8851f44b7c6a4da/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]: https://github.com/rails/rails/blob/13e2102517fafc8f8736fce5d57de901067202d0/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
  • Loading branch information
composerinteralia committed Jun 15, 2020
1 parent 870bf2a commit bfee5d8
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 29 deletions.
9 changes: 3 additions & 6 deletions lib/factory_bot_rails/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,9 @@ class Railtie < Rails::Railtie
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
Expand Down
13 changes: 4 additions & 9 deletions lib/factory_bot_rails/reloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,8 @@

module FactoryBotRails
class Reloader
def initialize(app, config)
def initialize(app)
@app = app
@config = config
@paths = DefinitionFilePaths.new(FactoryBot.definition_file_paths)
end

Expand All @@ -18,7 +17,7 @@ def run

private

attr_reader :app, :config
attr_reader :app

def build_reloader
reloader_class.new(@paths.files, @paths.directories) do
Expand All @@ -31,12 +30,8 @@ def reloader_class
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
Expand Down
60 changes: 46 additions & 14 deletions spec/factory_bot_rails/reloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,74 @@

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

0 comments on commit bfee5d8

Please sign in to comment.