(FACT-2499) Only resolve fact once. Fix test.

puppetlabs · Apr 1, 2020 · 93648eb · 93648eb
1 parent 5dec73b
commit 93648eb
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 13 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -379,7 +379,6 @@ RSpec/VerifiedDoubles:
     - 'spec/facter/resolvers/windows/win_os_description_resolver_spec.rb'
     - 'spec/framework/config/block_list_spec.rb'
     - 'spec/framework/core/fact/external/external_fact_manager_spec.rb'
-    - 'spec/framework/core/fact/internal/internal_fact_manager_spec.rb'
     - 'spec/framework/core/fact_loaders/external_fact_loader_spec.rb'
     - 'spec/framework/core/fact_loaders/fact_loader_spec.rb'
     - 'spec/framework/core/fact_manager_spec.rb'

diff --git a/lib/framework/core/fact/internal/internal_fact_manager.rb b/lib/framework/core/fact/internal/internal_fact_manager.rb
@@ -19,10 +19,20 @@ def filter_internal_facts(searched_facts)
     def start_threads(searched_facts)
       threads = []
 
-      searched_facts.reject { |elem| elem.fact_class.nil? }.each do |searched_fact|
+      # only resolve a fact once, even if multiple search facts depend on that fact
+      searched_facts
+        .reject { |elem| elem.fact_class.nil? }
+        .uniq { |searched_fact| searched_fact.fact_class.name }
+        .each do |searched_fact|
         threads << Thread.new do
-          fact = CoreFact.new(searched_fact)
-          fact.create
+          begin
+            fact = CoreFact.new(searched_fact)
+            fact.create
+          rescue StandardError => e
+            @log.error("Error while resolving fact: #{searched_fact.name}, #{e.backtrace}")
+
+            nil
+          end
         end
       end
 

diff --git a/spec/framework/core/fact/internal/internal_fact_manager_spec.rb b/spec/framework/core/fact/internal/internal_fact_manager_spec.rb
@@ -3,15 +3,16 @@
 describe Facter::InternalFactManager do
   describe '#resolve_facts' do
     it 'resolved one core fact' do
-      ubuntu_os_name = double(Facts::Debian::Os::Name)
+      os_name_class_spy = class_spy(Facts::Debian::Os::Name)
+      os_name_instance_spy = instance_spy(Facts::Debian::Os::Name)
 
       resolved_fact = mock_resolved_fact('os', 'Ubuntu', nil, [])
 
-      allow(ubuntu_os_name).to receive(:new).and_return(ubuntu_os_name)
-      allow(ubuntu_os_name).to receive(:call_the_resolver).and_return(resolved_fact)
+      allow(os_name_class_spy).to receive(:new).and_return(os_name_instance_spy)
+      allow(os_name_instance_spy).to receive(:call_the_resolver).and_return(resolved_fact)
 
-      searched_fact = double(Facter::SearchedFact, name: 'os', fact_class: ubuntu_os_name, filter_tokens: [],
-                                                   user_query: '', type: :core)
+      searched_fact = instance_spy(Facter::SearchedFact, name: 'os', fact_class: os_name_class_spy, filter_tokens: [],
+                                                         user_query: '', type: :core)
 
       core_fact_manager = Facter::InternalFactManager.new
       resolved_facts = core_fact_manager.resolve_facts([searched_fact])
@@ -20,15 +21,16 @@
     end
 
     it 'resolved one legacy fact' do
-      windows_networking_interface = double(Facts::Windows::NetworkInterfaces)
+      networking_interface_class_spy = class_spy(Facts::Windows::NetworkInterfaces)
+      windows_networking_interface = instance_spy(Facts::Windows::NetworkInterfaces)
 
       resolved_fact = mock_resolved_fact('network_Ethernet0', '192.168.5.121', nil, [], :legacy)
 
-      allow(windows_networking_interface).to receive(:new).and_return(windows_networking_interface)
+      allow(networking_interface_class_spy).to receive(:new).and_return(windows_networking_interface)
       allow(windows_networking_interface).to receive(:call_the_resolver).and_return(resolved_fact)
 
-      searched_fact = double(Facter::SearchedFact, name: 'network_.*', fact_class: windows_networking_interface,
-                                                   filter_tokens: [], user_query: '', type: :core)
+      searched_fact = instance_spy(Facter::SearchedFact, name: 'network_.*', fact_class: networking_interface_class_spy,
+                                                         filter_tokens: [], user_query: '', type: :core)
 
       core_fact_manager = Facter::InternalFactManager.new
       resolved_facts = core_fact_manager.resolve_facts([searched_fact])