From 06b1d5b120642720c4a291f97e970d6cfdb5930c Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Thu, 16 May 2024 19:43:06 -0700 Subject: [PATCH 1/5] (FACT-3465) Fix order dependent test failure on Windows The uname spec set a logger class variable on the uname resolver which leaked into the processor spec. This wasn't noticed until the processor resolver was modified to call the uname resolver. So the instance spy from the uname resolver spec leaked into the processor spec: C:\> bundle exec rspec -fd .\spec\facter\resolvers\uname_spec.rb:32 .\spec\facter\resolvers\processors_spec.rb:182 ... 1) Facter::Resolvers::Linux::Processors when on powerpc architecture returns list of models Failure/Error: logger.debug(format(STDERR_MESSAGE, command, msg.strip)) # was originally created in one example but has leaked into another example and can no longer be used. --- lib/facter/resolvers/uname.rb | 2 ++ spec/facter/resolvers/uname_spec.rb | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/lib/facter/resolvers/uname.rb b/lib/facter/resolvers/uname.rb index 49dd3809c5..8213dcb9fc 100644 --- a/lib/facter/resolvers/uname.rb +++ b/lib/facter/resolvers/uname.rb @@ -3,6 +3,8 @@ module Facter module Resolvers class Uname < BaseResolver + @log = Facter::Log.new(self) + init_resolver class << self diff --git a/spec/facter/resolvers/uname_spec.rb b/spec/facter/resolvers/uname_spec.rb index 59e2af6769..bfc3f9fcdc 100644 --- a/spec/facter/resolvers/uname_spec.rb +++ b/spec/facter/resolvers/uname_spec.rb @@ -22,6 +22,11 @@ Darwin Kernel Version 18.2.0: Fri Oct 5 19:41:49 PDT 2018; root:xnu-4903.221.2~2/RELEASE_X86_64') end + after do + uname_resolver.instance_variable_set(:@log, nil) + Facter::Resolvers::Uname.invalidate_cache + end + it 'returns machine' do expect(uname_resolver.resolve(:machine)).to eq('x86_64') end From e5776fc43d9616acf049c460b1d0c8d188d5f66b Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Thu, 16 May 2024 19:43:26 -0700 Subject: [PATCH 2/5] (FACT-3465) Stub Socket.tcp on Windows Running the ec2_spec test in isolation failed on Windows because the call to Socket.tcp would timeout and skip the rest of the code being tested. --- spec/facter/resolvers/ec2_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/facter/resolvers/ec2_spec.rb b/spec/facter/resolvers/ec2_spec.rb index 31305b1276..85d27a6106 100644 --- a/spec/facter/resolvers/ec2_spec.rb +++ b/spec/facter/resolvers/ec2_spec.rb @@ -11,6 +11,7 @@ before do Facter::Util::Resolvers::Http.instance_variable_set(:@log, log_spy) + allow(Socket).to receive(:tcp) if Gem.win_platform? end after do From e39513acbca0eb33518cbd852b87f461d8600e12 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Thu, 16 May 2024 19:45:13 -0700 Subject: [PATCH 3/5] (FACT-3465) Allow File.exist? and Dir.entries to receive other arguments The RSpec `allow` method creates a proxy object to wrap the original object. This way it can intercept calls to the original object that are "allowed". However, if the proxy receives a method that isn't allowed, then it will raise an RSpec exception. Several tests were rescuing the RSpec exception, but still passing (for the wrong reason). The rule is if you stub an object and it might be called with other arguments, then you need to add `and_call_original`. This is especially true for File.exist? because it's called by rubygems. --- spec/facter/resolvers/processors_spec.rb | 10 ++++++++++ spec/facter/resolvers/xen_spec.rb | 1 + spec/facter/util/linux/if_inet6_spec.rb | 2 ++ 3 files changed, 13 insertions(+) diff --git a/spec/facter/resolvers/processors_spec.rb b/spec/facter/resolvers/processors_spec.rb index 246f3699c9..972d25e369 100644 --- a/spec/facter/resolvers/processors_spec.rb +++ b/spec/facter/resolvers/processors_spec.rb @@ -52,8 +52,10 @@ allow(Facter::Util::FileHelper).to receive(:safe_readlines) .with('/proc/cpuinfo') .and_return(load_fixture('cpuinfo_wo_physical_id').readlines) + allow(Dir).to receive(:entries).and_call_original allow(Dir).to receive(:entries).with('/sys/devices/system/cpu').and_return(%w[cpu0 cpu1 cpuindex]) + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?) .with('/sys/devices/system/cpu/cpu0/topology/physical_package_id') .and_return(true) @@ -117,9 +119,13 @@ .with('/proc/cpuinfo') .and_return(load_fixture('cpuinfo_powerpc').readlines) + allow(Dir).to receive(:entries).and_call_original allow(Dir).to receive(:entries).with('/sys/devices/system/cpu').and_return(%w[cpu0 cpu1 cpuindex]) + + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with('/sys/devices/system/cpu/cpu0/topology/physical_package_id').and_return(true) allow(File).to receive(:exist?).with('/sys/devices/system/cpu/cpu1/topology/physical_package_id').and_return(true) + allow(Facter::Util::FileHelper).to receive(:safe_read) .with('/sys/devices/system/cpu/cpu0/topology/physical_package_id') .and_return('0') @@ -164,9 +170,13 @@ .with('/proc/cpuinfo') .and_return(load_fixture('cpuinfo_arm64').readlines) + allow(Dir).to receive(:entries).and_call_original allow(Dir).to receive(:entries).with('/sys/devices/system/cpu').and_return(%w[cpu0 cpu1 cpuindex]) + + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with('/sys/devices/system/cpu/cpu0/topology/physical_package_id').and_return(true) allow(File).to receive(:exist?).with('/sys/devices/system/cpu/cpu1/topology/physical_package_id').and_return(true) + allow(Facter::Util::FileHelper).to receive(:safe_read) .with('/sys/devices/system/cpu/cpu0/topology/physical_package_id') .and_return('0') diff --git a/spec/facter/resolvers/xen_spec.rb b/spec/facter/resolvers/xen_spec.rb index 222b6de1ab..c94132250e 100644 --- a/spec/facter/resolvers/xen_spec.rb +++ b/spec/facter/resolvers/xen_spec.rb @@ -10,6 +10,7 @@ before do xen_resolver.instance_variable_set(:@log, log_spy) + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with('/dev/xen/evtchn').and_return(evtchn_file) allow(File).to receive(:exist?).with('/proc/xen').and_return(proc_xen_file) allow(File).to receive(:exist?).with('/dev/xvda1').and_return(xvda1_file) diff --git a/spec/facter/util/linux/if_inet6_spec.rb b/spec/facter/util/linux/if_inet6_spec.rb index 5adcd3fcd2..61f60c10e7 100644 --- a/spec/facter/util/linux/if_inet6_spec.rb +++ b/spec/facter/util/linux/if_inet6_spec.rb @@ -28,6 +28,7 @@ describe '#read_flags' do context 'when only ipv6 link-local and lo ipv6 present' do before do + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with('/proc/net/if_inet6').and_return(true) allow(Facter::Util::FileHelper).to receive(:safe_read) .with('/proc/net/if_inet6', nil).and_return(load_fixture('proc_net_if_inet6').read) @@ -38,6 +39,7 @@ context 'when multiple IPv6 addresses present with different flags' do before do + allow(File).to receive(:exist?).and_call_original allow(File).to receive(:exist?).with('/proc/net/if_inet6').and_return(true) allow(Facter::Util::FileHelper).to receive(:safe_read) .with('/proc/net/if_inet6', nil).and_return(load_fixture('proc_net_if_inet6_complex').read) From 2ee7f00c716ab4d4c3f10cf9603477c9ada21456 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Thu, 16 May 2024 19:45:42 -0700 Subject: [PATCH 4/5] (FACT-3465) Use instance_spy's consistently We use instance_spys for the logger everywhere else. --- spec/facter/resolvers/amzn/os_release_rpm_spec.rb | 2 +- spec/facter/resolvers/uname_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/facter/resolvers/amzn/os_release_rpm_spec.rb b/spec/facter/resolvers/amzn/os_release_rpm_spec.rb index b2ca9073ad..be2e24d672 100644 --- a/spec/facter/resolvers/amzn/os_release_rpm_spec.rb +++ b/spec/facter/resolvers/amzn/os_release_rpm_spec.rb @@ -3,7 +3,7 @@ describe Facter::Resolvers::Amzn::OsReleaseRpm do subject(:os_release_resolver) { Facter::Resolvers::Amzn::OsReleaseRpm } - let(:log_spy) { Facter::Log } + let(:log_spy) { instance_spy(Facter::Log) } before do os_release_resolver.instance_variable_set(:@log, log_spy) diff --git a/spec/facter/resolvers/uname_spec.rb b/spec/facter/resolvers/uname_spec.rb index bfc3f9fcdc..e035d58237 100644 --- a/spec/facter/resolvers/uname_spec.rb +++ b/spec/facter/resolvers/uname_spec.rb @@ -3,7 +3,7 @@ describe Facter::Resolvers::Uname do subject(:uname_resolver) { Facter::Resolvers::Uname } - let(:log_spy) { Facter::Log } + let(:log_spy) { instance_spy(Facter::Log) } before do uname_resolver.instance_variable_set(:@log, log_spy) From d857633679fbd523c5f972b806d4209ebe28100f Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Thu, 16 May 2024 19:46:02 -0700 Subject: [PATCH 5/5] (FACT-3465) Handle case where we're not xen Previously if neither /sur/sbin/xl nor xm existed, then the `find_command` method returned the array of XEN_COMMANDS. Since that wasn't nil, the resolver tried to execute the Array of commands and failed (for the wrong reason). Now explicitly return nil in the case neither file exists. --- lib/facter/resolvers/xen.rb | 2 ++ spec/facter/resolvers/xen_spec.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/lib/facter/resolvers/xen.rb b/lib/facter/resolvers/xen.rb index 2f02bfa01a..cb2125322f 100644 --- a/lib/facter/resolvers/xen.rb +++ b/lib/facter/resolvers/xen.rb @@ -66,6 +66,8 @@ def find_command return XEN_TOOLSTACK if num_stacks > 1 && File.exist?(XEN_TOOLSTACK) XEN_COMMANDS.each { |command| return command if File.exist?(command) } + + nil end end end diff --git a/spec/facter/resolvers/xen_spec.rb b/spec/facter/resolvers/xen_spec.rb index c94132250e..3b3db6b9f7 100644 --- a/spec/facter/resolvers/xen_spec.rb +++ b/spec/facter/resolvers/xen_spec.rb @@ -27,6 +27,18 @@ xen_resolver.invalidate_cache end + context 'when not xen' do + let(:evtchn_file) { false } + let(:proc_xen_file) { false } + let(:xvda1_file) { false } + + it 'returns' do + allow(File).to receive(:exist?).with('/usr/sbin/xm').and_return(false) + + expect(xen_resolver.resolve(:vm)).to be_nil + end + end + context 'when xen is privileged' do context 'when /dev/xen/evtchn exists' do let(:domains) { load_fixture('xen_domains').read }