Merge pull request #2556 from AriaXLi/FACT-3194/facter_ordering

(FACT-3194) Difference in Facter 3 & 4 ordering

Author: mhashizume

lib/facter.rb

@@ -380,7 +380,14 @@ def to_hash
       resolved_facts = Facter::FactManager.instance.resolve_facts
       resolved_facts.reject! { |fact| fact.type == :custom && fact.value.nil? }
       collection = Facter::FactCollection.new.build_fact_collection!(resolved_facts)
-      Hash[collection]
+
+      # Ensures order of keys in hash returned from Facter.to_hash() and
+      # Facter.resolve() is always stable
+      if collection.empty?
+        Hash[collection]
+      else
+        Hash[Facter::Utils.sort_hash_by_key(collection)]
+      end
     end
 
     # Check whether printing stack trace is enabled
@@ -510,8 +517,10 @@ def queried_facts(user_query)
       resolved_facts = Facter::FactManager.instance.resolve_facts(user_query)
       resolved_facts.reject! { |fact| fact.type == :custom && fact.value.nil? }
 
+      # Ensures order of keys in hash returned from Facter.to_hash() and
+      # Facter.resolve() is always stable
       if user_query.count.zero?
-        Facter::FactCollection.new.build_fact_collection!(resolved_facts)
+        Facter::Utils.sort_hash_by_key(Facter::FactCollection.new.build_fact_collection!(resolved_facts))
       else
         FormatterHelper.retrieve_facts_to_display_for_user_query(user_query, resolved_facts)
       end

spec/facter/facter_spec.rb

@@ -8,10 +8,18 @@
     # note the type is the :nil symbol
     Facter::ResolvedFact.new('missing_fact', nil, :nil, 'missing_fact')
   end
+  let(:hash_fact) do
+    Facter::ResolvedFact.new('mountpoints', { '/boot' => { 'filesystem' => 'ext4', 'device' => 'dev1' },
+                                              '/' => { 'filesystem' => 'ext4', 'device' => 'dev2' } },
+                             :core, 'mountpoints')
+  end
 
   let(:logger) { instance_spy(Facter::Log) }
   let(:config_reader_double) { class_spy(Facter::ConfigReader) }
 
+  sorted_fact_hash = { 'mountpoints' => { '/' => { 'device' => 'dev2', 'filesystem' => 'ext4' },
+                                          '/boot' => { 'device' => 'dev1', 'filesystem' => 'ext4' } } }
+
   before do
     allow(Facter::ConfigReader).to receive(:init).and_return(config_reader_double)
     allow(config_reader_double).to receive(:cli).and_return(nil)
@@ -56,13 +64,32 @@ def stub_no_fact
       expect(Facter.resolve('')).to be_an_instance_of(Hash)
     end
 
+    context 'when user query in arguments' do
+      it 'returns a stable order hash' do
+        stub_facts([hash_fact])
+        expect(Facter.resolve('mountpoints')).to eq(sorted_fact_hash)
+      end
+    end
+
     context 'when user query and options in arguments' do
       it 'returns one resolved fact' do
         stub_facts([os_fact])
 
         expect(Facter.resolve('os.name --show-legacy')).to eq('os.name' => 'ubuntu')
       end
     end
+
+    context 'when there is no user query' do
+      let(:hash_fact_no_user_query) do
+        Facter::ResolvedFact.new('mountpoints', { '/boot' => { 'filesystem' => 'ext4', 'device' => 'dev1' },
+                                                  '/' => { 'filesystem' => 'ext4', 'device' => 'dev2' } }, :core, '')
+      end
+
+      it 'returns a stable order hash' do
+        stub_facts([hash_fact_no_user_query])
+        expect(Facter.resolve('')).to eq(sorted_fact_hash)
+      end
+    end
   end
 
   describe '#to_hash' do
@@ -85,6 +112,11 @@ def stub_no_fact
         expect(Facter.to_hash).to eq({})
       end
     end
+
+    it 'returns a stable order hash' do
+      stub_facts([hash_fact])
+      expect(Facter.to_hash).to eq(sorted_fact_hash)
+    end
   end
 
   describe '#to_user_output' do