diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index fe763710e..80ce96339 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -1,461 +1,451 @@
 # A module to collect utility functions.
 
 require 'English'
 require 'puppet/util/monkey_patches'
 require 'sync'
+require 'tempfile'
 require 'puppet/external/lock'
 require 'monitor'
 require 'puppet/util/execution_stub'
 
 module Puppet
   # A command failed to execute.
   require 'puppet/error'
   class ExecutionFailure < Puppet::Error
   end
 module Util
   require 'benchmark'
 
   # These are all for backward compatibility -- these are methods that used
   # to be in Puppet::Util but have been moved into external modules.
   require 'puppet/util/posix'
   extend Puppet::Util::POSIX
 
   @@sync_objects = {}.extend MonitorMixin
 
   def self.activerecord_version
     if (defined?(::ActiveRecord) and defined?(::ActiveRecord::VERSION) and defined?(::ActiveRecord::VERSION::MAJOR) and defined?(::ActiveRecord::VERSION::MINOR))
       ([::ActiveRecord::VERSION::MAJOR, ::ActiveRecord::VERSION::MINOR].join('.').to_f)
     else
       0
     end
   end
 
   def self.synchronize_on(x,type)
     sync_object,users = 0,1
     begin
       @@sync_objects.synchronize {
         (@@sync_objects[x] ||= [Sync.new,0])[users] += 1
       }
       @@sync_objects[x][sync_object].synchronize(type) { yield }
     ensure
       @@sync_objects.synchronize {
         @@sync_objects.delete(x) unless (@@sync_objects[x][users] -= 1) > 0
       }
     end
   end
 
   # Change the process to a different user
   def self.chuser
     if group = Puppet[:group]
       begin
         Puppet::Util::SUIDManager.change_group(group, true)
       rescue => detail
         Puppet.warning "could not change to group #{group.inspect}: #{detail}"
         $stderr.puts "could not change to group #{group.inspect}"
 
         # Don't exit on failed group changes, since it's
         # not fatal
         #exit(74)
       end
     end
 
     if user = Puppet[:user]
       begin
         Puppet::Util::SUIDManager.change_user(user, true)
       rescue => detail
         $stderr.puts "Could not change to user #{user}: #{detail}"
         exit(74)
       end
     end
   end
 
   # Create instance methods for each of the log levels.  This allows
   # the messages to be a little richer.  Most classes will be calling this
   # method.
   def self.logmethods(klass, useself = true)
     Puppet::Util::Log.eachlevel { |level|
       klass.send(:define_method, level, proc { |args|
         args = args.join(" ") if args.is_a?(Array)
         if useself
 
           Puppet::Util::Log.create(
             :level => level,
             :source => self,
             :message => args
           )
         else
 
           Puppet::Util::Log.create(
             :level => level,
             :message => args
           )
         end
       })
     }
   end
 
   # Proxy a bunch of methods to another object.
   def self.classproxy(klass, objmethod, *methods)
     classobj = class << klass; self; end
     methods.each do |method|
       classobj.send(:define_method, method) do |*args|
         obj = self.send(objmethod)
 
         obj.send(method, *args)
       end
     end
   end
 
   # Proxy a bunch of methods to another object.
   def self.proxy(klass, objmethod, *methods)
     methods.each do |method|
       klass.send(:define_method, method) do |*args|
         obj = self.send(objmethod)
 
         obj.send(method, *args)
       end
     end
   end
 
   # XXX this should all be done using puppet objects, not using
   # normal mkdir
   def self.recmkdir(dir,mode = 0755)
     if FileTest.exist?(dir)
       return false
     else
       tmp = dir.sub(/^\//,'')
       path = [File::SEPARATOR]
       tmp.split(File::SEPARATOR).each { |dir|
         path.push dir
         if ! FileTest.exist?(File.join(path))
           Dir.mkdir(File.join(path), mode)
         elsif FileTest.directory?(File.join(path))
           next
         else FileTest.exist?(File.join(path))
           raise "Cannot create #{dir}: basedir #{File.join(path)} is a file"
         end
       }
       return true
     end
   end
 
   # Execute a given chunk of code with a new umask.
   def self.withumask(mask)
     cur = File.umask(mask)
 
     begin
       yield
     ensure
       File.umask(cur)
     end
   end
 
   def benchmark(*args)
     msg = args.pop
     level = args.pop
     object = nil
 
     if args.empty?
       if respond_to?(level)
         object = self
       else
         object = Puppet
       end
     else
       object = args.pop
     end
 
     raise Puppet::DevError, "Failed to provide level to :benchmark" unless level
 
     unless level == :none or object.respond_to? level
       raise Puppet::DevError, "Benchmarked object does not respond to #{level}"
     end
 
     # Only benchmark if our log level is high enough
     if level != :none and Puppet::Util::Log.sendlevel?(level)
       result = nil
       seconds = Benchmark.realtime {
         yield
       }
       object.send(level, msg + (" in %0.2f seconds" % seconds))
       return seconds
     else
       yield
     end
   end
 
   def which(bin)
     if absolute_path?(bin)
       return bin if FileTest.file? bin and FileTest.executable? bin
     else
       ENV['PATH'].split(File::PATH_SEPARATOR).each do |dir|
         dest=File.join(dir, bin)
         return dest if FileTest.file? dest and FileTest.executable? dest
       end
     end
     nil
   end
   module_function :which
 
   # Determine in a platform-specific way whether a path is absolute. This
   # defaults to the local platform if none is specified.
   def absolute_path?(path, platform=nil)
     # Escape once for the string literal, and once for the regex.
     slash = '[\\\\/]'
     name = '[^\\\\/]+'
     regexes = {
       :windows => %r!^([A-Z]:#{slash})|(#{slash}#{slash}#{name}#{slash}#{name})|(#{slash}#{slash}\?#{slash}#{name})!i,
       :posix   => %r!^/!,
     }
     platform ||= Puppet.features.microsoft_windows? ? :windows : :posix
 
     !! (path =~ regexes[platform])
   end
   module_function :absolute_path?
 
   # Execute the provided command in a pipe, yielding the pipe object.
   def execpipe(command, failonfail = true)
     if respond_to? :debug
       debug "Executing '#{command}'"
     else
       Puppet.debug "Executing '#{command}'"
     end
 
     output = open("| #{command} 2>&1") do |pipe|
       yield pipe
     end
 
     if failonfail
       unless $CHILD_STATUS == 0
         raise ExecutionFailure, output
       end
     end
 
     output
   end
 
   def execfail(command, exception)
       output = execute(command)
       return output
   rescue ExecutionFailure
       raise exception, output
   end
 
+  def execute_posix(command, arguments, stdin, stdout, stderr)
+    child_pid = Kernel.fork do
+      command = Array(command)
+      Process.setsid
+      begin
+        $stdin.reopen(stdin)
+        $stdout.reopen(stdout)
+        $stderr.reopen(stderr)
+
+        3.upto(256){|fd| IO::new(fd).close rescue nil}
+
+        Puppet::Util::SUIDManager.change_group(arguments[:gid], true) if arguments[:gid]
+        Puppet::Util::SUIDManager.change_user(arguments[:uid], true)  if arguments[:uid]
+
+        ENV['LANG'] = ENV['LC_ALL'] = ENV['LC_MESSAGES'] = ENV['LANGUAGE'] = 'C'
+        Kernel.exec(*command)
+      rescue => detail
+        puts detail.to_s
+        exit!(1)
+      end
+    end
+    child_pid
+  end
+  module_function :execute_posix
+
+  def execute_windows(command, arguments, stdin, stdout, stderr)
+    command = command.map do |part|
+      part.include?(' ') ? %Q["#{part.gsub(/"/, '\"')}"] : part
+    end.join(" ") if command.is_a?(Array)
+
+    process_info = Process.create( :command_line => command, :startup_info => {:stdin => stdin, :stdout => stdout, :stderr => stderr} )
+    process_info.process_id
+  end
+  module_function :execute_windows
+
   # Execute the desired command, and return the status and output.
   # def execute(command, failonfail = true, uid = nil, gid = nil)
   # :combine sets whether or not to combine stdout/stderr in the output
   # :stdinfile sets a file that can be used for stdin. Passing a string
   # for stdin is not currently supported.
   def execute(command, arguments = {:failonfail => true, :combine => true})
     if command.is_a?(Array)
-      command = command.flatten.collect { |i| i.to_s }
+      command = command.flatten.map(&:to_s)
       str = command.join(" ")
-    else
-      # We require an array here so we know where we're incorrectly
-      # using a string instead of an array.  Once everything is
-      # switched to an array, we might relax this requirement.
-      raise ArgumentError, "Must pass an array to execute()"
+    elsif command.is_a?(String)
+      str = command
     end
 
     if respond_to? :debug
       debug "Executing '#{str}'"
     else
       Puppet.debug "Executing '#{str}'"
     end
 
-    if execution_stub = Puppet::Util::ExecutionStub.current_value
-      return execution_stub.call(command, arguments)
-    end
+    null_file = Puppet.features.microsoft_windows? ? 'NUL' : '/dev/null'
 
-    @@os ||= Facter.value(:operatingsystem)
-    output = nil
-    child_pid, child_status = nil
-    # There are problems with read blocking with badly behaved children
-    # read.partialread doesn't seem to capture either stdout or stderr
-    # We hack around this using a temporary file
-
-    # The idea here is to avoid IO#read whenever possible.
-    output_file="/dev/null"
-    error_file="/dev/null"
-    if ! arguments[:squelch]
-      require "tempfile"
-      output_file = Tempfile.new("puppet")
-      error_file=output_file if arguments[:combine]
-    end
+    stdin = File.open(arguments[:stdinfile] || null_file, 'r')
+    stdout = arguments[:squelch] ? File.open(null_file, 'w') : Tempfile.new('puppet')
+    stderr = arguments[:combine] ? stdout : File.open(null_file, 'w')
 
-    if Puppet.features.posix?
-      oldverb = $VERBOSE
-      $VERBOSE = nil
-      child_pid = Kernel.fork
-      $VERBOSE = oldverb
-      if child_pid
-        # Parent process executes this
-        child_status = (Process.waitpid2(child_pid)[1]).to_i >> 8
-      else
-        # Child process executes this
-        Process.setsid
-        begin
-          if arguments[:stdinfile]
-            $stdin.reopen(arguments[:stdinfile])
-          else
-            $stdin.reopen("/dev/null")
-          end
-          $stdout.reopen(output_file)
-          $stderr.reopen(error_file)
-
-          3.upto(256){|fd| IO::new(fd).close rescue nil}
-          Puppet::Util::SUIDManager.change_group(arguments[:gid], true) if arguments[:gid]
-          Puppet::Util::SUIDManager.change_user(arguments[:uid], true) if arguments[:uid]
-          ENV['LANG'] = ENV['LC_ALL'] = ENV['LC_MESSAGES'] = ENV['LANGUAGE'] = 'C'
-          if command.is_a?(Array)
-            Kernel.exec(*command)
-          else
-            Kernel.exec(command)
-          end
-        rescue => detail
-          puts detail.to_s
-          exit!(1)
-        end
-      end
+
+    exec_args = [command, arguments, stdin, stdout, stderr]
+
+    if execution_stub = Puppet::Util::ExecutionStub.current_value
+      return execution_stub.call(*exec_args)
+    elsif Puppet.features.posix?
+      child_pid = execute_posix(*exec_args)
     elsif Puppet.features.microsoft_windows?
-      command = command.collect {|part| '"' + part.gsub(/"/, '\\"') + '"'}.join(" ") if command.is_a?(Array)
-      Puppet.debug "Creating process '#{command}'"
-      processinfo = Process.create( :command_line => command )
-      child_status = (Process.waitpid2(child_pid)[1]).to_i >> 8
+      child_pid = execute_windows(*exec_args)
     end
 
-    # read output in if required
-    if ! arguments[:squelch]
-
-      # Make sure the file's actually there.  This is
-      # basically a race condition, and is probably a horrible
-      # way to handle it, but, well, oh well.
-      unless FileTest.exists?(output_file.path)
-        Puppet.warning "sleeping"
-        sleep 0.5
-        unless FileTest.exists?(output_file.path)
-          Puppet.warning "sleeping 2"
-          sleep 1
-          unless FileTest.exists?(output_file.path)
-            Puppet.warning "Could not get output"
-            output = ""
-          end
-        end
-      end
-      unless output
-        # We have to explicitly open here, so that it reopens
-        # after the child writes.
-        output = output_file.open.read
+    child_status = Process.waitpid2(child_pid).last
 
-        # The 'true' causes the file to get unlinked right away.
-        output_file.close(true)
-      end
+    [stdin, stdout, stderr].each {|io| io.close rescue nil}
+
+    # read output in if required
+    unless arguments[:squelch]
+      output = wait_for_output(stdout)
+      Puppet.warning "Could not get output" unless output
     end
 
-    if arguments[:failonfail]
-      unless child_status == 0
-        raise ExecutionFailure, "Execution of '#{str}' returned #{child_status}: #{output}"
-      end
+    if arguments[:failonfail] and child_status != 0
+      raise ExecutionFailure, "Execution of '#{str}' returned #{child_status.exitstatus}: #{output}"
     end
 
     output
   end
 
   module_function :execute
 
+  def wait_for_output(stdout)
+    # Make sure the file's actually been written.  This is basically a race
+    # condition, and is probably a horrible way to handle it, but, well, oh
+    # well.
+    2.times do |try|
+      if File.exists?(stdout.path)
+        output = stdout.open.read
+
+        stdout.close(true)
+
+        return output
+      else
+        time_to_sleep = try / 2.0
+        Puppet.warning "Waiting for output; will sleep #{time_to_sleep} seconds"
+        sleep(time_to_sleep)
+      end
+    end
+    nil
+  end
+  module_function :wait_for_output
+
   # Create an exclusive lock.
   def threadlock(resource, type = Sync::EX)
     Puppet::Util.synchronize_on(resource,type) { yield }
   end
 
   # Because some modules provide their own version of this method.
   alias util_execute execute
 
   module_function :benchmark
 
   def memory
     unless defined?(@pmap)
       @pmap = which('pmap')
     end
     if @pmap
       %x{#{@pmap} #{Process.pid}| grep total}.chomp.sub(/^\s*total\s+/, '').sub(/K$/, '').to_i
     else
       0
     end
   end
 
   def symbolize(value)
     if value.respond_to? :intern
       value.intern
     else
       value
     end
   end
 
   def symbolizehash(hash)
     newhash = {}
     hash.each do |name, val|
       if name.is_a? String
         newhash[name.intern] = val
       else
         newhash[name] = val
       end
     end
   end
 
   def symbolizehash!(hash)
     hash.each do |name, val|
       if name.is_a? String
         hash[name.intern] = val
         hash.delete(name)
       end
     end
 
     hash
   end
   module_function :symbolize, :symbolizehash, :symbolizehash!
 
   # Just benchmark, with no logging.
   def thinmark
     seconds = Benchmark.realtime {
       yield
     }
 
     seconds
   end
 
   module_function :memory, :thinmark
 
   def secure_open(file,must_be_w,&block)
     raise Puppet::DevError,"secure_open only works with mode 'w'" unless must_be_w == 'w'
     raise Puppet::DevError,"secure_open only requires a block"    unless block_given?
     Puppet.warning "#{file} was a symlink to #{File.readlink(file)}" if File.symlink?(file)
     if File.exists?(file) or File.symlink?(file)
       wait = File.symlink?(file) ? 5.0 : 0.1
       File.delete(file)
       sleep wait # give it a chance to reappear, just in case someone is actively trying something.
     end
     begin
       File.open(file,File::CREAT|File::EXCL|File::TRUNC|File::WRONLY,&block)
     rescue Errno::EEXIST
       desc = File.symlink?(file) ? "symlink to #{File.readlink(file)}" : File.stat(file).ftype
       puts "Warning: #{file} was apparently created by another process (as"
       puts "a #{desc}) as soon as it was deleted by this process.  Someone may be trying"
       puts "to do something objectionable (such as tricking you into overwriting system"
       puts "files if you are running as root)."
       raise
     end
   end
   module_function :secure_open
 end
 end
 
 require 'puppet/util/errors'
 require 'puppet/util/methodhelper'
 require 'puppet/util/metaid'
 require 'puppet/util/classgen'
 require 'puppet/util/docs'
 require 'puppet/util/execution'
 require 'puppet/util/logging'
 require 'puppet/util/package'
 require 'puppet/util/warnings'
diff --git a/spec/integration/util_spec.rb b/spec/integration/util_spec.rb
new file mode 100644
index 000000000..a50f78326
--- /dev/null
+++ b/spec/integration/util_spec.rb
@@ -0,0 +1,13 @@
+#!/usr/bin/env ruby
+
+require 'spec_helper'
+
+describe Puppet::Util do
+  describe "#execute" do
+    it "should properly allow stdout and stderr to share a file" do
+      command = "ruby -e '(1..10).each {|i| (i%2==0) ? $stdout.puts(i) : $stderr.puts(i)}'"
+
+      Puppet::Util.execute(command, :combine => true).split.should =~ [*'1'..'10']
+    end
+  end
+end
diff --git a/spec/unit/util_spec.rb b/spec/unit/util_spec.rb
index e06583215..4481f3863 100644
--- a/spec/unit/util_spec.rb
+++ b/spec/unit/util_spec.rb
@@ -1,49 +1,335 @@
 #!/usr/bin/env ruby
 
 require 'spec_helper'
 
 describe Puppet::Util do
   describe "#absolute_path?" do
     it "should default to the platform of the local system" do
       Puppet.features.stubs(:posix?).returns(true)
       Puppet.features.stubs(:microsoft_windows?).returns(false)
 
       Puppet::Util.should be_absolute_path('/foo')
       Puppet::Util.should_not be_absolute_path('C:/foo')
 
       Puppet.features.stubs(:posix?).returns(false)
       Puppet.features.stubs(:microsoft_windows?).returns(true)
 
       Puppet::Util.should be_absolute_path('C:/foo')
       Puppet::Util.should_not be_absolute_path('/foo')
     end
 
     describe "when using platform :posix" do
       %w[/ /foo /foo/../bar //foo //Server/Foo/Bar //?/C:/foo/bar /\Server/Foo].each do |path|
         it "should return true for #{path}" do
           Puppet::Util.should be_absolute_path(path, :posix)
         end
       end
 
       %w[. ./foo \foo C:/foo \\Server\Foo\Bar \\?\C:\foo\bar \/?/foo\bar \/Server/foo].each do |path|
         it "should return false for #{path}" do
           Puppet::Util.should_not be_absolute_path(path, :posix)
         end
       end
     end
 
     describe "when using platform :windows" do
       %w[C:/foo C:\foo \\\\Server\Foo\Bar \\\\?\C:\foo\bar //Server/Foo/Bar //?/C:/foo/bar /\?\C:/foo\bar \/Server\Foo/Bar].each do |path|
         it "should return true for #{path}" do
           Puppet::Util.should be_absolute_path(path, :windows)
         end
       end
 
       %w[/ . ./foo \foo /foo /foo/../bar //foo C:foo/bar].each do |path|
         it "should return false for #{path}" do
           Puppet::Util.should_not be_absolute_path(path, :windows)
         end
       end
     end
   end
+
+  describe "execution methods" do
+    let(:pid) { 5501 }
+    let(:null_file) { Puppet.features.microsoft_windows? ? 'NUL' : '/dev/null' }
+
+    describe "#execute_posix" do
+      before :each do
+        # Most of the things this method does are bad to do during specs. :/
+        Kernel.stubs(:fork).returns(pid).yields
+        Process.stubs(:setsid)
+        Kernel.stubs(:exec)
+        Puppet::Util::SUIDManager.stubs(:change_user)
+        Puppet::Util::SUIDManager.stubs(:change_group)
+
+        $stdin.stubs(:reopen)
+        $stdout.stubs(:reopen)
+        $stderr.stubs(:reopen)
+
+        @stdin  = File.open(null_file, 'r')
+        @stdout = Tempfile.new('stdout')
+        @stderr = File.open(null_file, 'w')
+      end
+
+      it "should fork a child process to execute the command" do
+        Kernel.expects(:fork).returns(pid).yields
+        Kernel.expects(:exec).with('test command')
+
+        Puppet::Util.execute_posix('test command', {}, @stdin, @stdout, @stderr)
+      end
+
+      it "should start a new session group" do
+        Process.expects(:setsid)
+
+        Puppet::Util.execute_posix('test command', {}, @stdin, @stdout, @stderr)
+      end
+
+      it "should close all open file descriptors except stdin/stdout/stderr" do
+        # This is ugly, but I can't really think of a better way to do it without
+        # letting it actually close fds, which seems risky
+        (0..2).each {|n| IO.expects(:new).with(n).never}
+        (3..256).each {|n| IO.expects(:new).with(n).returns mock('io', :close) }
+
+        Puppet::Util.execute_posix('test command', {}, @stdin, @stdout, @stderr)
+      end
+
+      it "should permanently change to the correct user and group if specified" do
+        Puppet::Util::SUIDManager.expects(:change_group).with(55, true)
+        Puppet::Util::SUIDManager.expects(:change_user).with(50, true)
+
+        Puppet::Util.execute_posix('test command', {:uid => 50, :gid => 55}, @stdin, @stdout, @stderr)
+      end
+
+      it "should exit failure if there is a problem execing the command" do
+        Kernel.expects(:exec).with('test command').raises("failed to execute!")
+        Puppet::Util.stubs(:puts)
+        Puppet::Util.expects(:exit!).with(1)
+
+        Puppet::Util.execute_posix('test command', {}, @stdin, @stdout, @stderr)
+      end
+
+      it "should properly execute commands specified as arrays" do
+        Kernel.expects(:exec).with('test command', 'with', 'arguments')
+
+        Puppet::Util.execute_posix(['test command', 'with', 'arguments'], {:uid => 50, :gid => 55}, @stdin, @stdout, @stderr)
+      end
+
+      it "should return the pid of the child process" do
+        Puppet::Util.execute_posix('test command', {}, @stdin, @stdout, @stderr).should == pid
+      end
+    end
+
+    describe "#execute_windows" do
+      let(:proc_info_stub) { stub 'processinfo', :process_id => pid }
+
+      before :each do
+        Process.stubs(:create).returns(proc_info_stub)
+        Process.stubs(:waitpid2).with(pid).returns([pid, 0])
+
+        @stdin  = File.open(null_file, 'r')
+        @stdout = Tempfile.new('stdout')
+        @stderr = File.open(null_file, 'w')
+      end
+
+      it "should create a new process for the command" do
+        Process.expects(:create).with(
+          :command_line => "test command",
+          :startup_info => {:stdin => @stdin, :stdout => @stdout, :stderr => @stderr}
+        ).returns(proc_info_stub)
+
+        Puppet::Util.execute_windows('test command', {}, @stdin, @stdout, @stderr)
+      end
+
+      it "should return the pid of the child process" do
+        Puppet::Util.execute_windows('test command', {}, @stdin, @stdout, @stderr).should == pid
+      end
+
+      it "should quote arguments containing spaces if command is specified as an array" do
+        Process.expects(:create).with do |args|
+          args[:command_line] == '"test command" with some "arguments \"with spaces"'
+        end.returns(proc_info_stub)
+
+        Puppet::Util.execute_windows(['test command', 'with', 'some', 'arguments "with spaces'], {}, @stdin, @stdout, @stderr)
+      end
+    end
+
+    describe "#execute" do
+      before :each do
+        Process.stubs(:waitpid2).with(pid).returns([pid, 0])
+      end
+
+      describe "when an execution stub is specified" do
+        before :each do
+          Puppet::Util::ExecutionStub.set do |command,args,stdin,stdout,stderr|
+            "execution stub output"
+          end
+        end
+
+        it "should call the block on the stub" do
+          Puppet::Util.execute("/usr/bin/run_my_execute_stub").should == "execution stub output"
+        end
+
+        it "should not actually execute anything" do
+          Puppet::Util.expects(:execute_posix).never
+          Puppet::Util.expects(:execute_windows).never
+
+          Puppet::Util.execute("/usr/bin/run_my_execute_stub")
+        end
+      end
+
+      describe "when setting up input and output files" do
+        include PuppetSpec::Files
+        let(:executor) { Puppet.features.microsoft_windows? ? 'execute_windows' : 'execute_posix' }
+
+        before :each do
+          Puppet::Util.stubs(:wait_for_output)
+        end
+
+        it "should set stdin to the stdinfile if specified" do
+          input = tmpfile('stdin')
+          FileUtils.touch(input)
+
+          Puppet::Util.expects(executor).with do |_,_,stdin,_,_|
+            stdin.path == input
+          end.returns(pid)
+
+          Puppet::Util.execute('test command', :stdinfile => input)
+        end
+
+        it "should set stdin to the null file if not specified" do
+          Puppet::Util.expects(executor).with do |_,_,stdin,_,_|
+            stdin.path == null_file
+          end.returns(pid)
+
+          Puppet::Util.execute('test command')
+        end
+
+        describe "when squelch is set" do
+          it "should set stdout and stderr to the null file" do
+            Puppet::Util.expects(executor).with do |_,_,_,stdout,stderr|
+              stdout.path == null_file and stderr.path == null_file
+            end.returns(pid)
+
+            Puppet::Util.execute('test command', :squelch => true)
+          end
+        end
+
+        describe "when squelch is not set" do
+          it "should set stdout to a temporary output file" do
+            outfile = Tempfile.new('stdout')
+            Tempfile.stubs(:new).returns(outfile)
+
+            Puppet::Util.expects(executor).with do |_,_,_,stdout,_|
+              stdout.path == outfile.path
+            end.returns(pid)
+
+            Puppet::Util.execute('test command', :squelch => false)
+          end
+
+          it "should set stderr to the same file as stdout if combine is true" do
+            outfile = Tempfile.new('stdout')
+            Tempfile.stubs(:new).returns(outfile)
+
+            Puppet::Util.expects(executor).with do |_,_,_,stdout,stderr|
+              stdout.path == outfile.path and stderr.path == outfile.path
+            end.returns(pid)
+
+            Puppet::Util.execute('test command', :squelch => false, :combine => true)
+          end
+
+          it "should set stderr to the null device if combine is false" do
+            outfile = Tempfile.new('stdout')
+            Tempfile.stubs(:new).returns(outfile)
+
+            Puppet::Util.expects(executor).with do |_,_,_,stdout,stderr|
+              stdout.path == outfile.path and stderr.path == null_file
+            end.returns(pid)
+
+            Puppet::Util.execute('test command', :squelch => false, :combine => false)
+          end
+        end
+      end
+    end
+
+    describe "after execution" do
+      let(:executor) { Puppet.features.microsoft_windows? ? 'execute_windows' : 'execute_posix' }
+      before :each do
+        Process.stubs(:waitpid2).with(pid).returns([pid, 0])
+
+        Puppet::Util.stubs(executor).returns(pid)
+      end
+
+      it "should wait for the child process to exit" do
+        Puppet::Util.stubs(:wait_for_output)
+
+        Process.expects(:waitpid2).with(pid).returns([pid, 0])
+
+        Puppet::Util.execute('test command')
+      end
+
+      it "should close the stdin/stdout/stderr files used by the child" do
+        stdin = mock 'file', :close
+        stdout = mock 'file', :close
+        stderr = mock 'file', :close
+
+        File.expects(:open).
+          times(3).
+          returns(stdin).
+          then.returns(stdout).
+          then.returns(stderr)
+
+        Puppet::Util.execute('test command', :squelch => true)
+      end
+
+      it "should read and return the output if squelch is false" do
+        stdout = Tempfile.new('test')
+        Tempfile.stubs(:new).returns(stdout)
+        stdout.write("My expected command output")
+
+        Puppet::Util.execute('test command').should == "My expected command output"
+      end
+
+      it "should not read the output if squelch is true" do
+        stdout = Tempfile.new('test')
+        Tempfile.stubs(:new).returns(stdout)
+        stdout.write("My expected command output")
+
+        Puppet::Util.execute('test command', :squelch => true).should == nil
+      end
+
+      it "should delete the file used for output if squelch is false" do
+        stdout = Tempfile.new('test')
+        Tempfile.stubs(:new).returns(stdout)
+
+        Puppet::Util.execute('test command')
+
+        # Tempfile#path returns nil if the file has been unlinked
+        stdout.path.should == nil
+      end
+
+      it "should raise an error if failonfail is true and the child failed" do
+        child_status = stub('child_status', :exitstatus => 1)
+
+        Process.expects(:waitpid2).with(pid).returns([pid, child_status])
+
+        expect {
+          Puppet::Util.execute('fail command', :failonfail => true)
+        }.to raise_error(Puppet::ExecutionFailure, /Execution of 'fail command' returned 1/)
+      end
+
+      it "should not raise an error if failonfail is false and the child failed" do
+        Process.expects(:waitpid2).with(pid).returns([pid, 1])
+
+        expect {
+          Puppet::Util.execute('fail command', :failonfail => false)
+        }.not_to raise_error
+      end
+
+      it "should not raise an error if failonfail is true and the child succeeded" do
+        Process.expects(:waitpid2).with(pid).returns([pid, 0])
+
+        expect {
+          Puppet::Util.execute('fail command', :failonfail => true)
+        }.not_to raise_error
+      end
+    end
+  end
 end
diff --git a/test/util/utiltest.rb b/test/util/utiltest.rb
index 1c934d612..9bc243c1c 100755
--- a/test/util/utiltest.rb
+++ b/test/util/utiltest.rb
@@ -1,236 +1,223 @@
 #!/usr/bin/env ruby
 
 require File.expand_path(File.dirname(__FILE__) + '/../lib/puppettest')
 
 require 'puppettest'
 require 'mocha'
 
 class TestPuppetUtil < Test::Unit::TestCase
   include PuppetTest
 
   def test_withumask
     oldmask = File.umask
 
     path = tempfile
 
     # FIXME this fails on FreeBSD with a mode of 01777
     Puppet::Util.withumask(000) do
       Dir.mkdir(path, 0777)
     end
 
     assert(File.stat(path).mode & 007777 == 0777, "File has the incorrect mode")
     assert_equal(oldmask, File.umask, "Umask was not reset")
   end
 
   def test_benchmark
     path = tempfile
     str = "yayness"
     File.open(path, "w") do |f| f.print "yayness" end
 
     # First test it with the normal args
     assert_nothing_raised do
       val = nil
       result = Puppet::Util.benchmark(:notice, "Read file") do
         val = File.read(path)
       end
 
       assert_equal(str, val)
 
       assert_instance_of(Float, result)
 
     end
 
     # Now test it with a passed object
     assert_nothing_raised do
       val = nil
       Puppet::Util.benchmark(Puppet, :notice, "Read file") do
         val = File.read(path)
       end
 
       assert_equal(str, val)
     end
   end
 
   def test_proxy
     klass = Class.new do
       attr_accessor :hash
       class << self
         attr_accessor :ohash
       end
     end
     klass.send(:include, Puppet::Util)
 
     klass.ohash = {}
 
     inst = klass.new
     inst.hash = {}
     assert_nothing_raised do
       Puppet::Util.proxy klass, :hash, "[]", "[]=", :clear, :delete
     end
 
     assert_nothing_raised do
       Puppet::Util.classproxy klass, :ohash, "[]", "[]=", :clear, :delete
     end
 
     assert_nothing_raised do
       inst[:yay] = "boo"
       inst["cool"] = :yayness
     end
 
     [:yay, "cool"].each do |var|
       assert_equal(inst.hash[var], inst[var], "Var #{var} did not take")
     end
 
     assert_nothing_raised do
       klass[:Yay] = "boo"
       klass["Cool"] = :yayness
     end
 
     [:Yay, "Cool"].each do |var|
       assert_equal(inst.hash[var], inst[var], "Var #{var} did not take")
     end
   end
 
   def test_symbolize
     ret = nil
     assert_nothing_raised {
       ret = Puppet::Util.symbolize("yayness")
     }
 
     assert_equal(:yayness, ret)
 
     assert_nothing_raised {
       ret = Puppet::Util.symbolize(:yayness)
     }
 
     assert_equal(:yayness, ret)
 
     assert_nothing_raised {
       ret = Puppet::Util.symbolize(43)
     }
 
     assert_equal(43, ret)
 
     assert_nothing_raised {
       ret = Puppet::Util.symbolize(nil)
     }
 
     assert_equal(nil, ret)
   end
 
   def test_execute
     command = tempfile
     File.open(command, "w") { |f|
       f.puts %{#!/bin/sh\n/bin/echo "$1">&1; echo "$2">&2}
     }
     File.chmod(0755, command)
     output = nil
     assert_nothing_raised do
       output = Puppet::Util.execute([command, "yaytest", "funtest"])
     end
     assert_equal("yaytest\nfuntest\n", output)
 
     # Now try it with a single quote
     assert_nothing_raised do
       output = Puppet::Util.execute([command, "yay'test", "funtest"])
     end
     assert_equal("yay'test\nfuntest\n", output)
 
     # Now make sure we can squelch output (#565)
     assert_nothing_raised do
       output = Puppet::Util.execute([command, "yay'test", "funtest"], :squelch => true)
     end
     assert_equal(nil, output)
 
     # Now test that we correctly fail if the command returns non-zero
     assert_raise(Puppet::ExecutionFailure) do
       out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"])
     end
 
     # And that we can tell it not to fail
     assert_nothing_raised do
       out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"], :failonfail => false)
     end
 
     if Process.uid == 0
       # Make sure we correctly set our uid and gid
       user = nonrootuser
       group = nonrootgroup
       file = tempfile
       assert_nothing_raised do
         Puppet::Util.execute(["touch", file], :uid => user.name, :gid => group.name)
       end
       assert(FileTest.exists?(file), "file was not created")
       assert_equal(user.uid, File.stat(file).uid, "uid was not set correctly")
 
       # We can't really check the gid, because it just behaves too
       # inconsistently everywhere.
       # assert_equal(group.gid, File.stat(file).gid,
       #    "gid was not set correctly")
     end
 
     # (#565) Test the case of patricide.
     patricidecommand = tempfile
     File.open(patricidecommand, "w") { |f|
       f.puts %{#!/bin/bash\n/bin/bash -c 'kill -TERM \$PPID' &;\n while [ 1 ]; do echo -n ''; done;\n}
     }
     File.chmod(0755, patricidecommand)
     assert_nothing_raised do
       output = Puppet::Util.execute([patricidecommand], :squelch => true)
     end
     assert_equal(nil, output)
     # See what happens if we try and read the pipe to the command...
     assert_raise(Puppet::ExecutionFailure) do
       output = Puppet::Util.execute([patricidecommand])
     end
     assert_nothing_raised do
       output = Puppet::Util.execute([patricidecommand], :failonfail => false)
     end
   end
 
   def test_lang_environ_in_execute
     orig_lang = ENV["LANG"]
     orig_lc_all = ENV["LC_ALL"]
     orig_lc_messages = ENV["LC_MESSAGES"]
     orig_language = ENV["LANGUAGE"]
 
     cleanup do
       ENV["LANG"] = orig_lang
       ENV["LC_ALL"] = orig_lc_all
       ENV["LC_MESSAGES"] = orig_lc_messages
       ENV["LANGUAGE"] = orig_lc_messages
     end
 
     # Mmm, we love gettext(3)
     ENV["LANG"] = "en_US"
     ENV["LC_ALL"] = "en_US"
     ENV["LC_MESSAGES"] = "en_US"
     ENV["LANGUAGE"] = "en_US"
 
     %w{LANG LC_ALL LC_MESSAGES LANGUAGE}.each do |env|
 
       assert_equal(
         'C',
           Puppet::Util.execute(['ruby', '-e', "print ENV['#{env}']"]),
 
           "Environment var #{env} wasn't set to 'C'")
 
       assert_equal 'en_US', ENV[env], "Environment var #{env} not set back correctly"
     end
 
   end
-
-  # Check whether execute accepts strings in addition to arrays.
-  def test_string_exec
-    cmd = "/bin/echo howdy"
-    output = nil
-    assert_raise(ArgumentError) {
-      output = Puppet::Util.execute(cmd)
-    }
-    #assert_equal("howdy\n", output)
-    #assert_raise(RuntimeError) {
-    #    Puppet::Util.execute(cmd, 0, 0)
-    #}
-  end
 end