It'd be nice to have the tests demonstrate that something is done with stderr, but otherwise the patch looks good.
On Nov 4, 2009, at 3:44 PM, Peter Meier wrote: > > So far messages to stderr haven't been catched by > Facter::Util::Resolution.exec and were insted printed out to > stderr. This will cause facter and even puppet to print to stderr > themself, which is not very nice when running puppetd by cron, > as you might get every run a mail if a command outputs to stderr. > > We are now wrapping the command execution with Open3.popen3 to > catch stderr and logging it to Facter.debug > > We are also catching multiline outputs chomping newlines and > returning an array if there have been more than one line. Otherwise > we return an array containing the different lines. > > This prevents in general cases as described in #2766 and should > handle command execution in a bit saner way. > > Signed-off-by: Peter Meier <[email protected]> > --- > lib/facter/util/resolution.rb | 23 ++++++++++++++++++----- > spec/unit/util/resolution.rb | 26 ++++++++++++++++++++++++++ > 2 files changed, 44 insertions(+), 5 deletions(-) > > diff --git a/lib/facter/util/resolution.rb b/lib/facter/util/ > resolution.rb > index b9e28e8..b9d0cf8 100644 > --- a/lib/facter/util/resolution.rb > +++ b/lib/facter/util/resolution.rb > @@ -7,6 +7,7 @@ require 'facter/util/confine' > > require 'timeout' > require 'rbconfig' > +require 'open3' > > class Facter::Util::Resolution > attr_accessor :interpreter, :code, :name, :timeout > @@ -42,17 +43,29 @@ class Facter::Util::Resolution > end > > out = nil > + err = nil > begin > - out = %x{#{code}}.chomp > + 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 > - if out == "" > - return nil > - else > - return out > + if err and err != '' > + Facter.debug("#{code} printed \"#{err}\" to stderr") > end > + > + return nil if out == "" > + out > + end > + > + def self.parse_output(output) > + return nil unless output and output.size > 0 > + result = output.collect{|line| line.chomp } > + return result.first unless result.size > 1 > + result > end > > # Add a new confine to the resolution mechanism. > diff --git a/spec/unit/util/resolution.rb b/spec/unit/util/ > resolution.rb > index d4bb781..2ad7f93 100755 > --- a/spec/unit/util/resolution.rb > +++ b/spec/unit/util/resolution.rb > @@ -227,10 +227,36 @@ describe Facter::Util::Resolution do > Facter::Util::Resolution.should respond_to(:exec) > end > > + it "should have a class method to parse output" do > + Facter::Util::Resolution.should respond_to(:parse_output) > + end > + > # 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) > end > end > + > + describe "when parsing output" do > + it "should return nil on nil" do > + Facter::Util::Resolution.parse_output(nil).should be_nil > + end > + > + it "should return nil on empty string" do > + Facter::Util::Resolution.parse_output('').should be_nil > + end > + > + it "should return nil on an empty array" do > + Facter::Util::Resolution.parse_output([]).should be_nil > + end > + > + it "should return a string on a 1 size array" do > + Facter::Util::Resolution.parse_output(["aaa\n"]).should > == "aaa" > + end > + > + it "should return a string on a multi sized array > containing a seperator" do > + result = Facter::Util::Resolution.parse_output(["aaa > \n","bbb\n","ccc\n"]).should == [ "aaa", "bbb", "ccc" ] > + end > + end > end > -- > 1.6.3.3 > > > > -- I had a linguistics professor who said that it's man's ability to use language that makes him the dominant species on the planet. That may be. But I think there's one other thing that separates us from animals. We aren't afraid of vacuum cleaners. --Jeff Stilson --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
