@@ -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,27 @@ 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 - end + Facter.warn(err) + + 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. lib/facter/util/resolution.rb @@ -227,10 +227,54 @@ 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 + + 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.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) + + Facter.expects(:warn).with(nil) + Facter::Util::Resolution.exec("/bin/true").should == 'aaa' + 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 spec/unit/util/resolution.rb 810980659d86a30cc9dde6018a4749f659fe2d00 Peter Meier peter.meier@immerda.ch http://github.com/jamtur01/facter/commit/fabe21e702e7d3878fc0775a7dcf24c44aaf4698 fabe21e702e7d3878fc0775a7dcf24c44aaf4698 2009-11-07T03:16:14-08:00 2009-11-07T03:10:56-08:00 use popen3 in Resolution.exec to catch stderr 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 passing them to the new introduced Facter.warn method. 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. c28a42981ccf4d7547079e7d3dae04e6104e7bee James Turnbull james@lovedthanlost.net