diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index 4458bf081..be0ece023 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -1,516 +1,514 @@
 module Puppet
   newtype(:exec) do
     include Puppet::Util::Execution
     require 'timeout'
 
     @doc = "Executes external commands.  It is critical that all commands
       executed using this mechanism can be run multiple times without
       harm, i.e., they are *idempotent*.  One useful way to create idempotent
       commands is to use the checks like `creates` to avoid running the
       command unless some condition is met.
 
       Note that you can restrict an `exec` to only run when it receives
       events by using the `refreshonly` parameter; this is a useful way to
       have your configuration respond to events with arbitrary commands.
 
       Note also that if an `exec` receives an event from another resource,
       it will get executed again (or execute the command specified in `refresh`, if there is one).
 
       There is a strong tendency to use `exec` to do whatever work Puppet
       can't already do; while this is obviously acceptable (and unavoidable)
       in the short term, it is highly recommended to migrate work from `exec`
       to native Puppet types as quickly as possible.  If you find that
       you are doing a lot of work with `exec`, please at least notify
       us at Puppet Labs what you are doing, and hopefully we can work with
       you to get a native resource type for the work you are doing.
 
       **Autorequires:** If Puppet is managing an exec's cwd or the executable file used in an exec's command, the exec resource will autorequire those files. If Puppet is managing the user that an exec should run as, the exec resource will autorequire that user."
 
     # Create a new check mechanism.  It's basically just a parameter that
     # provides one extra 'check' method.
     def self.newcheck(name, options = {}, &block)
       @checks ||= {}
 
       check = newparam(name, options, &block)
       @checks[name] = check
     end
 
     def self.checks
       @checks.keys
     end
 
     newproperty(:returns, :array_matching => :all, :event => :executed_command) do |property|
       include Puppet::Util::Execution
       munge do |value|
         value.to_s
       end
 
       def event_name
         :executed_command
       end
 
       defaultto "0"
 
       attr_reader :output
       desc "The expected return code(s).  An error will be returned if the
         executed command returns something else.  Defaults to 0. Can be
         specified as an array of acceptable return codes or a single value."
 
       # Make output a bit prettier
       def change_to_s(currentvalue, newvalue)
         "executed successfully"
       end
 
       # First verify that all of our checks pass.
       def retrieve
         # We need to return :notrun to trigger evaluation; when that isn't
         # true, we *LIE* about what happened and return a "success" for the
         # value, which causes us to be treated as in_sync?, which means we
         # don't actually execute anything.  I think. --daniel 2011-03-10
         if @resource.check_all_attributes
           return :notrun
         else
           return self.should
         end
       end
 
       # Actually execute the command.
       def sync
         olddir = nil
 
         # We need a dir to change to, even if it's just the cwd
         dir = self.resource[:cwd] || Dir.pwd
 
         event = :executed_command
         tries = self.resource[:tries]
         try_sleep = self.resource[:try_sleep]
 
         begin
           tries.times do |try|
             # Only add debug messages for tries > 1 to reduce log spam.
             debug("Exec try #{try+1}/#{tries}") if tries > 1
             @output, @status = provider.run(self.resource[:command])
             break if self.should.include?(@status.exitstatus.to_s)
             if try_sleep > 0 and tries > 1
               debug("Sleeping for #{try_sleep} seconds between tries")
               sleep try_sleep
             end
           end
         rescue Timeout::Error
           self.fail "Command exceeded timeout" % value.inspect
         end
 
         if log = @resource[:logoutput]
           case log
           when :true
             log = @resource[:loglevel]
           when :on_failure
             unless self.should.include?(@status.exitstatus.to_s)
               log = @resource[:loglevel]
             else
               log = :false
             end
           end
           unless log == :false
             @output.split(/\n/).each { |line|
               self.send(log, line)
             }
           end
         end
 
         unless self.should.include?(@status.exitstatus.to_s)
           self.fail("#{self.resource[:command]} returned #{@status.exitstatus} instead of one of [#{self.should.join(",")}]")
         end
 
         event
       end
     end
 
     newparam(:command) do
       isnamevar
       desc "The actual command to execute.  Must either be fully qualified
         or a search path for the command must be provided.  If the command
         succeeds, any output produced will be logged at the instance's
         normal log level (usually `notice`), but if the command fails
         (meaning its return code does not match the specified code) then
         any output is logged at the `err` log level."
     end
 
     newparam(:path) do
       desc "The search path used for command execution.
         Commands must be fully qualified if no path is specified.  Paths
         can be specified as an array or as a colon separated list."
 
       # Support both arrays and colon-separated fields.
       def value=(*values)
         @value = values.flatten.collect { |val|
           if val =~ /;/ # recognize semi-colon separated paths
             val.split(";")
           elsif val =~ /^\w:[^:]*$/ # heuristic to avoid splitting a driveletter away
             val
           else
             val.split(":")
           end
         }.flatten
       end
     end
 
     newparam(:user) do
       desc "The user to run the command as.  Note that if you
         use this then any error output is not currently captured.  This
         is because of a bug within Ruby.  If you are using Puppet to
         create this user, the exec will automatically require the user,
         as long as it is specified by name."
 
       # Most validation is handled by the SUIDManager class.
       validate do |user|
         self.fail "Only root can execute commands as other users" unless Puppet.features.root?
       end
     end
 
     newparam(:group) do
       desc "The group to run the command as.  This seems to work quite
         haphazardly on different platforms -- it is a platform issue
         not a Ruby or Puppet one, since the same variety exists when
         running commnands as different users in the shell."
       # Validation is handled by the SUIDManager class.
     end
 
     newparam(:cwd, :parent => Puppet::Parameter::Path) do
       desc "The directory from which to run the command.  If
         this directory does not exist, the command will fail."
     end
 
     newparam(:logoutput) do
       desc "Whether to log output.  Defaults to logging output at the
         loglevel for the `exec` resource. Use *on_failure* to only
         log the output when the command reports an error.  Values are
         **true**, *false*, *on_failure*, and any legal log level."
 
       newvalues(:true, :false, :on_failure)
     end
 
     newparam(:refresh) do
       desc "How to refresh this command.  By default, the exec is just
         called again when it receives an event from another resource,
         but this parameter allows you to define a different command
         for refreshing."
 
       validate do |command|
         provider.validatecmd(command)
       end
     end
 
     newparam(:env) do
       desc "This parameter is deprecated. Use 'environment' instead."
 
       munge do |value|
         warning "'env' is deprecated on exec; use 'environment' instead."
         resource[:environment] = value
       end
     end
 
     newparam(:environment) do
       desc "Any additional environment variables you want to set for a
         command.  Note that if you use this to set PATH, it will override
         the `path` attribute.  Multiple environment variables should be
         specified as an array."
 
       validate do |values|
         values = [values] unless values.is_a? Array
         values.each do |value|
           unless value =~ /\w+=/
             raise ArgumentError, "Invalid environment setting '#{value}'"
           end
         end
       end
     end
 
     newparam(:timeout) do
       desc "The maximum time the command should take.  If the command takes
         longer than the timeout, the command is considered to have failed
-        and will be stopped.  Use any negative number to disable the timeout.
+        and will be stopped.  Use 0 to disable the timeout.
         The time is specified in seconds."
 
       munge do |value|
         value = value.shift if value.is_a?(Array)
-        if value.is_a?(String)
-          unless value =~ /^[-\d.]+$/
-            raise ArgumentError, "The timeout must be a number."
-          end
-          Float(value)
-        else
-          value
+        begin
+          value = Float(value)
+        rescue ArgumentError => e
+          raise ArgumentError, "The timeout must be a number."
         end
+        [value, 0.0].max
       end
 
       defaultto 300
     end
 
     newparam(:tries) do
       desc "The number of times execution of the command should be tried.
         Defaults to '1'. This many attempts will be made to execute
         the command until an acceptable return code is returned.
         Note that the timeout paramater applies to each try rather than
         to the complete set of tries."
 
       munge do |value|
         if value.is_a?(String)
           unless value =~ /^[\d]+$/
             raise ArgumentError, "Tries must be an integer"
           end
           value = Integer(value)
         end
         raise ArgumentError, "Tries must be an integer >= 1" if value < 1
         value
       end
 
       defaultto 1
     end
 
     newparam(:try_sleep) do
       desc "The time to sleep in seconds between 'tries'."
 
       munge do |value|
         if value.is_a?(String)
           unless value =~ /^[-\d.]+$/
             raise ArgumentError, "try_sleep must be a number"
           end
           value = Float(value)
         end
         raise ArgumentError, "try_sleep cannot be a negative number" if value < 0
         value
       end
 
       defaultto 0
     end
 
 
     newcheck(:refreshonly) do
       desc "The command should only be run as a
         refresh mechanism for when a dependent object is changed.  It only
         makes sense to use this option when this command depends on some
         other object; it is useful for triggering an action:
 
             # Pull down the main aliases file
             file { \"/etc/aliases\":
               source => \"puppet://server/module/aliases\"
             }
 
             # Rebuild the database, but only when the file changes
             exec { newaliases:
               path => [\"/usr/bin\", \"/usr/sbin\"],
               subscribe => File[\"/etc/aliases\"],
               refreshonly => true
             }
 
         Note that only `subscribe` and `notify` can trigger actions, not `require`,
         so it only makes sense to use `refreshonly` with `subscribe` or `notify`."
 
       newvalues(:true, :false)
 
       # We always fail this test, because we're only supposed to run
       # on refresh.
       def check(value)
         # We have to invert the values.
         if value == :true
           false
         else
           true
         end
       end
     end
 
     newcheck(:creates, :parent => Puppet::Parameter::Path) do
       desc "A file that this command creates.  If this
         parameter is provided, then the command will only be run
         if the specified file does not exist:
 
             exec { \"tar xf /my/tar/file.tar\":
               cwd => \"/var/tmp\",
               creates => \"/var/tmp/myfile\",
               path => [\"/usr/bin\", \"/usr/sbin\"]
             }
 
         "
 
       accept_arrays
 
       # If the file exists, return false (i.e., don't run the command),
       # else return true
       def check(value)
         ! FileTest.exists?(value)
       end
     end
 
     newcheck(:unless) do
       desc "If this parameter is set, then this `exec` will run unless
         the command returns 0.  For example:
 
             exec { \"/bin/echo root >> /usr/lib/cron/cron.allow\":
               path => \"/usr/bin:/usr/sbin:/bin\",
               unless => \"grep root /usr/lib/cron/cron.allow 2>/dev/null\"
             }
 
         This would add `root` to the cron.allow file (on Solaris) unless
         `grep` determines it's already there.
 
         Note that this command follows the same rules as the main command,
         which is to say that it must be fully qualified if the path is not set.
         "
 
       validate do |cmds|
         cmds = [cmds] unless cmds.is_a? Array
 
         cmds.each do |command|
           provider.validatecmd(command)
         end
       end
 
       # Return true if the command does not return 0.
       def check(value)
         begin
           output, status = provider.run(value, true)
         rescue Timeout::Error
           err "Check #{value.inspect} exceeded timeout"
           return false
         end
 
         status.exitstatus != 0
       end
     end
 
     newcheck(:onlyif) do
       desc "If this parameter is set, then this `exec` will only run if
         the command returns 0.  For example:
 
             exec { \"logrotate\":
               path => \"/usr/bin:/usr/sbin:/bin\",
               onlyif => \"test `du /var/log/messages | cut -f1` -gt 100000\"
             }
 
         This would run `logrotate` only if that test returned true.
 
         Note that this command follows the same rules as the main command,
         which is to say that it must be fully qualified if the path is not set.
 
         Also note that onlyif can take an array as its value, e.g.:
 
             onlyif => [\"test -f /tmp/file1\", \"test -f /tmp/file2\"]
 
         This will only run the exec if /all/ conditions in the array return true.
         "
 
       validate do |cmds|
         cmds = [cmds] unless cmds.is_a? Array
 
         cmds.each do |command|
           provider.validatecmd(command)
         end
       end
 
       # Return true if the command returns 0.
       def check(value)
         begin
           output, status = provider.run(value, true)
         rescue Timeout::Error
           err "Check #{value.inspect} exceeded timeout"
           return false
         end
 
         status.exitstatus == 0
       end
     end
 
     # Exec names are not isomorphic with the objects.
     @isomorphic = false
 
     validate do
       provider.validatecmd(self[:command])
     end
 
     # FIXME exec should autorequire any exec that 'creates' our cwd
     autorequire(:file) do
       reqs = []
 
       # Stick the cwd in there if we have it
       reqs << self[:cwd] if self[:cwd]
 
       self[:command].scan(/^(#{File::SEPARATOR}\S+)/) { |str|
         reqs << str
       }
 
       self[:command].scan(/^"([^"]+)"/) { |str|
         reqs << str
       }
 
       [:onlyif, :unless].each { |param|
         next unless tmp = self[param]
 
         tmp = [tmp] unless tmp.is_a? Array
 
         tmp.each do |line|
           # And search the command line for files, adding any we
           # find.  This will also catch the command itself if it's
           # fully qualified.  It might not be a bad idea to add
           # unqualified files, but, well, that's a bit more annoying
           # to do.
           reqs += line.scan(%r{(#{File::SEPARATOR}\S+)})
         end
       }
 
       # For some reason, the += isn't causing a flattening
       reqs.flatten!
 
       reqs
     end
 
     autorequire(:user) do
       # Autorequire users if they are specified by name
       if user = self[:user] and user !~ /^\d+$/
         user
       end
     end
 
     def self.instances
       []
     end
 
     # Verify that we pass all of the checks.  The argument determines whether
     # we skip the :refreshonly check, which is necessary because we now check
     # within refresh
     def check_all_attributes(refreshing = false)
       self.class.checks.each { |check|
         next if refreshing and check == :refreshonly
         if @parameters.include?(check)
           val = @parameters[check].value
           val = [val] unless val.is_a? Array
           val.each do |value|
             return false unless @parameters[check].check(value)
           end
         end
       }
 
       true
     end
 
     def output
       if self.property(:returns).nil?
         return nil
       else
         return self.property(:returns).output
       end
     end
 
     # Run the command, or optionally run a separately-specified command.
     def refresh
       if self.check_all_attributes(true)
         if cmd = self[:refresh]
           provider.run(cmd)
         else
           self.property(:returns).sync
         end
       end
     end
   end
 end
diff --git a/spec/unit/type/exec_spec.rb b/spec/unit/type/exec_spec.rb
index 07ad1df4c..e155506b0 100755
--- a/spec/unit/type/exec_spec.rb
+++ b/spec/unit/type/exec_spec.rb
@@ -1,677 +1,689 @@
 #!/usr/bin/env ruby
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 describe Puppet::Type.type(:exec) do
   def exec_tester(command, exitstatus = 0, rest = {})
     @user_name  = 'some_user_name'
     @group_name = 'some_group_name'
     Puppet.features.stubs(:root?).returns(true)
 
     output = rest.delete(:output) || ''
     tries  = rest[:tries] || 1
 
     args = {
       :name      => command,
       :path      => @example_path,
       :user      => @user_name,
       :group     => @group_name,
       :logoutput => false,
       :loglevel  => :err,
       :returns   => 0
     }.merge(rest)
 
     exec = Puppet::Type.type(:exec).new(args)
 
     status = stub "process", :exitstatus => exitstatus
     Puppet::Util::SUIDManager.expects(:run_and_capture).times(tries).
       with([command], @user_name, @group_name).returns([output, status])
 
     return exec
   end
 
   before do
     @command = Puppet.features.posix? ? '/bin/true whatever' : '"C:/Program Files/something.exe" whatever'
   end
 
   describe "when not stubbing the provider" do
     before do
       @executable = Puppet.features.posix? ? '/bin/true' : 'C:/Program Files/something.exe'
       File.stubs(:exists?).returns false
       File.stubs(:exists?).with(@executable).returns true
       File.stubs(:exists?).with('/bin/false').returns true
       @example_path = Puppet.features.posix? ? %w{/usr/bin /bin} : [ "C:/Program Files/something/bin", "C:/Ruby/bin" ]
       File.stubs(:exists?).with(File.join(@example_path[0],"true")).returns true
       File.stubs(:exists?).with(File.join(@example_path[0],"false")).returns true
     end
 
     it "should return :executed_command as its event" do
       resource = Puppet::Type.type(:exec).new :command => @command
       resource.parameter(:returns).event.name.should == :executed_command
     end
 
     describe "when execing" do
       it "should use the 'run_and_capture' method to exec" do
         exec_tester("true").refresh.should == :executed_command
       end
 
       it "should report a failure" do
         proc { exec_tester('false', 1).refresh }.
           should raise_error(Puppet::Error, /^false returned 1 instead of/)
       end
 
       it "should not report a failure if the exit status is specified in a returns array" do
         proc { exec_tester("false", 1, :returns => [0, 1]).refresh }.should_not raise_error
       end
 
       it "should report a failure if the exit status is not specified in a returns array" do
         proc { exec_tester('false', 1, :returns => [0, 100]).refresh }.
           should raise_error(Puppet::Error, /^false returned 1 instead of/)
       end
 
       it "should log the output on success" do
         output = "output1\noutput2\n"
         exec_tester('false', 0, :output => output, :logoutput => true).refresh
         output.split("\n").each do |line|
           log = @logs.shift
           log.level.should == :err
           log.message.should == line
         end
       end
 
       it "should log the output on failure" do
         output = "output1\noutput2\n"
         proc { exec_tester('false', 1, :output => output, :logoutput => true).refresh }.
           should raise_error(Puppet::Error)
 
         output.split("\n").each do |line|
           log = @logs.shift
           log.level.should == :err
           log.message.should == line
         end
       end
     end
 
     describe "when logoutput=>on_failure is set" do
       it "should log the output on failure" do
         output = "output1\noutput2\n"
         proc { exec_tester('false', 1, :output => output, :logoutput => :on_failure).refresh }.
           should raise_error(Puppet::Error, /^false returned 1 instead of/)
 
         output.split("\n").each do |line|
           log = @logs.shift
           log.level.should == :err
           log.message.should == line
         end
       end
 
       it "should log the output on failure when returns is specified as an array" do
         output = "output1\noutput2\n"
 
         proc {
           exec_tester('false', 1, :output => output, :returns => [0, 100],
                :logoutput => :on_failure).refresh
         }.should raise_error(Puppet::Error, /^false returned 1 instead of/)
 
         output.split("\n").each do |line|
           log = @logs.shift
           log.level.should == :err
           log.message.should == line
         end
       end
 
       it "shouldn't log the output on success" do
         exec_tester('true', 0, :output => "a\nb\nc\n", :logoutput => :on_failure).refresh
         @logs.should == []
       end
     end
 
     it "shouldn't log the output on success when non-zero exit status is in a returns array" do
       exec_tester("true", 100, :output => "a\n", :logoutput => :on_failure, :returns => [1, 100]).refresh
       @logs.should == []
     end
 
     describe " when multiple tries are set," do
       it "should repeat the command attempt 'tries' times on failure and produce an error" do
         tries = 5
         resource = exec_tester("false", 1, :tries => tries, :try_sleep => 0)
         proc { resource.refresh }.should raise_error(Puppet::Error)
       end
     end
   end
 
   it "should be able to autorequire files mentioned in the command" do
     catalog = Puppet::Resource::Catalog.new
     tmp = Puppet::Type.type(:file).new(:name => "/bin/foo")
     catalog.add_resource tmp
     execer = Puppet::Type.type(:exec).new(:name => "/bin/foo")
     catalog.add_resource execer
 
     catalog.relationship_graph.dependencies(execer).should == [tmp]
   end
 
   describe "when handling the path parameter" do
     expect = %w{one two three four}
     { "an array"                        => expect,
       "a colon separated list"          => "one:two:three:four",
       "a semi-colon separated list"     => "one;two;three;four",
       "both array and colon lists"      => ["one", "two:three", "four"],
       "both array and semi-colon lists" => ["one", "two;three", "four"],
       "colon and semi-colon lists"      => ["one:two", "three;four"]
     }.each do |test, input|
       it "should accept #{test}" do
         type = Puppet::Type.type(:exec).new(:name => @command, :path => input)
         type[:path].should == expect
       end
     end
   end
 
   describe "when setting user" do
     it "should fail if we are not root" do
       Puppet.features.stubs(:root?).returns(false)
       expect { Puppet::Type.type(:exec).new(:name => @command, :user => 'input') }.
         should raise_error Puppet::Error, /Parameter user failed/
     end
 
     ['one', 2, 'root', 4294967295, 4294967296].each do |value|
       it "should accept '#{value}' as user if we are root" do
         Puppet.features.stubs(:root?).returns(true)
         type = Puppet::Type.type(:exec).new(:name => @command, :user => value)
         type[:user].should == value
       end
     end
   end
 
   describe "when setting group" do
     shared_examples_for "exec[:group]" do
       ['one', 2, 'wheel', 4294967295, 4294967296].each do |value|
         it "should accept '#{value}' without error or judgement" do
           type = Puppet::Type.type(:exec).new(:name => @command, :group => value)
           type[:group].should == value
         end
       end
     end
 
     describe "when running as root" do
       before :each do Puppet.features.stubs(:root?).returns(true) end
       it_behaves_like "exec[:group]"
     end
 
     describe "when not running as root" do
       before :each do Puppet.features.stubs(:root?).returns(false) end
       it_behaves_like "exec[:group]"
     end
   end
 
   describe "when setting cwd" do
     it_should_behave_like "all path parameters", :cwd, :array => false do
       def instance(path)
         Puppet::Type.type(:exec).new(:name => '/bin/true', :cwd => path)
       end
     end
   end
 
   shared_examples_for "all exec command parameters" do |param|
     { "relative" => "example", "absolute" => "/bin/example" }.sort.each do |name, command|
       describe "if command is #{name}" do
         before :each do
           @param = param
         end
 
         def test(command, valid)
           if @param == :name then
             instance = Puppet::Type.type(:exec).new()
           else
             instance = Puppet::Type.type(:exec).new(:name => "/bin/true")
           end
           if valid then
             instance.provider.expects(:validatecmd).returns(true)
           else
             instance.provider.expects(:validatecmd).raises(Puppet::Error, "from a stub")
           end
           instance[@param] = command
         end
 
         it "should work if the provider calls the command valid" do
           expect { test(command, true) }.should_not raise_error
         end
 
         it "should fail if the provider calls the command invalid" do
           expect { test(command, false) }.
             should raise_error Puppet::Error, /Parameter #{@param} failed: from a stub/
         end
       end
     end
   end
 
   shared_examples_for "all exec command parameters that take arrays" do |param|
     describe "when given an array of inputs" do
       before :each do
         @test = Puppet::Type.type(:exec).new(:name => "/bin/true")
       end
 
       it "should accept the array when all commands return valid" do
         input = %w{one two three}
         @test.provider.expects(:validatecmd).times(input.length).returns(true)
         @test[param] = input
         @test[param].should == input
       end
 
       it "should reject the array when any commands return invalid" do
         input = %w{one two three}
         @test.provider.expects(:validatecmd).with(input.first).returns(false)
         input[1..-1].each do |cmd|
           @test.provider.expects(:validatecmd).with(cmd).returns(true)
         end
         @test[param] = input
         @test[param].should == input
       end
 
       it "should reject the array when all commands return invalid" do
         input = %w{one two three}
         @test.provider.expects(:validatecmd).times(input.length).returns(false)
         @test[param] = input
         @test[param].should == input
       end
     end
   end
 
   describe "when setting refresh" do
     it_should_behave_like "all exec command parameters", :refresh
   end
 
   describe "for simple parameters" do
     before :each do
       @exec = Puppet::Type.type(:exec).new(:name => '/bin/true')
     end
 
     describe "when setting env" do
       it "should issue a deprecation warning" do
         expect { @exec[:env] = 'foo=bar' }.should_not raise_error
         @logs.first.message.should =~ /deprecate.*environment/
       end
 
       it "should update the value of env" do
         data = ['foo=bar']
         @exec[:env] = data
         @exec[:env].should == data
       end
 
       it "should forward to environment" do
         data = ['foo=bar']
         @exec[:env] = data
         @exec[:environment].should == data
       end
 
       it "should not override environment if both are set" do
         pending "can't fix: too disruptive for 2.6, removed in 2.7"
         # ...so this test is here to validate that we know about the problem.
         # This ensures correct order of evaluation to trigger the bug; don't
         # count on this happening in the constructor. --daniel 2011-03-01
         @exec[:environment] = 'environment=true'
         @exec[:env]         = 'env=true'
 
         @exec[:environment].should == "environment=true"
       end
     end
 
     describe "when setting environment" do
       { "single values"   => "foo=bar",
         "multiple values" => ["foo=bar", "baz=quux"],
       }.each do |name, data|
         it "should accept #{name}" do
           @exec[:environment] = data
           @exec[:environment].should == data
         end
       end
 
       { "single values" => "foo",
         "only values"   => ["foo", "bar"],
         "any values"    => ["foo=bar", "baz"]
       }.each do |name, data|
         it "should reject #{name} without assignment" do
           expect { @exec[:environment] = data }.
             should raise_error Puppet::Error, /Invalid environment setting/
         end
       end
     end
 
     describe "when setting timeout" do
-      [-3.5, -1, 0, 0.1, 1, 10, 4294967295].each do |valid|
+      [0, 0.1, 1, 10, 4294967295].each do |valid|
         it "should accept '#{valid}' as valid" do
           @exec[:timeout] = valid
           @exec[:timeout].should == valid
         end
 
         it "should accept '#{valid}' in an array as valid" do
           @exec[:timeout] = [valid]
           @exec[:timeout].should == valid
         end
       end
 
-      ['1/2', '1_000_000', '+12', '', 'foo'].each do |invalid|
+      ['1/2', '', 'foo', '5foo'].each do |invalid|
         it "should reject '#{invalid}' as invalid" do
           expect { @exec[:timeout] = invalid }.
             should raise_error Puppet::Error, /The timeout must be a number/
         end
 
         it "should reject '#{invalid}' in an array as invalid" do
           expect { @exec[:timeout] = [invalid] }.
             should raise_error Puppet::Error, /The timeout must be a number/
         end
       end
 
       it "should fail if timeout is exceeded" do
         File.stubs(:exists?).with('/bin/sleep').returns(true)
         File.stubs(:exists?).with('sleep').returns(false)
         sleep_exec = Puppet::Type.type(:exec).new(:name => 'sleep 1', :path => ['/bin'], :timeout => '0.2')
         lambda { sleep_exec.refresh }.should raise_error Puppet::Error, "Command exceeded timeout"
       end
+
+      it "should convert timeout to a float" do
+        resource = Puppet::Type.type(:exec).new :command => "/bin/false", :timeout => "12"
+        resource[:timeout].should be_a(Float)
+        resource[:timeout].should == 12.0
+      end
+
+      it "should munge negative timeouts to 0.0" do
+        resource = Puppet::Type.type(:exec).new :command => "/bin/false", :timeout => "-12.0"
+        resource.parameter(:timeout).value.should be_a(Float)
+        resource.parameter(:timeout).value.should == 0.0
+      end
     end
 
     describe "when setting tries" do
       [1, 10, 4294967295].each do |valid|
         it "should accept '#{valid}' as valid" do
           @exec[:tries] = valid
           @exec[:tries].should == valid
         end
 
         if "REVISIT: too much test log spam" == "a good thing" then
           it "should accept '#{valid}' in an array as valid" do
             pending "inconsistent, but this is not supporting arrays, unlike timeout"
             @exec[:tries] = [valid]
             @exec[:tries].should == valid
           end
         end
       end
 
       [-3.5, -1, 0, 0.2, '1/2', '1_000_000', '+12', '', 'foo'].each do |invalid|
         it "should reject '#{invalid}' as invalid" do
           expect { @exec[:tries] = invalid }.
             should raise_error Puppet::Error, /Tries must be an integer/
         end
 
         if "REVISIT: too much test log spam" == "a good thing" then
           it "should reject '#{invalid}' in an array as invalid" do
             pending "inconsistent, but this is not supporting arrays, unlike timeout"
             expect { @exec[:tries] = [invalid] }.
               should raise_error Puppet::Error, /Tries must be an integer/
           end
         end
       end
     end
 
     describe "when setting try_sleep" do
       [0, 0.2, 1, 10, 4294967295].each do |valid|
         it "should accept '#{valid}' as valid" do
           @exec[:try_sleep] = valid
           @exec[:try_sleep].should == valid
         end
 
         if "REVISIT: too much test log spam" == "a good thing" then
           it "should accept '#{valid}' in an array as valid" do
             pending "inconsistent, but this is not supporting arrays, unlike timeout"
             @exec[:try_sleep] = [valid]
             @exec[:try_sleep].should == valid
           end
         end
       end
 
       { -3.5        => "cannot be a negative number",
         -1          => "cannot be a negative number",
         '1/2'       => 'must be a number',
         '1_000_000' => 'must be a number',
         '+12'       => 'must be a number',
         ''          => 'must be a number',
         'foo'       => 'must be a number',
       }.each do |invalid, error|
         it "should reject '#{invalid}' as invalid" do
           expect { @exec[:try_sleep] = invalid }.
             should raise_error Puppet::Error, /try_sleep #{error}/
         end
 
         if "REVISIT: too much test log spam" == "a good thing" then
           it "should reject '#{invalid}' in an array as invalid" do
             pending "inconsistent, but this is not supporting arrays, unlike timeout"
             expect { @exec[:try_sleep] = [invalid] }.
               should raise_error Puppet::Error, /try_sleep #{error}/
           end
         end
       end
     end
 
     describe "when setting refreshonly" do
       [:true, :false].each do |value|
         it "should accept '#{value}'" do
           @exec[:refreshonly] = value
           @exec[:refreshonly].should == value
         end
       end
 
       [1, 0, "1", "0", "yes", "y", "no", "n"].each do |value|
         it "should reject '#{value}'" do
           expect { @exec[:refreshonly] = value }.
             should raise_error(Puppet::Error,
               /Invalid value #{value.inspect}\. Valid values are true, false/
             )
         end
       end
     end
 
     describe "when setting creates" do
       it_should_behave_like "all path parameters", :creates, :array => true do
         def instance(path)
           Puppet::Type.type(:exec).new(:name => '/bin/true', :creates => path)
         end
       end
     end
   end
 
   describe "when setting unless" do
     it_should_behave_like "all exec command parameters", :unless
     it_should_behave_like "all exec command parameters that take arrays", :unless
   end
 
   describe "when setting onlyif" do
     it_should_behave_like "all exec command parameters", :onlyif
     it_should_behave_like "all exec command parameters that take arrays", :onlyif
   end
 
   describe "#check" do
     before :each do
       @test = Puppet::Type.type(:exec).new(:name => "/bin/true")
     end
 
     describe ":refreshonly" do
       { :true => false, :false => true }.each do |input, result|
         it "should return '#{result}' when given '#{input}'" do
           @test[:refreshonly] = input
           @test.check_all_attributes.should == result
         end
       end
     end
 
     describe ":creates" do
       before :all do
         @exist   = "/"
         @unexist = "/this/path/should/never/exist"
         while FileTest.exist?(@unexist) do @unexist += "/foo" end
       end
 
       context "with a single item" do
         it "should run when the item does not exist" do
           @test[:creates] = @unexist
           @test.check_all_attributes.should == true
         end
 
         it "should not run when the item exists" do
           @test[:creates] = @exist
           @test.check_all_attributes.should == false
         end
       end
 
       context "with an array with one item" do
         it "should run when the item does not exist" do
           @test[:creates] = [@unexist]
           @test.check_all_attributes.should == true
         end
 
         it "should not run when the item exists" do
           @test[:creates] = [@exist]
           @test.check_all_attributes.should == false
         end
       end
 
       context "with an array with multiple items" do
         it "should run when all items do not exist" do
           @test[:creates] = [@unexist] * 3
           @test.check_all_attributes.should == true
         end
 
         it "should not run when one item exists" do
           @test[:creates] = [@unexist, @exist, @unexist]
           @test.check_all_attributes.should == false
         end
 
         it "should not run when all items exist" do
           @test[:creates] = [@exist] * 3
         end
       end
     end
 
     { :onlyif => { :pass => false, :fail => true  },
       :unless => { :pass => true,  :fail => false },
     }.each do |param, sense|
       describe ":#{param}" do
         before :each do
           @pass = "/magic/pass"
           @fail = "/magic/fail"
 
           @pass_status = stub('status', :exitstatus => sense[:pass] ? 0 : 1)
           @fail_status = stub('status', :exitstatus => sense[:fail] ? 0 : 1)
 
           @test.provider.stubs(:checkexe).returns(true)
           [true, false].each do |check|
             @test.provider.stubs(:run).with(@pass, check).
               returns(['test output', @pass_status])
             @test.provider.stubs(:run).with(@fail, check).
               returns(['test output', @fail_status])
           end
         end
 
         context "with a single item" do
           it "should run if the command exits non-zero" do
             @test[param] = @fail
             @test.check_all_attributes.should == true
           end
 
           it "should not run if the command exits zero" do
             @test[param] = @pass
             @test.check_all_attributes.should == false
           end
         end
 
         context "with an array with a single item" do
           it "should run if the command exits non-zero" do
             @test[param] = [@fail]
             @test.check_all_attributes.should == true
           end
 
           it "should not run if the command exits zero" do
             @test[param] = [@pass]
             @test.check_all_attributes.should == false
           end
         end
 
         context "with an array with multiple items" do
           it "should run if all the commands exits non-zero" do
             @test[param] = [@fail] * 3
             @test.check_all_attributes.should == true
           end
 
           it "should not run if one command exits zero" do
             @test[param] = [@pass, @fail, @pass]
             @test.check_all_attributes.should == false
           end
 
           it "should not run if all command exits zero" do
             @test[param] = [@pass] * 3
             @test.check_all_attributes.should == false
           end
         end
       end
     end
   end
 
   describe "#retrieve" do
     before :each do
       @exec_resource = Puppet::Type.type(:exec).new(:name => "/bogus/cmd")
     end
 
     it "should return :notrun when check_all_attributes returns true" do
       @exec_resource.stubs(:check_all_attributes).returns true
       @exec_resource.retrieve[:returns].should == :notrun
     end
 
     it "should return default exit code 0 when check_all_attributes returns false" do
       @exec_resource.stubs(:check_all_attributes).returns false
       @exec_resource.retrieve[:returns].should == ['0']
     end
 
     it "should return the specified exit code when check_all_attributes returns false" do
       @exec_resource.stubs(:check_all_attributes).returns false
       @exec_resource[:returns] = 42
       @exec_resource.retrieve[:returns].should == ["42"]
     end
   end
 
   describe "#output" do
     before :each do
       @exec_resource = Puppet::Type.type(:exec).new(:name => "/bogus/cmd")
     end
 
     it "should return the provider's run output" do
       provider = stub 'provider'
       status = stubs "process_status"
       status.stubs(:exitstatus).returns("0")
       provider.expects(:run).returns(["silly output", status])
       @exec_resource.stubs(:provider).returns(provider)
 
       @exec_resource.refresh
       @exec_resource.output.should == 'silly output'
     end
   end
 
   describe "#refresh" do
     before :each do
       @exec_resource = Puppet::Type.type(:exec).new(:name => "/bogus/cmd")
     end
 
     it "should call provider run with the refresh parameter if it is set" do
       provider = stub 'provider'
       @exec_resource.stubs(:provider).returns(provider)
       @exec_resource.stubs(:[]).with(:refresh).returns('/myother/bogus/cmd')
       provider.expects(:run).with('/myother/bogus/cmd')
 
       @exec_resource.refresh
     end
 
     it "should call provider run with the specified command if the refresh parameter is not set" do
       provider = stub 'provider'
       status = stubs "process_status"
       status.stubs(:exitstatus).returns("0")
       provider.expects(:run).with('/bogus/cmd').returns(["silly output", status])
       @exec_resource.stubs(:provider).returns(provider)
 
       @exec_resource.refresh
     end
 
     it "should not run the provider if check_all_attributes is false" do
       @exec_resource.stubs(:check_all_attributes).returns false
       provider = stub 'provider'
       provider.expects(:run).never
       @exec_resource.stubs(:provider).returns(provider)
 
       @exec_resource.refresh
     end
   end
 end