Merge branch 'bug/next/5510-deterministic-fact-load-order' into next

2 files changed, 45 insertions, 2 deletions

diff --git a/lib/facter/util/loader.rb b/lib/facter/util/loader.rb
index ac90c48..2d2d9e8 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).find_all { |f| f =~ /\.rb$/ }.sort.each do |file|
             load_file(File.join(dir, file))
         end
     end
diff --git a/spec/unit/util/loader_spec.rb b/spec/unit/util/loader_spec.rb
index 0bb823e..eed533a 100755
--- a/spec/unit/util/loader_spec.rb
+++ b/spec/unit/util/loader_spec.rb
@@ -4,6 +4,20 @@ require File.expand_path(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 = {}
@@ -96,6 +110,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 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 @@ 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}