From: Bruce Williams <[email protected]> The failures were caused by a couple issues that may have been limited to OSX (possibly only SL). * The absence of /bin/true caused a couple Resolution.exec tests to fail. Instead of expecting a binary to exist, the exists? check is now stubbed. * Changes to system_profiler output or the Resolution.exec result type caused in an exception with the plist parser (which would cause Factor.list, among other things, to fail); this is now handled gracefully.
Signed-off-by: Bruce Williams <[email protected]> --- .gitignore | 1 + Rakefile | 4 +--- lib/facter/util/macosx.rb | 17 ++++++++--------- lib/facter/util/resolution.rb | 14 +++++++++----- spec/spec.opts | 1 + spec/unit/interfaces.rb | 12 ++---------- spec/unit/operatingsystem.rb | 4 ---- spec/unit/selinux.rb | 4 ---- spec/unit/util/resolution.rb | 38 +++++++++++++++++++++++--------------- spec/unit/virtual.rb | 3 --- 10 files changed, 45 insertions(+), 53 deletions(-) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..4ebc8ae --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +coverage diff --git a/Rakefile b/Rakefile index e3549ca..d1b7e56 100644 --- a/Rakefile +++ b/Rakefile @@ -47,9 +47,7 @@ end Rake::GemPackageTask.new(spec) do |pkg| end -task :default do - sh %{rake -T} -end +task :default => :spec desc "Run the specs under spec/" task :spec do diff --git a/lib/facter/util/macosx.rb b/lib/facter/util/macosx.rb index 6754f18..7300afe 100644 --- a/lib/facter/util/macosx.rb +++ b/lib/facter/util/macosx.rb @@ -25,7 +25,7 @@ module Facter::Util::Macosx # by looking at the _name key of the _items dict for each _dataType def self.profiler_xml(data_field) - Facter::Util::Resolution.exec("/usr/sbin/system_profiler -xml #{data_field}") + Array(Facter::Util::Resolution.exec("/usr/sbin/system_profiler -xml #{data_field}")).join("\n") end def self.intern_xml(xml) @@ -35,14 +35,13 @@ module Facter::Util::Macosx # Return an xml result, modified as we need it. def self.profiler_data(data_field) - begin - return nil unless parsed_xml = intern_xml(profiler_xml(data_field)) - return nil unless data = parsed_xml[0]['_items'][0] - data.delete '_name' - data - rescue - return nil - end + return nil unless parsed_xml = intern_xml(profiler_xml(data_field)) + return nil unless data = parsed_xml[0]['_items'][0] + data.delete '_name' + data + rescue + # If there's an exception, this field isn't available or is + # incorrectly formatted. end def self.hardware_overview diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/resolution.rb index f6afce6..862ea37 100644 --- a/lib/facter/util/resolution.rb +++ b/lib/facter/util/resolution.rb @@ -39,26 +39,30 @@ class Facter::Util::Resolution path = binary end - return nil unless FileTest.exists?(path) + return nil unless binary_exists?(path) end out = nil err = nil begin - Open3.popen3(code) do |stdin,stdout,stderr| - out = self.parse_output(stdout.readlines) - err = self.parse_output(stderr.readlines) + Open3.popen3(code) do |stdin, stdout, stderr| + out = self.parse_output(stdout.readlines) + err = self.parse_output(stderr.readlines) end rescue => detail $stderr.puts detail return nil end Facter.warn(err) - + return nil if out == "" out end + def self.binary_exists?(path) + File.exists?(path) + end + def self.parse_output(output) return nil unless output and output.size > 0 result = output.collect{|line| line.chomp } diff --git a/spec/spec.opts b/spec/spec.opts index 695852c..91cd642 100644 --- a/spec/spec.opts +++ b/spec/spec.opts @@ -3,3 +3,4 @@ s --colour --loadby mtime +--backtrace diff --git a/spec/unit/interfaces.rb b/spec/unit/interfaces.rb index 49d5d1f..bf8d39a 100644 --- a/spec/unit/interfaces.rb +++ b/spec/unit/interfaces.rb @@ -2,18 +2,10 @@ require File.dirname(__FILE__) + '/../spec_helper' -require 'facter' - describe "Per Interface IP facts" do - before do - Facter.loadfacts - end - it "should replace the ':' in an interface list with '_'" do - # So we look supported - Facter.fact(:kernel).stubs(:value).returns("SunOS") - - Facter::Util::IP.expects(:get_interfaces).returns %w{eth0:1 eth1:2} + Facter.fact(:kernel).stubs(:value => "SunOS") + Facter::Util::IP.stubs(:get_interfaces => %w{eth0:1 eth1:2}) Facter.fact(:interfaces).value.should == %{eth0_1,eth1_2} end end diff --git a/spec/unit/operatingsystem.rb b/spec/unit/operatingsystem.rb index de86230..ab21682 100644 --- a/spec/unit/operatingsystem.rb +++ b/spec/unit/operatingsystem.rb @@ -1,9 +1,5 @@ -#!/usr/bin/env ruby - require File.dirname(__FILE__) + '/../spec_helper' -require 'facter' - describe "Operating System fact" do before do diff --git a/spec/unit/selinux.rb b/spec/unit/selinux.rb index 8afa463..5e54c29 100644 --- a/spec/unit/selinux.rb +++ b/spec/unit/selinux.rb @@ -1,9 +1,5 @@ -#!/usr/bin/env ruby - require File.dirname(__FILE__) + '/../spec_helper' -require 'facter' - describe "SELinux facts" do diff --git a/spec/unit/util/resolution.rb b/spec/unit/util/resolution.rb index 27cb150..ee835e3 100755 --- a/spec/unit/util/resolution.rb +++ b/spec/unit/util/resolution.rb @@ -233,27 +233,35 @@ describe Facter::Util::Resolution do # It's not possible, AFAICT, to mock %x{}, so I can't really test this bit. describe "when executing code" do - it "should fail if any interpreter other than /bin/sh is requested" do - lambda { Facter::Util::Resolution.exec("/something", "/bin/perl") }.should raise_error(ArgumentError) + + describe "using an interpreter other than /bin/sh" do + it "should fail" do + lambda { Facter::Util::Resolution.exec("/something", "/bin/perl") }.should raise_error(ArgumentError) + end end - it "should produce stderr content as a warning" do - stdout = stdin = stub('fh', :readlines => ["aaa\n"]) - stderr = stub('stderr', :readlines => %w{my content}) - Open3.expects(:popen3).with("/bin/true").yields(stdin, stdout, stderr) + describe "using /bin/sh as the interpreter" do - Facter.expects(:warn).with(['my','content']) - Facter::Util::Resolution.exec("/bin/true").should == 'aaa' - end + it "should produce stderr content as a warning" do + stdout = stdin = stub('fh', :readlines => ["aaa\n"]) + stderr = stub('stderr', :readlines => %w{my content}) + Open3.expects(:popen3).with("/bin/true").yields(stdin, stdout, stderr) + Facter.expects(:warn).with(['my','content']) + Facter::Util::Resolution.expects(:binary_exists?).with('/bin/true').returns(true) + Facter::Util::Resolution.exec("/bin/true").should == 'aaa' + end - it "should produce nil as a warning if nothing is printed to stderr" do - stdout = stdin = stub('fh', :readlines => ["aaa\n"]) - stderr = stub('stderr', :readlines => []) - Open3.expects(:popen3).with("/bin/true").yields(stdin, stdout, stderr) + it "should produce nil as a warning if nothing is printed to stderr" do + stdout = stdin = stub('fh', :readlines => ["aaa\n"]) + stderr = stub('stderr', :readlines => []) + Open3.expects(:popen3).with("/bin/true").yields(stdin, stdout, stderr) + Facter.expects(:warn).with(nil) + Facter::Util::Resolution.expects(:binary_exists?).with('/bin/true').returns(true) + Facter::Util::Resolution.exec("/bin/true").should == 'aaa' + end - Facter.expects(:warn).with(nil) - Facter::Util::Resolution.exec("/bin/true").should == 'aaa' end + end describe "when parsing output" do diff --git a/spec/unit/virtual.rb b/spec/unit/virtual.rb index cc72ffc..b11927c 100644 --- a/spec/unit/virtual.rb +++ b/spec/unit/virtual.rb @@ -1,8 +1,5 @@ require File.dirname(__FILE__) + '/../spec_helper' -require 'facter' -require 'facter/util/virtual' - describe "Virtual fact" do after do -- 1.6.0.4
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
