Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

(#5510) Facter should load custom fact definitions in filename order.

Ruby's Dir.entries will return files in different orders depending on
the OS and/or filesystem.  As a result Facter::Util::Loader will load
ruby custom fact definitions in different orders on different platforms.

Specs to expose the bugs, and code to ensure that custom fact files are
loaded in alphabetical order.

Addresses redmine issue #5510

  http://projects.puppetlabs.com/issues/5510

Signed-off-by: Rick Bradley <rick@rickbradley.com>
Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
  • Loading branch information...
commit b88a088c6e53ef96914280e6937b9b9214b6c64b 1 parent d9b8f2a
@rick rick authored committed
Showing with 45 additions and 2 deletions.
  1. +2 −2 lib/facter/util/loader.rb
  2. +43 −0 spec/unit/util/loader_spec.rb
View
4 lib/facter/util/loader.rb
@@ -30,7 +30,7 @@ def load_all
search_path.each do |dir|
next unless FileTest.directory?(dir)
- Dir.entries(dir).each do |file|
+ Dir.entries(dir).sort.each do |file|
path = File.join(dir, file)
if File.directory?(path)
load_dir(path)
@@ -62,7 +62,7 @@ def search_path
def load_dir(dir)
return if dir =~ /\/\.+$/ or dir =~ /\/util$/ or dir =~ /\/lib$/
- Dir.entries(dir).find_all { |f| f =~ /\.rb$/ }.each do |file|
+ Dir.entries(dir).find_all { |f| f =~ /\.rb$/ }.sort.each do |file|
load_file(File.join(dir, file))
end
end
View
43 spec/unit/util/loader_spec.rb
@@ -4,6 +4,20 @@
require 'facter/util/loader'
+
+# loader subclass for making assertions about file/directory ordering
+class TestLoader < Facter::Util::Loader
+ def loaded_files
+ @loaded_files ||= []
+ end
+
+ def load_file(file)
+ loaded_files << file
+ super
+ end
+end
+
+
describe Facter::Util::Loader do
def with_env(values)
old = {}
@@ -96,6 +110,21 @@ def with_env(values)
@loader.load(:testing)
end
+ it 'should load any ruby files in directories matching the fact name in the search path in sorted order regardless of the order returned by Dir.entries' do
+ @loader = TestLoader.new
+
+ @loader.stubs(:search_path).returns %w{/one/dir}
+ FileTest.stubs(:exist?).returns false
+ FileTest.stubs(:directory?).with("/one/dir/testing").returns true
+ @loader.stubs(:search_path).returns %w{/one/dir}
+
+ Dir.stubs(:entries).with("/one/dir/testing").returns %w{foo.rb bar.rb}
+ %w{/one/dir/testing/foo.rb /one/dir/testing/bar.rb}.each { |f| File.stubs(:directory?).with(f).returns false }
+
+ @loader.load(:testing)
+ @loader.loaded_files.should == %w{/one/dir/testing/bar.rb /one/dir/testing/foo.rb}
+ end
+
it "should load any ruby files in directories matching the fact name in the search path" do
@loader.expects(:search_path).returns %w{/one/dir}
FileTest.stubs(:exist?).returns false
@@ -166,6 +195,20 @@ def with_env(values)
@loader.load_all
end
+ it 'should load all files in sorted order for any given directory regardless of the order returned by Dir.entries' do
+ @loader = TestLoader.new
+
+ @loader.stubs(:search_path).returns %w{/one/dir}
+ Dir.stubs(:entries).with("/one/dir").returns %w{foo.rb bar.rb}
+
+ %w{/one/dir}.each { |f| File.stubs(:directory?).with(f).returns true }
+ %w{/one/dir/foo.rb /one/dir/bar.rb}.each { |f| File.stubs(:directory?).with(f).returns false }
+
+ @loader.load_all
+
+ @loader.loaded_files.should == %w{/one/dir/bar.rb /one/dir/foo.rb}
+ end
+
it "should not load files in the util subdirectory" do
@loader.expects(:search_path).returns %w{/one/dir}
Please sign in to comment.
Something went wrong with that request. Please try again.