On Sat, Jan 16, 2010 at 11:47 AM, <[email protected]> wrote: > 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. >
We can't just use /usr/bin/true instead? > * 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. > Do we have examples of system_profiler -xml output changing output format? Apple aren't meant to do that, they supposedly guarantee the xml/plist output formats do not change. > > 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") > We really shouldn't have to be doing this sort of join with xml output. Is there an example of where it was blowing up? > 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]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > > > -- nigel--
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.
