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 <[email protected]> --- Local-branch: ticket/master/5510 lib/facter/util/loader.rb | 4 ++-- spec/unit/util/loader.rb | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lib/facter/util/loader.rb b/lib/facter/util/loader.rb index ac90c48..b0f43dd 100644 --- a/lib/facter/util/loader.rb +++ b/lib/facter/util/loader.rb @@ -30,7 +30,7 @@ class Facter::Util::Loader 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 @@ class Facter::Util::Loader 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).sort.find_all { |f| f =~ /\.rb$/ }.each do |file| load_file(File.join(dir, file)) end end diff --git a/spec/unit/util/loader.rb b/spec/unit/util/loader.rb index 0a28020..ebd76ea 100755 --- a/spec/unit/util/loader.rb +++ b/spec/unit/util/loader.rb @@ -4,6 +4,19 @@ require File.dirname(__FILE__) + '/../../spec_helper' 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 = {} @@ -107,6 +120,21 @@ describe Facter::Util::Loader do @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 not load files that don't end in '.rb'" do @loader.expects(:search_path).returns %w{/one/dir} @@ -165,6 +193,20 @@ describe Facter::Util::Loader do @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} -- 1.7.0.2 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
