From bfee5d8dcc740cd1269dd3bcbc6a7e2e5c754705 Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Mon, 15 Jun 2020 10:41:04 -0400 Subject: [PATCH] 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]: 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 --- lib/factory_bot_rails/railtie.rb | 9 ++-- lib/factory_bot_rails/reloader.rb | 13 ++---- spec/factory_bot_rails/reloader_spec.rb | 60 +++++++++++++++++++------ 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/lib/factory_bot_rails/railtie.rb b/lib/factory_bot_rails/railtie.rb index 2e9cd81..8dca2f4 100644 --- a/lib/factory_bot_rails/railtie.rb +++ b/lib/factory_bot_rails/railtie.rb @@ -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 diff --git a/lib/factory_bot_rails/reloader.rb b/lib/factory_bot_rails/reloader.rb index dacb94c..57d13d7 100644 --- a/lib/factory_bot_rails/reloader.rb +++ b/lib/factory_bot_rails/reloader.rb @@ -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 diff --git a/spec/factory_bot_rails/reloader_spec.rb b/spec/factory_bot_rails/reloader_spec.rb index 70010d1..30ef75d 100644 --- a/spec/factory_bot_rails/reloader_spec.rb +++ b/spec/factory_bot_rails/reloader_spec.rb @@ -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