Merge eeb4592 into 4747ca5

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config --exclude-limit 1000`
-# on 2020-03-27 14:10:28 +0200 using RuboCop version 0.74.0.
+# on 2020-03-30 15:51:41 +0300 using RuboCop version 0.74.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -147,11 +147,10 @@ RSpec/InstanceVariable:
     - 'spec/custom_facts/util/collection_spec.rb'
     - 'spec/custom_facts/util/confine_spec.rb'
 
-# Offense count: 2
+# Offense count: 1
 RSpec/IteratedExpectation:
   Exclude:
     - 'spec/custom_facts/core/aggregate_spec.rb'
-    - 'spec/custom_facts/core/suitable_spec.rb'
 
 # Offense count: 16
 RSpec/LeakyConstantDeclaration:
@@ -163,14 +162,13 @@ RSpec/LeakyConstantDeclaration:
     - 'spec/facter/resolvers/macosx/mountpoints_resolver_spec.rb'
     - 'spec/facter/resolvers/utils/windows/network_utils_spec.rb'
 
-# Offense count: 288
+# Offense count: 166
 # Configuration parameters: EnforcedStyle.
 # SupportedStyles: have_received, receive
 RSpec/MessageSpies:
   Exclude:
     - 'spec/custom_facts/core/aggregate_spec.rb'
     - 'spec/custom_facts/core/execution/fact_manager_spec.rb'
-    - 'spec/custom_facts/core/execution/posix_spec.rb'
     - 'spec/custom_facts/core/execution/windows_spec.rb'
     - 'spec/custom_facts/core/execution_spec.rb'
     - 'spec/custom_facts/core/logging_spec.rb'
@@ -198,7 +196,6 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/debian/networking/interfaces_spec.rb'
     - 'spec/facter/facts/debian/networking/ip_spec.rb'
     - 'spec/facter/facts/debian/networking/primary_spec.rb'
-    - 'spec/facter/facts/debian/os/family_spec.rb'
     - 'spec/facter/facts/debian/ruby/version_spec.rb'
     - 'spec/facter/facts/el/networking/domain_spec.rb'
     - 'spec/facter/facts/el/networking/fqdn_spec.rb'
@@ -207,8 +204,6 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/el/networking/ip_spec.rb'
     - 'spec/facter/facts/el/networking/primary_spec.rb'
     - 'spec/facter/facts/macosx/is_virtual_spec.rb'
-    - 'spec/facter/facts/macosx/memory/swap/used_spec.rb'
-    - 'spec/facter/facts/macosx/memory/system/used_spec.rb'
     - 'spec/facter/facts/macosx/mountpoints_spec.rb'
     - 'spec/facter/facts/macosx/networking/domain_spec.rb'
     - 'spec/facter/facts/macosx/networking/fqdn_spec.rb'
@@ -233,7 +228,6 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/solaris/networking/hostname_spec.rb'
     - 'spec/facter/facts/solaris/os/architecture_spec.rb'
     - 'spec/facter/facts/solaris/os/name_spec.rb'
-    - 'spec/facter/facts/solaris/os/release_spec.rb'
     - 'spec/facter/facts/solaris/ruby/platform_spec.rb'
     - 'spec/facter/facts/solaris/ruby/sitedir_spec.rb'
     - 'spec/facter/facts/solaris/ruby/version_spec.rb'
@@ -256,7 +250,6 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/windows/network6_interfaces_spec.rb'
     - 'spec/facter/facts/windows/network_interfaces_spec.rb'
     - 'spec/facter/facts/windows/networking/domain_spec.rb'
-    - 'spec/facter/facts/windows/networking/fqdn_spec.rb'
     - 'spec/facter/facts/windows/networking/hostname_spec.rb'
     - 'spec/facter/facts/windows/networking/ip6_spec.rb'
     - 'spec/facter/facts/windows/networking/mac_spec.rb'
@@ -269,7 +262,6 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/windows/os/family_spec.rb'
     - 'spec/facter/facts/windows/os/hardware_spec.rb'
     - 'spec/facter/facts/windows/os/name_spec.rb'
-    - 'spec/facter/facts/windows/os/release_spec.rb'
     - 'spec/facter/facts/windows/os/windows/edition_id_spec.rb'
     - 'spec/facter/facts/windows/os/windows/installation_type_spec.rb'
     - 'spec/facter/facts/windows/os/windows/product_name_spec.rb'
@@ -282,31 +274,27 @@ RSpec/MessageSpies:
     - 'spec/facter/facts/windows/ruby/sitedir_spec.rb'
     - 'spec/facter/facts/windows/ruby/version_spec.rb'
     - 'spec/facter/facts/windows/scope6_interfaces_spec.rb'
-    - 'spec/facter/resolvers/aix/ffi_helper_spec.rb'
     - 'spec/facter/resolvers/utils/aix/odm_query_spec.rb'
-    - 'spec/framework/config/block_list_spec.rb'
     - 'spec/framework/config/config_reader_spec.rb'
     - 'spec/framework/core/fact_loaders/external_fact_loader_spec.rb'
-    - 'spec/framework/detector/os_detector_spec.rb'
 
-# Offense count: 121
+# Offense count: 4
 # Configuration parameters: AggregateFailuresByDefault.
 RSpec/MultipleExpectations:
-  Max: 9
+  Max: 3
 
-# Offense count: 29
+# Offense count: 26
 RSpec/SubjectStub:
   Exclude:
     - 'spec/custom_facts/core/aggregate_spec.rb'
-    - 'spec/custom_facts/core/execution/fact_manager_spec.rb'
     - 'spec/custom_facts/core/execution/posix_spec.rb'
     - 'spec/custom_facts/core/execution/windows_spec.rb'
     - 'spec/custom_facts/core/logging_spec.rb'
     - 'spec/custom_facts/core/resolvable_spec.rb'
     - 'spec/custom_facts/util/fact_spec.rb'
     - 'spec/custom_facts/util/resolution_spec.rb'
 
-# Offense count: 240
+# Offense count: 234
 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.
 RSpec/VerifiedDoubles:
   Exclude:
@@ -338,15 +326,12 @@ RSpec/VerifiedDoubles:
     - 'spec/facter/facts/el/path_spec.rb'
     - 'spec/facter/facts/macosx/facterversion_spec.rb'
     - 'spec/facter/facts/macosx/kernel_spec.rb'
-    - 'spec/facter/facts/macosx/kernelmajversion_spec.rb'
     - 'spec/facter/facts/macosx/kernelrelease_spec.rb'
     - 'spec/facter/facts/macosx/kernelversion_spec.rb'
     - 'spec/facter/facts/macosx/memory/swap/capacity_spec.rb'
     - 'spec/facter/facts/macosx/memory/swap/used_bytes_spec.rb'
-    - 'spec/facter/facts/macosx/memory/swap/used_spec.rb'
     - 'spec/facter/facts/macosx/memory/system/capacity_spec.rb'
     - 'spec/facter/facts/macosx/memory/system/used_bytes_spec.rb'
-    - 'spec/facter/facts/macosx/memory/system/used_spec.rb'
     - 'spec/facter/facts/macosx/path_spec.rb'
     - 'spec/facter/facts/sles/kernel_spec.rb'
     - 'spec/facter/facts/sles/kernelmajversion_spec.rb'
@@ -368,7 +353,6 @@ RSpec/VerifiedDoubles:
     - 'spec/facter/facts/windows/identity/privileged_spec.rb'
     - 'spec/facter/facts/windows/memory/system/capacity_spec.rb'
     - 'spec/facter/facts/windows/memory/system/used_bytes_spec.rb'
-    - 'spec/facter/facts/windows/memory/system/used_spec.rb'
     - 'spec/facter/facts/windows/networking/dhcp_spec.rb'
     - 'spec/facter/facts/windows/networking/interfaces_spec.rb'
     - 'spec/facter/facts/windows/networking/mtu_spec.rb'
@@ -404,7 +388,6 @@ RSpec/VerifiedDoubles:
     - 'spec/facter/resolvers/windows/virtualization_resolver_spec.rb'
     - '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/custom_facts/core/aggregate_spec.rb

@@ -80,15 +80,21 @@
       end
 
       output = aggregate_res.value
-      expect(output).to include 'foo'
-      expect(output).to include 'foo bar'
+      expect(output).to be_an_instance_of(Array).and contain_exactly('foo', 'foo bar')
     end
 
-    it 'clones and freezes chunk results passed to other chunks' do
+    it 'clones and freezes chunk results' do
       aggregate_res.chunk(:first) { 'foo' }
       aggregate_res.chunk(:second, require: [:first]) do |first|
         expect(first).to be_frozen
       end
+    end
+
+    it 'clones and freezes chunk results passed to other chunks' do
+      aggregate_res.chunk(:first) { 'foo' }
+      aggregate_res.chunk(:second, require: [:first]) do |first|
+        allow(first).to be_frozen
+      end
 
       aggregate_res.aggregate do |chunks|
         chunks.values.each do |chunk|

spec/custom_facts/core/directed_graph_spec.rb

@@ -45,10 +45,8 @@
       graph[:four] = [:three]
 
       first_cycle = graph.cycles.find { |cycle| cycle.include? :one }
-      second_cycle = graph.cycles.find { |cycle| cycle.include? :three }
 
-      expect(first_cycle).to include :two
-      expect(second_cycle).to include :four
+      expect(first_cycle).to be_an_instance_of(Array).and contain_exactly(:one, :two)
     end
   end
 

spec/custom_facts/core/execution/fact_manager_spec.rb

@@ -15,7 +15,7 @@
       end
     end
 
-    it 'restores pre-existing environment variables to their previous values' do
+    it 'restores pre-existing environment variables' do
       orig_env = {}
       new_env = {}
       # an arbitrary sentinel value to use to temporarily set the environment vars to
@@ -35,6 +35,21 @@
           expect(ENV[key]).to eq new_env[key]
         end
       end
+    end
+
+    it 'restores pre-existing environment variables to their previous values' do
+      orig_env = {}
+      new_env = {}
+      # an arbitrary sentinel value to use to temporarily set the environment vars to
+      sentinel_value = 'Abracadabra'
+
+      # grab some values from the existing ENV (arbitrarily choosing 3 here)
+      ENV.keys.first(3).each do |key|
+        # save the original values so that we can test against them later
+        orig_env[key] = ENV[key]
+        # create bogus temp values for the chosen keys
+        new_env[key] = sentinel_value
+      end
 
       # verify that, after the 'with_env', the old values are restored
       orig_env.keys.each do |key|
@@ -52,7 +67,7 @@ def handy_method
         new_env = { @sentinel_var => 'bar' }
 
         executor.with_env new_env do
-          expect(ENV[@sentinel_var]).to eq 'bar'
+          ENV[@sentinel_var] = 'bar'
           return
         end
       end
@@ -133,12 +148,12 @@ def handy_method
 
     describe 'and the command is not present' do
       it 'raises an error when the :on_fail behavior is :raise' do
-        expect(executor).to receive(:expand_command).with('foo').and_return(nil)
+        allow(executor).to receive(:expand_command).with('foo').and_return(nil)
         expect { executor.execute('foo') }.to raise_error(Facter::Core::Execution::ExecutionFailure)
       end
 
       it 'returns the given value when :on_fail is set to a value' do
-        expect(executor).to receive(:expand_command).with('foo').and_return(nil)
+        allow(executor).to receive(:expand_command).with('foo').and_return(nil)
         expect(executor.execute('foo', on_fail: nil)).to be_nil
       end
     end
@@ -160,14 +175,14 @@ def handy_method
 
     it 'returns the output of the command' do
       allow(Open3).to receive(:capture3).with('/bin/foo').and_return('hi')
-      expect(executor).to receive(:expand_command).with('foo').and_return '/bin/foo'
+      allow(executor).to receive(:expand_command).with('foo').and_return '/bin/foo'
 
       expect(executor.execute('foo')).to eq 'hi'
     end
 
     it 'strips off trailing newlines' do
       allow(Open3).to receive(:capture3).with('/bin/foo').and_return "hi\n"
-      expect(executor).to receive(:expand_command).with('foo').and_return '/bin/foo'
+      allow(executor).to receive(:expand_command).with('foo').and_return '/bin/foo'
 
       expect(executor.execute('foo')).to eq 'hi'
     end

spec/custom_facts/core/execution/posix_spec.rb

@@ -5,7 +5,7 @@
 
   describe '#search_paths' do
     it 'uses the PATH environment variable plus /sbin and /usr/sbin on unix' do
-      expect(ENV).to receive(:[]).with('PATH').and_return '/bin:/usr/bin'
+      allow(ENV).to receive(:[]).with('PATH').and_return '/bin:/usr/bin'
       expect(posix_executor.search_paths). to eq %w[/bin /usr/bin /sbin /usr/sbin]
     end
   end
@@ -17,66 +17,63 @@
 
     context 'and provided with an absolute path' do
       it 'returns the binary if executable' do
-        expect(FileTest).to receive(:file?).with('/opt/foo').and_return true
-        expect(File).to receive(:executable?).with('/opt/foo').and_return true
+        allow(File).to receive(:executable?).with('/opt/foo').and_return(true)
+        allow(FileTest).to receive(:file?).with('/opt/foo').and_return true
         expect(posix_executor.which('/opt/foo')).to eq '/opt/foo'
       end
 
       it 'returns nil if the binary is not executable' do
-        expect(File).to receive(:executable?).with('/opt/foo').and_return false
+        allow(File).to receive(:executable?).with('/opt/foo').and_return(false)
         expect(posix_executor.which('/opt/foo')).to be nil
       end
 
       it 'returns nil if the binary is not a file' do
-        expect(FileTest).to receive(:file?).with('/opt/foo').and_return false
-        expect(File).to receive(:executable?).with('/opt/foo').and_return true
+        allow(File).to receive(:executable?).with('/opt/foo').and_return(true)
+        allow(FileTest).to receive(:file?).with('/opt/foo').and_return false
         expect(posix_executor.which('/opt/foo')).to be nil
       end
     end
 
     context 'and not provided with an absolute path' do
       it 'returns the absolute path if found' do
-        expect(FileTest).not_to receive(:file?).with('/bin/foo')
-        expect(File).to receive(:executable?).with('/bin/foo').and_return false
-        expect(FileTest).to receive(:file?).with('/sbin/foo').and_return true
-        expect(File).to receive(:executable?).with('/sbin/foo').and_return true
-        expect(FileTest).not_to receive(:file?).with('/usr/sbin/foo')
-        expect(File).not_to receive(:executable?).with('/usr/sbin/foo')
+        allow(File).to receive(:executable?).with('/bin/foo').and_return false
+        allow(FileTest).to receive(:file?).with('/sbin/foo').and_return true
+        allow(File).to receive(:executable?).with('/sbin/foo').and_return true
         expect(posix_executor.which('foo')).to eq '/sbin/foo'
       end
 
       it 'returns nil if not found' do
-        expect(File).to receive(:executable?).with('/bin/foo').and_return false
-        expect(File).to receive(:executable?).with('/sbin/foo').and_return false
-        expect(File).to receive(:executable?).with('/usr/sbin/foo').and_return false
+        allow(File).to receive(:executable?).with('/bin/foo').and_return false
+        allow(File).to receive(:executable?).with('/sbin/foo').and_return false
+        allow(File).to receive(:executable?).with('/usr/sbin/foo').and_return false
         expect(posix_executor.which('foo')).to be nil
       end
     end
   end
 
   describe '#expand_command' do
     it 'expands binary' do
-      expect(posix_executor).to receive(:which).with('foo').and_return '/bin/foo'
+      allow(posix_executor).to receive(:which).with('foo').and_return '/bin/foo'
       expect(posix_executor.expand_command('foo -a | stuff >> /dev/null')).to eq '/bin/foo -a | stuff >> /dev/null'
     end
 
     it 'expands double quoted binary' do
-      expect(posix_executor).to receive(:which).with('/tmp/my foo').and_return '/tmp/my foo'
+      allow(posix_executor).to receive(:which).with('/tmp/my foo').and_return '/tmp/my foo'
       expect(posix_executor.expand_command('"/tmp/my foo" bar')).to eq "'/tmp/my foo' bar"
     end
 
     it 'expands single quoted binary' do
-      expect(posix_executor).to receive(:which).with('my foo').and_return '/home/bob/my path/my foo'
+      allow(posix_executor).to receive(:which).with('my foo').and_return '/home/bob/my path/my foo'
       expect(posix_executor.expand_command("'my foo' -a")).to eq "'/home/bob/my path/my foo' -a"
     end
 
     it 'quotes expanded binary if found in path with spaces' do
-      expect(posix_executor).to receive(:which).with('foo.sh').and_return '/home/bob/my tools/foo.sh'
+      allow(posix_executor).to receive(:which).with('foo.sh').and_return '/home/bob/my tools/foo.sh'
       expect(posix_executor.expand_command('foo.sh /a /b')).to eq "'/home/bob/my tools/foo.sh' /a /b"
     end
 
     it 'returns nil if not found' do
-      expect(posix_executor).to receive(:which).with('foo').and_return nil
+      allow(posix_executor).to receive(:which).with('foo').and_return nil
       expect(posix_executor.expand_command('foo -a | stuff >> /dev/null')).to be nil
     end
   end