Further progress towards the next release. Lots of small bugs fixed, the paths look much better now, and Transportable is much cleaner.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@740 980ebf18-57e1-0310-9a29-db15c13687c0

22 files changed, 443 insertions, 166 deletions

diff --git a/lib/puppet.rb b/lib/puppet.rb
index b66e00cb8..229816641 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -29,12 +29,12 @@ module Puppet
             str = nil
             if defined? @file and defined? @line and @file and @line
                 str = "%s in file %s at line %s" %
-                    [@message, @file, @line]
+                    [@message.to_s, @file, @line]
             elsif defined? @line and @line
                 str = "%s at line %s" %
-                    [@message, @line]
+                    [@message.to_s, @line]
             else
-                str = @message
+                str = @message.to_s
             end
 
             #if Puppet[:debug] and @stack
diff --git a/lib/puppet/client.rb b/lib/puppet/client.rb
index ece131b7b..56e78349a 100644
--- a/lib/puppet/client.rb
+++ b/lib/puppet/client.rb
@@ -274,7 +274,14 @@ module Puppet
                 transaction = container.evaluate
                 #transaction = Puppet::Transaction.new(objects)
                 transaction.toplevel = true
-                transaction.evaluate
+                begin
+                    transaction.evaluate
+                rescue Puppet::Error => detail
+                    Puppet.err "Could not apply complete configuration: %s" %
+                        detail
+                rescue => detail
+                    Puppet.err "Found a bug: %s" % detail
+                end
                 Puppet::Metric.gather
                 Puppet::Metric.tally
                 if Puppet[:rrdgraph] == true
diff --git a/lib/puppet/element.rb b/lib/puppet/element.rb
index 26c82ddf5..3991160db 100644
--- a/lib/puppet/element.rb
+++ b/lib/puppet/element.rb
@@ -1,12 +1,6 @@
-#!/usr/local/bin/ruby -w
-
-# $Id$
-
 # included so we can test object types
 require 'puppet'
 
-
-#---------------------------------------------------------------
 # the base class for both types and states
 # very little functionality; basically just defines the interface
 # and provides a few simple across-the-board functions like 'noop'
@@ -18,10 +12,9 @@ class Puppet::Element
         attr_accessor :doc, :nodoc
     end
 
-    #---------------------------------------------------------------
     # all of our subclasses must respond to each of these methods...
     @@interface_methods = [
-        :retrieve, :insync?, :sync, :path, :evaluate
+        :retrieve, :insync?, :sync, :evaluate
     ]
 
     # so raise an error if a method that isn't overridden gets called
@@ -31,7 +24,6 @@ class Puppet::Element
                 [self.class,method]
         }
     }
-    #---------------------------------------------------------------
 
     # create instance methods for each of the log levels, too
     Puppet::Log.eachlevel { |level|
@@ -47,7 +39,6 @@ class Puppet::Element
         })
     }
 
-    #---------------------------------------------------------------
     # for testing whether we should actually do anything
     def noop
         unless defined? @noop
@@ -55,7 +46,35 @@ class Puppet::Element
         end
         return @noop || Puppet[:noop] || false
     end
-    #---------------------------------------------------------------
+
+    # return the full path to us, for logging and rollback
+    # some classes (e.g., FileTypeRecords) will have to override this
+    def path
+        unless defined? @path
+            if defined? @parent and @parent
+                if self.is_a?(Puppet::Type::Component)
+                    @path = [@parent.path, self.name]
+                else
+                    @path = [@parent.path, self.class.name.to_s + "=" + self.name]
+                end
+            else
+                # The top-level name is always puppet[top], so we don't bother with
+                # that.  And we don't add the hostname here, it gets added
+                # in the log server thingy.
+                if self.name == "puppet[top]"
+                    @path = ["/"]
+                else
+                    # We assume that if we don't have a parent that we should not
+                    # cache the path
+                    Puppet.warning "%s has no parent" % self.name
+                    @path = [self.class.name.to_s + "=" + self.name]
+                end
+            end
+        end
+
+        return @path.join("/")
+    end
 
 end
-#---------------------------------------------------------------
+
+# $Id$
diff --git a/lib/puppet/log.rb b/lib/puppet/log.rb
index c470dd5df..6120df0ac 100644
--- a/lib/puppet/log.rb
+++ b/lib/puppet/log.rb
@@ -182,7 +182,7 @@ module Puppet # :nodoc:
                             msg.level, msg.to_s
                         ] + RESET
                     else
-                        puts @colors[msg.level] + "%s (%s): %s" % [
+                        puts @colors[msg.level] + "%s: %s: %s" % [
                             msg.source, msg.level, msg.to_s
                         ] + RESET
                     end
@@ -257,7 +257,8 @@ module Puppet # :nodoc:
             @levels.include?(level)
         end
 
-		attr_accessor :level, :message, :source, :time, :tags, :remote
+		attr_accessor :level, :message, :time, :tags, :remote
+        attr_reader :source
 
 		def initialize(args)
 			unless args.include?(:level) && args.include?(:message)
@@ -272,7 +273,7 @@ module Puppet # :nodoc:
 				raise Puppet::DevError,
                     "Level is not a string or symbol: #{args[:level].class}"
 			end
-			@message = args[:message]
+			@message = args[:message].to_s
 			@time = Time.now
 			# this should include the host name, and probly lots of other
 			# stuff, at some point
@@ -285,17 +286,7 @@ module Puppet # :nodoc:
             end
 
             if args.include?(:source)
-                # We can't store the actual source, we just store the path
-                if args[:source].respond_to?(:path)
-                    @source = args[:source].path
-                else
-                    @source = args[:source].to_s
-                end
-                unless defined? @tags and @tags
-                    if args[:source].respond_to?(:tags)
-                        @tags = args[:source].tags
-                    end
-                end
+                self.source = args[:source]
             else
                 @source = "Puppet"
             end
@@ -303,6 +294,20 @@ module Puppet # :nodoc:
             Log.newmessage(self)
 		end
 
+        def source=(source)
+            # We can't store the actual source, we just store the path
+            if source.respond_to?(:path)
+                @source = source.path
+            else
+                @source = source.to_s
+            end
+            unless defined? @tags and @tags
+                if source.respond_to?(:tags)
+                    @tags = source.tags
+                end
+            end
+        end
+
 		def to_s
             return @message
 		end
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index 6e0854206..996a497d5 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -544,15 +544,13 @@ module Puppet
                     if tmp = self.objectdefined?(name, type)
                         msg = "Duplicate definition: %s[%s] is already defined" %
                             [type, name]
+                        error = Puppet::ParseError.new(msg)
                         if tmp.line
-                            msg += " at line %s" % tmp.line
+                            error.line = tmp.line
                         end
                         if tmp.file
-                            msg += " in file %s" % tmp.file
+                            error.file = tmp.file
                         end
-                        error = Puppet::ParseError.new(msg)
-                        error.file = file
-                        error.line = line
                         raise error
                     end
 
diff --git a/lib/puppet/server/master.rb b/lib/puppet/server/master.rb
index d5dc6bc22..261d93d97 100644
--- a/lib/puppet/server/master.rb
+++ b/lib/puppet/server/master.rb
@@ -64,9 +64,9 @@ class Server
                 begin
                     facts = Marshal::load(CGI.unescape(facts))
                 rescue => detail
-                    puts "AAAAA"
-                    puts detail
-                    exit
+                    raise XMLRPC::FaultException.new(
+                        1, "Could not rebuild facts"
+                    )
                 end
             end
 
diff --git a/lib/puppet/statechange.rb b/lib/puppet/statechange.rb
index 2221088b0..8b8478799 100644
--- a/lib/puppet/statechange.rb
+++ b/lib/puppet/statechange.rb
@@ -26,14 +26,13 @@ module Puppet
 		#---------------------------------------------------------------
         def go
             if @state.is == @state.should
-                @state.info "%s.%s is already in sync" %
-                    [@state.parent.name, @state.name]
+                @state.info "already in sync"
                 return nil
             end
 
             if @state.noop
-                @state.parent.log "%s is %s, should be %s" %
-                    [@state, state.is_to_s, state.should_to_s]
+                @state.log "is %s, should be %s" %
+                    [state.is_to_s, state.should_to_s]
                 #@state.debug "%s is noop" % @state
                 return nil
             end
@@ -65,8 +64,7 @@ module Puppet
                     # should basically point to that, right?
                         #:state => @state,
                         #:object => @state.parent,
-                    # FIXME this is where loglevel stuff should go
-                    @state.parent.log @state.change_to_s
+                    @state.log @state.change_to_s
                     Puppet::Event.new(
                         :event => event,
                         :change => self,
@@ -85,7 +83,7 @@ module Puppet
                 #    pname = pname.id2name
                 #end
                     #:state => @state,
-                @state.parent.log "Failed: " + @state.change_to_s
+                @state.log "Failed: " + @state.change_to_s
                 return Puppet::Event.new(
                     :event => pname + "_failed",
                     :change => self,
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 57f43b17a..47b18986e 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -53,7 +53,7 @@ class Transaction
                     events = [change.forward].flatten.reject { |e| e.nil? }
                     #@@changed.push change.state.parent
                 rescue => detail
-                    Puppet.err("%s failed: %s" % [change,detail])
+                    Puppet.err("%s failed: %s" % [change.to_s,detail])
                     if Puppet[:debug] and detail.respond_to?(:stack)
                         puts detail.stack
                     end
@@ -67,9 +67,6 @@ class Transaction
                     change.changed = true
                 end
                 events
-            elsif change.is_a?(Puppet::Transaction)
-                raise Puppet::DevError, "Got a sub-transaction"
-                change.evaluate
             else
                 raise Puppet::DevError,
                     "Transactions cannot handle objects of type %s" % child.class
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index 5ea00713e..ffa751eb6 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -27,7 +27,10 @@ module Puppet
             return "%s(%s) => %s" % [@type,self[:name],super]
         end
 
-        def to_type
+        def to_type(parent = nil)
+            if parent
+                self[:parent] = parent
+            end
             retobj = nil
             if type = Puppet::Type.type(self.type)
                 unless retobj = type.create(self)
@@ -44,6 +47,10 @@ module Puppet
                 retobj.tags = @tags
             end
 
+            if parent
+                parent.push retobj
+            end
+
             return retobj
         end
     end
@@ -60,14 +67,15 @@ module Puppet
                 when Puppet::TransBucket, Puppet::TransObject
                     # nada
                 else
-                    raise "TransBuckets cannot handle objects of type %s" %
-                        arg.class
+                    raise Puppet::DevError,
+                        "TransBuckets cannot handle objects of type %s" %
+                            arg.class
                 end
             }
             super
         end
 
-        def to_type
+        def to_type(parent = nil)
             # this container will contain the equivalent of all objects at
             # this level
             #container = Puppet::Component.new(:name => @name, :type => @type)
@@ -88,64 +96,39 @@ module Puppet
                     hash[param] = value
                 }
             else
-                Puppet.debug "%s has no parameters" % @name
+                Puppet.debug "%s[%s] has no parameters" % [@type, @name]
+            end
+
+            if parent
+                hash[:parent] = parent
             end
             container = Puppet::Type::Component.create(hash)
 
+            if parent
+                parent.push container
+            end
+
             # unless we successfully created the container, return an error
             unless container
+                Puppet.warning "Got no container back"
                 return nil
             end
-            nametable = {}
 
             self.each { |child|
                 # the fact that we descend here means that we are
                 # always going to execute depth-first
                 # which is _probably_ a good thing, but one never knows...
-                if child.is_a?(Puppet::TransBucket)
-                    # just perform the same operation on any children
-                    obj = child.to_type
-                    if obj
-                        container.push(obj)
-                    else
-                        # FIXME we need to figure out error handling.
-                        # really
-                    end
-                elsif child.is_a?(Puppet::TransObject)
-                    # do a simple little naming hack to see if the object already
-                    # exists in our scope
-                    # this assumes that type/name combinations are globally
-                    # unique
-
-                    # FIXME this still might be wrong, because it doesn't search
-                    # up scopes
-                    # either that, or it's redundant
-                    name = [child[:name],child.type].join("--")
-
-                    if nametable.include?(name)
-                        object = nametable[name]
-                        child.each { |var,value|
-                            # don't rename; this shouldn't be possible anyway
-                            next if var == :name
-
-                            Puppet.debug "Adding %s to %s" % [var,name]
-                            # override any existing values
-                            object[var] = value
-                        }
-                        object.parent = self
-                    else # the object does not exist yet in our scope
-                        # now we have the object instantiated, in our scope
-                        if object = child.to_type
-                            # the object will be nil if it failed
-                            nametable[name] = object
-
-                            # this sets the order of the object
-                            container.push object
-                        end
-                    end
-                else
-                    raise "TransBucket#to_type cannot handle objects of type %s" %
-                        child.class
+                unless  child.is_a?(Puppet::TransBucket) or
+                        child.is_a?(Puppet::TransObject)
+                    raise Puppet::DevError,
+                        "TransBucket#to_type cannot handle objects of type %s" %
+                            child.class
+                end
+
+                # Now just call to_type on them with the container as a parent
+                unless obj = child.to_type(container)
+                    # nothing; we assume the method already warned
+                    Puppet.warning "Could not create child %s" % child.name
                 end
             }
 
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 6ac98359e..528453f1a 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -206,7 +206,7 @@ class Type < Puppet::Element
         if object.is_a?(Puppet::Type)
             newobj = object
         else
-            raise "must pass a Puppet::Type object"
+            raise Puppet::DevError, "must pass a Puppet::Type object"
         end
 
         if @objects.has_key?(newobj.name)
@@ -391,6 +391,12 @@ class Type < Puppet::Element
             else
                 return nil
             end
+        elsif Puppet::Type.metaparam?(name)
+            if @metaparams.include?(name)
+                return @metaparams[name]
+            else
+                return nil
+            end
         elsif self.class.validparameter?(name)
             if @parameters.include?(name)
                 return @parameters[name]
@@ -453,10 +459,17 @@ class Type < Puppet::Element
     # remove a state from the object; useful in testing or in cleanup
     # when an error has been encountered
     def delete(attr)
-        if @states.has_key?(attr)
-            @states.delete(attr)
+        case attr
+        when Puppet::Type
+            if @children.include?(attr)
+                @children.delete(attr)
+            end
         else
-            raise Puppet::DevError.new("Undefined state '#{attr}' in #{self}")
+            if @states.has_key?(attr)
+                @states.delete(attr)
+            else
+                raise Puppet::DevError.new("Undefined state '#{attr}' in #{self}")
+            end
         end
     end
 
@@ -776,6 +789,12 @@ class Type < Puppet::Element
         @syncedchanges = 0
         @failedchanges = 0
 
+        # Before anything else, set our parent if it was included
+        if hash.include?(:parent)
+            @parent = hash[:parent]
+            hash.delete(:parent)
+        end
+
         hash = self.argclean(hash)
 
         # now get all of the arguments, in a specific order
@@ -886,6 +905,10 @@ class Type < Puppet::Element
         # we have to set the name of our object before anything else,
         # because it might be used in creating the other states
         hash = hash.dup
+
+        if hash.include?(:parent)
+            hash.delete(:parent)
+        end
         namevar = self.class.namevar
 
         hash.each { |var,value|
@@ -913,27 +936,6 @@ class Type < Puppet::Element
         return hash
     end
 
-    # return the full path to us, for logging and rollback
-    # some classes (e.g., FileTypeRecords) will have to override this
-    def path
-        unless defined? @path
-            if defined? @parent
-                if self.is_a?(Puppet::Type::Component)
-                    @path = @parent.path + "/" + self.name
-                else
-                    @path = @parent.path + "/" + self.class.name.to_s + "=" + self.name
-                end
-            else
-                # The top-level name is always puppet[top], so we don't bother with
-                # that.  And we don't add the hostname here, it gets added
-                # in the log server thingy.
-                @path = "/"
-            end
-        end
-
-        return @path
-    end
-
     # retrieve the current value of all contained states
     def retrieve
         # it's important to use the method here, as it follows the order
@@ -1143,26 +1145,30 @@ class Type < Puppet::Element
         end
     end
 
+    # Currently nonfunctional
     def metaonerror=(response)
         self.debug("Would have called metaonerror")
         @onerror = response
     end
 
+    # Currently nonfunctional
     def metaschedule=(schedule)
         @schedule = schedule
     end
+
+    # Determines what our objects log at.  Defaults to :notice.
     def metaloglevel=(loglevel)
         if loglevel.is_a?(String)
-            loglevel.intern
+            loglevel = loglevel.intern
         end
         if loglevel == :verbose
             loglevel = :info 
         end        
 
-        if Puppet::Log.levels.include?(loglevel)
+        if Puppet::Log.validlevel?(loglevel)
             @metaparams[:loglevel] = loglevel
         else
-            raise Puppet::Error.new("Invalid loglevel '%s%'" % loglevel)       
+            raise Puppet::Error.new("Invalid loglevel '%s'" % loglevel)       
         end
     end
 
diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb
index c3a3440c3..6e4438ab3 100644
--- a/lib/puppet/type/component.rb
+++ b/lib/puppet/type/component.rb
@@ -82,8 +82,6 @@ module Puppet
                 args[:type] = "component"
             end
             super(args)
-            #self.debug "Made component with name %s and type %s" %
-            #    [self.name, self[:type]]
         end
 
         # flatten all children, sort them, and evaluate them in order
diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index 428c1d021..59c7c40e4 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -185,8 +185,7 @@ module Puppet
                 if self[:command] !~ /^\//
                     if self[:path].nil?
                         raise TypeError,
-                            "'%s' is both unqualifed and specified no search path" %
-                                self[:command]
+                            "both unqualifed and specified no search path"
                     end
                 end
             end
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index 2c1a68a25..ff097fe64 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -274,6 +274,7 @@ module Puppet
                     }
                 else # create it anew
                     #notice "Creating new file with args %s" % args.inspect
+                    args[:parent] = self
                     begin
                         child = klass.implicitcreate(args)
                         
@@ -281,7 +282,6 @@ module Puppet
                         if child.nil?
                             return nil
                         end
-                        child.parent = self
                         @children << child
                     rescue Puppet::Error => detail
                         self.notice(
@@ -302,6 +302,41 @@ module Puppet
                 return child
             end
 
+            # Paths are special for files, because we don't actually want to show
+            # the parent's full path.
+            def path
+                unless defined? @path
+                    if defined? @parent
+                        # We only need to behave specially when our parent is also
+                        # a file
+                        if @parent.is_a?(self.class)
+                            # Remove the parent file name
+                            ppath = @parent.path.sub(/\/?file=.+/, '')
+                            @path = []
+                            if ppath != "/" and ppath != ""
+                                @path << ppath
+                            end
+                            @path << self.class.name.to_s + "=" + self.name
+                        else
+                            super
+                        end
+                    else
+                        # The top-level name is always puppet[top], so we don't
+                        # bother with that.  And we don't add the hostname
+                        # here, it gets added in the log server thingy.
+                        if self.name == "puppet[top]"
+                            @path = ["/"]
+                        else
+                            # We assume that if we don't have a parent that we
+                            # should not cache the path
+                            @path = [self.class.name.to_s + "=" + self.name]
+                        end
+                    end
+                end
+
+                return @path.join("/")
+            end
+
             # Recurse into the directory.  This basically just calls 'localrecurse'
             # and maybe 'sourcerecurse'.
             def recurse
@@ -355,16 +390,18 @@ module Puppet
                 if @parameters.include?(:ignore)
                     children = handleignore(children)
                 end  
-            
+
                 added = []
                 children.each { |file|
                     file = File.basename(file)
-                    next if file =~ /^\.\.?/ # skip . and .. 
+                    next if file =~ /^\.\.?$/ # skip . and .. 
                     if child = self.newchild(file, :recurse => recurse)
                         unless @children.include?(child)
                             self.push child
                             added.push file
+
                         end
+                        child.retrieve
                     end
                 }
             end
diff --git a/lib/puppet/type/pfile/mode.rb b/lib/puppet/type/pfile/mode.rb
index b49e3ff53..7ef04441d 100755
--- a/lib/puppet/type/pfile/mode.rb
+++ b/lib/puppet/type/pfile/mode.rb
@@ -10,20 +10,29 @@ module Puppet
             @name = :mode
             @event = :inode_changed
 
-            # Our modes are octal, so make sure they print correctly.
+            # Our modes are octal, so make sure they print correctly.  Other
+            # valid values are symbols, basically
             def is_to_s
-                if @is.is_a?(Integer)
+                case @is
+                when Integer
                     return "%o" % @is
-                else
+                when Symbol
                     return @is
+                else
+                    raise Puppet::DevError, "Invalid 'is' value for mode: %s" %
+                        @is.inspect
                 end
             end
 
             def should_to_s
-                if @should.is_a?(Integer)
-                    return "%o" % @should
+                case self.should
+                when Integer
+                    return "%o" % self.should
+                when Symbol
+                    return self.should
                 else
-                    return @should
+                    raise Puppet::DevError, "Invalid 'should' value for mode: %s" %
+                        self.should.inspect
                 end
             end
 
@@ -32,14 +41,20 @@ module Puppet
                 # octal, yet the number can only be specified as a string right now
                 value = should
                 if value.is_a?(String)
+                    unless value =~ /^[0-9]+$/
+                        raise Puppet::Error, "File modes can only be numbers"
+                    end
                     unless value =~ /^0/
                         value = "0" + value
                     end
-                    value = Integer(value)
+                    begin
+                        value = Integer(value)
+                    rescue ArgumentError => detail
+                        raise Puppet::DevError, "Could not convert %s to integer" %
+                            value.inspect
+                    end
                 end
 
-                #self.warning "Should is %o from %s" % [value, should]
-
                 return value
             end
 
diff --git a/lib/puppet/type/pfile/uid.rb b/lib/puppet/type/pfile/uid.rb
index 299d92756..ce91bdea6 100755
--- a/lib/puppet/type/pfile/uid.rb
+++ b/lib/puppet/type/pfile/uid.rb
@@ -10,6 +10,8 @@ module Puppet
             def id2name(id)
                 begin
                     user = Etc.getpwuid(id)
+                rescue TypeError
+                    return nil
                 rescue ArgumentError
                     return nil
                 end
@@ -60,7 +62,15 @@ module Puppet
             end
 
             def should_to_s
-                id2name(self.should) || self.should
+                case self.should
+                when Integer
+                    id2name(self.should) || self.should
+                when String
+                    self.should
+                else
+                    raise Puppet::DevError, "Invalid uid type %s(%s)" %
+                        [self.should.class, self.should]
+                end
             end
 
             def retrieve
diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index c30a4f4d1..05f127a9c 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -72,6 +72,18 @@ class State < Puppet::Element
         return false
     end
 
+    def log(msg)
+        unless @parent[:loglevel]
+            raise Puppet::DevError, "Parent %s has no loglevel" %
+                @parent.to_s
+        end
+        Puppet::Log.create(
+            :level => @parent[:loglevel],
+            :message => msg,
+            :source => self
+        )
+    end
+
     # each state class must define the name() method, and state instances
     # do not change that name
     # this implicitly means that a given object can only have one state
@@ -93,7 +105,7 @@ class State < Puppet::Element
     # return the full path to us, for logging and rollback; not currently
     # used
     def path
-        return [@parent.path, self.name].flatten
+        return [@parent.path, self.name].join("/")
     end
 
     # Only return the first value
@@ -130,16 +142,24 @@ class State < Puppet::Element
 
     # How should a state change be printed as a string?
     def change_to_s
+        begin
         if @is == :notfound
-            return "%s: defined '%s' as '%s'" %
-                [@parent, self.name, self.should_to_s]
+            return "defined '%s' as '%s'" %
+                [self.name, self.should_to_s]
         elsif self.should == :notfound
-            return "%s: undefined %s from '%s'" %
-                [self.parent, self.name, self.is_to_s]
+            return "undefined %s from '%s'" %
+                [self.name, self.is_to_s]
         else
+            self.should_to_s
             return "%s changed '%s' to '%s'" %
                 [self.name, self.is_to_s, self.should_to_s]
         end
+        rescue Puppet::Error, Puppet::DevError
+            raise
+        rescue => detail
+            raise Puppet::DevError, "Could not convert change to string: %s" %
+                change_to_s
+        end
     end
 
     # because the @should and @is vars might be in weird formats,
diff --git a/test/language/scope.rb b/test/language/scope.rb
index 3937ad361..67792daae 100755
--- a/test/language/scope.rb
+++ b/test/language/scope.rb
@@ -305,7 +305,7 @@ class TestScope < Test::Unit::TestCase
 
     # Verify that two statements about a file within the same scope tree
     # will cause a conflict.
-    def test_noconflicts
+    def test_znoconflicts
         filename = tempfile()
         children = []
 
@@ -357,7 +357,7 @@ class TestScope < Test::Unit::TestCase
     end
 
     # Verify that we override statements that we find within our scope
-    def test_zsuboverrides
+    def test_suboverrides
         filename = tempfile()
         children = []
 
diff --git a/test/language/snippets.rb b/test/language/snippets.rb
index a9471ffea..f7cf21548 100755
--- a/test/language/snippets.rb
+++ b/test/language/snippets.rb
@@ -425,6 +425,15 @@ class TestSnippets < Test::Unit::TestCase
                 assert_nothing_raised {
                     trans = client.apply()
                 }
+
+                Puppet::Type.eachtype { |type|
+                    type.each { |obj|
+                        unless obj.name == "puppet[top]"
+                            assert(obj.parent, "%s has no parent" % obj.name)
+                        end
+                        assert(obj.name)
+                    }
+                }
                 assert_nothing_raised {
                     self.send(mname, trans)
                 }
diff --git a/test/other/log.rb b/test/other/log.rb
index 0fd72887a..764a3c83f 100644
--- a/test/other/log.rb
+++ b/test/other/log.rb
@@ -153,4 +153,43 @@ class TestLog < Test::Unit::TestCase
             )
         }
     end
+
+    # Verify that the error and source are always strings
+    def test_argsAreStrings
+        msg = nil
+        file = Puppet::Type::PFile.create(
+            :path => tempfile(),
+            :check => %w{owner group}
+        )
+        assert_nothing_raised {
+            msg = Puppet::Log.new(:level => :info, :message => "This is a message")
+        }
+        assert_nothing_raised {
+            msg.source = file
+        }
+
+        assert_instance_of(String, msg.to_s)
+        assert_instance_of(String, msg.source)
+    end
+
+    # Verify that loglevel behaves as one expects
+    def test_loglevel
+        path = tempfile()
+        file = Puppet::Type::PFile.create(
+            :path => path,
+            :create => true
+        )
+
+        assert_nothing_raised {
+            assert_equal(:notice, file[:loglevel])
+        }
+
+        assert_nothing_raised {
+            file[:loglevel] = "warning"
+        }
+
+        assert_nothing_raised {
+            assert_equal(:warning, file[:loglevel])
+        }
+    end
 end
diff --git a/test/puppet/error.rb b/test/puppet/error.rb
new file mode 100755
index 000000000..f27d79822
--- /dev/null
+++ b/test/puppet/error.rb
@@ -0,0 +1,23 @@
+if __FILE__ == $0
+    $:.unshift '..'
+    $:.unshift '../../lib'
+    $puppetbase = "../.."
+end
+
+require 'puppet'
+require 'puppettest'
+require 'test/unit'
+
+class TestError < Test::Unit::TestCase
+    include TestPuppet
+
+    def test_errorisstring
+        error = nil
+        assert_nothing_raised {
+            error = Puppet::ParseError.new("This is an error")
+        }
+        assert_instance_of(String, error.to_s)
+    end
+end
+
+# $Id$
diff --git a/test/puppettest.rb b/test/puppettest.rb
index d38de89a2..f45356740 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -578,6 +578,8 @@ module ParserTesting
         end
         unless args.include?(:code)
             args[:code] = AST::ASTArray.new(
+                :file => tempfile(),
+                :line => rand(100),
                 :children => [
                     varobj("%svar" % name, "%svalue" % name),
                     fileobj("/%s" % name)
@@ -590,8 +592,12 @@ module ParserTesting
     end
 
     def compobj(name, args = {})
+        args[:file] = tempfile()
+        args[:line] = rand(100)
         args[:name] = nameobj(name)
         args[:code] = AST::ASTArray.new(
+            :file => tempfile(),
+            :line => rand(100),
             :children => [
                 varobj("%svar" % name, "%svalue" % name),
                 fileobj("/%s" % name)
@@ -605,6 +611,8 @@ module ParserTesting
     def fileobj(path, hash = {"owner" => "root"})
         assert_nothing_raised("Could not create file %s" % path) {
             return AST::ObjectDef.new(
+                :file => tempfile(),
+                :line => rand(100),
                 :name => stringobj(path),
                 :type => nameobj("file"),
                 :params => objectinst(hash)
@@ -615,6 +623,8 @@ module ParserTesting
     def nameobj(name)
         assert_nothing_raised("Could not create name %s" % name) {
             return AST::Name.new(
+                :file => tempfile(),
+                :line => rand(100),
                 :value => name
             )
         }
@@ -623,6 +633,8 @@ module ParserTesting
     def nodeobj(name)
         assert_nothing_raised("Could not create node %s" % name) {
             return AST::NodeDef.new(
+                :file => tempfile(),
+                :line => rand(100),
                 :names => nameobj(name),
                 :code => AST::ASTArray.new(
                     :children => [
@@ -640,6 +652,8 @@ module ParserTesting
                 objectparam(param, value)
             }
             return AST::ObjectInst.new(
+                :file => tempfile(),
+                :line => rand(100),
                 :children => params
             )
         }
@@ -648,6 +662,8 @@ module ParserTesting
     def objectparam(param, value)
         assert_nothing_raised("Could not create param %s" % param) {
             return AST::ObjectParam.new(
+                :file => tempfile(),
+                :line => rand(100),
                 :param => nameobj(param),
                 :value => stringobj(value)
             )
@@ -655,12 +671,18 @@ module ParserTesting
     end
 
     def stringobj(value)
-        AST::String.new(:value => value)
+        AST::String.new(
+            :file => tempfile(),
+            :line => rand(100),
+            :value => value
+        )
     end
 
     def varobj(name, value)
         assert_nothing_raised("Could not create %s code" % name) {
             return AST::VarDef.new(
+                :file => tempfile(),
+                :line => rand(100),
                 :name => nameobj(name),
                 :value => stringobj(value)
             )
diff --git a/test/types/file.rb b/test/types/file.rb
index b160ae1da..420c0bbcf 100644
--- a/test/types/file.rb
+++ b/test/types/file.rb
@@ -116,7 +116,7 @@ class TestFile < Test::Unit::TestCase
     end
 
     if Process.uid == 0
-        def test_zcreateasuser
+        def test_createasuser
             dir = tmpdir()
 
             user = nonrootuser()
@@ -431,19 +431,42 @@ class TestFile < Test::Unit::TestCase
     end
 
     def test_recursion
-        path = "/tmp/filerecursetest"
-        @@tmpfiles.push path
-        tmpfile = File.join(path,"testing")
-        system("mkdir -p #{path}")
-        cyclefile(path)
-        File.open(tmpfile, File::WRONLY|File::CREAT|File::APPEND) { |of|
-            of.puts "yayness"
-        }
-        cyclefile(path)
-        File.open(tmpfile, File::WRONLY|File::APPEND) { |of|
-            of.puts "goodness"
-        }
-        cyclefile(path)
+        basedir = tempfile()
+        subdir = File.join(basedir, "this", "is", "sub", "dir")
+        tmpfile = File.join(subdir,"testing")
+        FileUtils.mkdir_p(subdir)
+
+        dir = nil
+        assert_nothing_raised {
+            dir = Puppet::Type::PFile.create(
+                :path => basedir,
+                :recurse => true,
+                :check => %w{owner mode group}
+            )
+        }
+
+        assert_nothing_raised {
+            dir.retrieve
+        }
+
+        subobj = nil
+        assert_nothing_raised {
+            subobj = Puppet::Type::PFile[subdir]
+        }
+
+        assert(subobj, "Could not retrieve subdir object")
+
+        File.open(tmpfile, "w") { |f| f.puts "yayness" }
+
+        dir.retrieve
+
+        file = nil
+        assert_nothing_raised {
+            file = Puppet::Type::PFile[tmpfile]
+        }
+
+        assert(file, "Could not retrieve file object")
+
     end
 
 =begin
@@ -519,6 +542,75 @@ class TestFile < Test::Unit::TestCase
             file.sync
         }
     end
+
+    def test_remove
+        basedir = tempfile()
+        subdir = File.join(basedir, "this")
+        FileUtils.mkdir_p(subdir)
+
+        dir = nil
+        assert_nothing_raised {
+            dir = Puppet::Type::PFile.create(
+                :path => basedir,
+                :recurse => true,
+                :check => %w{owner mode group}
+            )
+        }
+
+        assert_nothing_raised {
+            dir.retrieve
+        }
+
+        obj = nil
+        assert_nothing_raised {
+            obj = Puppet::Type::PFile[subdir]
+        }
+
+        assert(obj, "Could not retrieve subdir object")
+
+        assert_nothing_raised {
+            obj.remove(true)
+        }
+
+        assert_nothing_raised {
+            obj = Puppet::Type::PFile[subdir]
+        }
+
+        assert_nil(obj, "Retrieved removed object")
+    end
+
+    def test_zpath
+        dir = tempfile()
+
+        path = File.join(dir, "and", "a", "sub", "dir")
+
+        assert_nothing_raised("Could not make file") {
+            FileUtils.mkdir_p(File.dirname(path))
+            File.open(path, "w") { |f| f.puts "yayness" }
+        }
+
+        file = nil
+        dirobj = nil
+        assert_nothing_raised("Could not make file object") {
+            dirobj = Puppet::Type::PFile.create(
+                :path => dir,
+                :recurse => true,
+                :check => %w{mode owner group}
+            )
+        }
+
+        assert_nothing_raised {
+            dirobj.retrieve
+        }
+
+        assert_nothing_raised {
+            file = dirobj.class[path]
+        }
+
+        assert(file, "Could not retrieve file object")
+
+        assert_equal("file=%s" % file.name, file.path)
+    end
 end
 
 # $Id$