Skip to content

Commit

Permalink
Do not register a reloader when file paths not exist
Browse files Browse the repository at this point in the history
If all the paths specified in `FactoryBot.definition_file_paths` do not
exist, an empty array is passed to the reloader.

If an empty array is specified, reloader passes it to `Listen.to` as it
is. This causes the application root to be watched and `node_modules` to
become a target of listening.

The behavior when an empty array is passed to reloader depends on the
Rails side. Fixes to not register a reloader when file paths do not exist
consistent behavior regardless of Rails side.

Fixes thoughtbot#328.
  • Loading branch information
y-yagi committed Apr 9, 2019
1 parent dcfc9f6 commit d5b71d0
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 3 deletions.
4 changes: 4 additions & 0 deletions lib/factory_bot_rails/definition_file_paths.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,9 @@ def directories
def files
@files.select { |file| File.exist?(file) }
end

def any?
directories.any? || files.any?
end
end
end
7 changes: 4 additions & 3 deletions lib/factory_bot_rails/reloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@ class Reloader
def initialize(app, config)
@app = app
@config = config
@paths = DefinitionFilePaths.new(FactoryBot.definition_file_paths)
end

def run
return unless @paths.any?

register_reloader(build_reloader)
end

Expand All @@ -18,9 +21,7 @@ def run
attr_reader :app, :config

def build_reloader
paths = DefinitionFilePaths.new(FactoryBot.definition_file_paths)

reloader_class.new(paths.files, paths.directories) do
reloader_class.new(@paths.files, @paths.directories) do
FactoryBot.reload
end
end
Expand Down
10 changes: 10 additions & 0 deletions spec/factory_bot_rails/definition_file_paths_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,14 @@
expect(directories).to eq("spec/fixtures/factories" => [:rb])
end
end

describe "#any?" do
it "returns true only if definition file paths exist" do
definition_file_paths = ["spec/fixtures/factories", "not_exist_directory"]
expect(described_class.new(definition_file_paths).any?).to eq true

definition_file_paths = ["not_exist_directory"]
expect(described_class.new(definition_file_paths).any?).to eq false
end
end
end
53 changes: 53 additions & 0 deletions spec/factory_bot_rails/reloader_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# frozen_string_literal: true

describe FactoryBotRails::Reloader do
describe "#run" do
before do
@original_definition_file_paths = FactoryBot.definition_file_paths
end

after do
FactoryBot.definition_file_paths = @original_definition_file_paths
end

context "when a definition file paths exist" do
it "registers a reloader" do
allow(reloader_class).to receive(:new)

run_reloader(["spec/fixtures/factories", "not_exist_directory"])

expect(reloader_class).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)

run_reloader(["spec/fake_app", "not_exist_directory"])

expect(reloader_class).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)

run_reloader(["not_exist_directory"])

expect(reloader_class).not_to have_received(:new)
end
end

def run_reloader(definition_file_paths)
FactoryBot.definition_file_paths = definition_file_paths
FactoryBotRails::Reloader.
new(Rails.application, Rails.application.config).run
end

def reloader_class
Rails.application.config.file_watcher
end
end
end

0 comments on commit d5b71d0

Please sign in to comment.