Allow reloading of factory definitions

Closes #236

This commit uses ActiveSupport's FileUpdateChecker to allow reloading
FactoryBot of definitions whenever a file in
`FactoryBot.definition_file_paths` gets updated.
This is similar to reloading for
[I18n](ced104d579/activesupport/lib/active_support/i18n_railtie.rb (L60-L70))
and [react rails](83b6175460/lib/react/rails/railtie.rb (L35-L41))

This allows us to get rid of any Spring-specific logic in the railtie,
since [Spring hooks into the application
reloader](0c711ff10b/lib/spring/application.rb (L161)).

This partly solves #211, since we no longer call `FactoryBot.reload` at
all in `after_initialize`. Instead, we will only call it when one of the
files in `definition_file_paths` gets updated. I say it partly
solves #211 because once a definition file gets updated, reloading
would still give warnings about redefining any constants in the
definition files. I wonder how common it is to define constants in the
same file as factory definitions. It's probably better to keep constants
in the autoload path, allowing Rails to handle unloading and reloading
them in development. I would want to see some specific examples before
worrying too much about it.

I would also like to offer a better way to configure the definition file
paths (see #165 and #166) and possibly an option to opt out of loading
definitions at all (could help with issues like #192).

A couple of quirks here:
* the to_prepare block could potentially get
[called multiple times](https://github.com/rails/rails/issues/28108).
It shouldn't matter, since `execute_if_updated` is a no-op if no
updates have been made.

* I noticed that the first time I call `reload!` in the console the
factory definitions get reloaded even when I made no updates to the
definition files. I think it is related to
[this](f7c5a8ce26/activesupport/lib/active_support/evented_file_update_checker.rb (L18)). After the first call
`reload!` works as expected, only reloading the factory definitions
if the definition files were updated.

* Rails uses execute rather than execute_if_updated for the
[route
reloader](https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L133)
and for the [watchable file
reloader](https://github.com/rails/rails/blob/master/railties/lib/rails/application/finisher.rb#L173).
This means that changes to factory definitions will cause reloading of
the whole application. I think this is fine.
This commit is contained in:
Daniel Colson 2018-07-16 18:31:25 -04:00
parent db7c163efb
commit 02a0f58487
12 changed files with 197 additions and 14 deletions

3
.rspec Normal file
View File

@ -0,0 +1,3 @@
--color
--format progress
--require spec_helper

View File

@ -1,5 +1,6 @@
require 'bundler/setup'
require 'cucumber/rake/task'
require 'rspec/core/rake_task'
Bundler::GemHelper.install_tasks name: 'factory_bot_rails'
@ -8,12 +9,14 @@ Cucumber::Rake::Task.new(:cucumber) do |t|
t.cucumber_opts = ['--format', (ENV['CUCUMBER_FORMAT'] || 'progress')]
end
RSpec::Core::RakeTask.new(:spec)
require 'appraisal'
desc 'Run the test suite'
task :default do |t|
if ENV['BUNDLE_GEMFILE'] =~ /gemfiles/
exec 'rake cucumber'
exec 'rake spec && rake cucumber'
else
Rake::Task['appraise'].execute
end

View File

@ -0,0 +1,17 @@
# frozen_string_literal: true
module FactoryBotRails
class DefinitionFilePaths
attr_reader :files, :directories
def initialize(definition_file_paths)
@files = []
@directories = {}
definition_file_paths.each do |path|
@files << "#{path}.rb"
@directories[path.to_s] = [:rb]
end
end
end
end

View File

@ -1,28 +1,31 @@
require 'factory_bot'
require 'factory_bot_rails/generator'
require 'rails'
# frozen_string_literal: true
module FactoryBot
require "factory_bot"
require "factory_bot_rails/generator"
require "factory_bot_rails/reloader"
require "rails"
module FactoryBotRails
class Railtie < Rails::Railtie
initializer "factory_bot.set_fixture_replacement" do
FactoryBotRails::Generator.new(config).run
Generator.new(config).run
end
initializer "factory_bot.set_factory_paths" do
FactoryBot.definition_file_paths = [
Rails.root.join('factories'),
Rails.root.join('test', 'factories'),
Rails.root.join('spec', 'factories')
Rails.root.join("factories"),
Rails.root.join("test", "factories"),
Rails.root.join("spec", "factories"),
]
end
initializer "factory_bot.register_reloader" do |app|
Reloader.new(app, config).run
end
config.after_initialize do
FactoryBot.find_definitions
if defined?(Spring)
Spring.after_fork { FactoryBot.reload }
end
end
end
end

View File

@ -0,0 +1,40 @@
# frozen_string_literal: true
require "factory_bot_rails/definition_file_paths"
module FactoryBotRails
class Reloader
def initialize(app, config)
@app = app
@config = config
end
def run
register_reloader(build_reloader)
end
private
attr_reader :app, :config
def build_reloader
paths = DefinitionFilePaths.new(FactoryBot.definition_file_paths)
reloader_class.new(paths.files, paths.directories) do
FactoryBot.reload
end
end
def reloader_class
app.config.file_watcher
end
def register_reloader(reloader)
config.to_prepare do
reloader.execute_if_updated
end
app.reloaders << reloader
end
end
end

View File

@ -91,7 +91,7 @@ RUBY
end
def generators
config = FactoryBot::Railtie.config
config = FactoryBotRails::Railtie.config
config.respond_to?(:app_generators) ? config.app_generators : config.generators
end
end

View File

@ -0,0 +1,34 @@
# frozen_string_literal: true
describe FactoryBotRails::DefinitionFilePaths do
describe "#files" do
it "returns a list of definition files" do
definition_file_paths = ["definition_path", "another_definition_path"]
files = described_class.new(definition_file_paths).files
expect(files).to eq ["definition_path.rb", "another_definition_path.rb"]
end
end
describe "#directories" do
it "returns a hash of definition directories" do
definition_file_paths = ["definition_path", "another_definition_path"]
directories = described_class.new(definition_file_paths).directories
expect(directories).to eq(
"definition_path" => [:rb],
"another_definition_path" => [:rb],
)
end
it "converts Pathname objects to strings" do
definition_file_paths = [Pathname.new("definition_path")]
directories = described_class.new(definition_file_paths).directories
expect(directories).to eq("definition_path" => [:rb])
end
end
end

View File

@ -0,0 +1,57 @@
# frozen_string_literal: true
describe FactoryBotRails::Railtie do
describe "application reloading" do
context "when a definition file has been updated" do
it "reloads the factory definitions" do
allow(FactoryBot).to receive(:reload)
touch("factories.rb")
reload_rails!
expect(FactoryBot).to have_received(:reload)
end
end
context "when a file in a definition directory has been updated" do
it "reloads the factory definitions" do
allow(FactoryBot).to receive(:reload)
touch("factories/definitions.rb")
reload_rails!
expect(FactoryBot).to have_received(:reload)
end
end
context "when the factory definitions have NOT been updated" do
it "does NOT reload the factory definitions" do
allow(FactoryBot).to receive(:reload)
reload_rails!
expect(FactoryBot).not_to have_received(:reload)
end
end
def touch(file)
FileUtils.touch(Rails.root.join(file))
end
def reload_rails!
if defined? ActiveSupport::Reloader
Rails.application.reloader.reload!
else
# For Rails 4
ActionDispatch::Reloader.cleanup!
ActionDispatch::Reloader.prepare!
end
wait_for_rails_to_reload
end
def wait_for_rails_to_reload
sleep 0.01
end
end
end

12
spec/fake_app.rb Normal file
View File

@ -0,0 +1,12 @@
# frozen_string_literal: true
module Dummy
class Application < Rails::Application
config.eager_load = false
config.root = "spec/fixtures"
end
end
Rails.logger = Logger.new("/dev/null")
Rails.application.initialize!

1
spec/fixtures/factories.rb vendored Normal file
View File

@ -0,0 +1 @@
# Rails.root.join('factories.rb')

View File

@ -0,0 +1 @@
# Rails.root.join('factories', 'definitions.rb')

12
spec/spec_helper.rb Normal file
View File

@ -0,0 +1,12 @@
# frozen_string_literal: true
ENV["RAILS_ENV"] = "test"
require "factory_bot_rails"
require "fake_app"
RSpec.configure do |config|
config.run_all_when_everything_filtered = true
config.filter_run :focus
config.order = "random"
end