Converting transport format to YAML instead of Marshal, and caching the file in a YAML format, also. This required a significant rework of both Transportable classes. Lastly, I am also now caching the list of classes in a class file in /etc/puppet.

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

21 files changed, 347 insertions, 105 deletions

diff --git a/lib/puppet.rb b/lib/puppet.rb
index 22f7c73d0..f0801968c 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -80,7 +80,8 @@ PUPPETVERSION = '0.10.2'
         # then the files},
         :manifestdir    => [:puppetconf,     "manifests"],
         :manifest       => [:manifestdir,    "site.pp"],
-        :localconfig    => [:puppetconf,     "localconfig.ma"],
+        :localconfig    => [:puppetconf,     "localconfig"],
+        :classfile      => [:puppetconf,     "classes.txt"],
         :logfile        => [:logdir,         "puppet.log"],
         :httplogfile    => [:logdir,         "http.log"],
         :masterlog      => [:logdir,         "puppetmaster.log"],
diff --git a/lib/puppet/client.rb b/lib/puppet/client.rb
index 15c2acdd7..a0fb994d8 100644
--- a/lib/puppet/client.rb
+++ b/lib/puppet/client.rb
@@ -18,6 +18,7 @@ begin
     require 'cgi'
     require 'xmlrpc/client'
     require 'xmlrpc/server'
+    require 'yaml'
 rescue LoadError => detail
     $noclientnetworking = detail
     raise Puppet::Error, "You must have the Ruby XMLRPC, CGI, and Webrick libraries installed"
@@ -257,25 +258,18 @@ module Puppet
                     end
                 end
 
-                # FIXME this should be in getconfig, not apply
-                container = @objects.to_type
-
-                # Now perform any necessary final actions before we evaluate.
-                Puppet::Type.finalize
-                #if @local
-                #    container = @objects.to_type
-                #else
-                #    container = Marshal::load(@objects).to_type
-                #end
-
+                #Puppet.err :yay
+                #p @objects
+                #Puppet.err :mark
+                #@objects = @objects.to_type
                 # this is a gross hack... but i don't see a good way around it
                 # set all of the variables to empty
                 Puppet::Transaction.init
 
-                # for now we just evaluate the top-level container, but eventually
+                # For now we just evaluate the top-level object, but eventually
                 # there will be schedules and such associated with each object,
-                # and probably with the container itself
-                transaction = container.evaluate
+                # and probably with the container itself.
+                transaction = @objects.evaluate
                 #transaction = Puppet::Transaction.new(objects)
                 transaction.toplevel = true
                 begin
@@ -297,8 +291,9 @@ module Puppet
                 return transaction
             end
 
+            # Retrieve the config from a remote server.  If this fails, then
+            # use the cached copy.
             def getconfig
-                #client.loadproperty('files/sslclient.properties')
                 Puppet.debug("getting config")
 
                 facts = self.class.facts
@@ -311,16 +306,16 @@ module Puppet
 
                 objects = nil
                 if @local
-                    objects = @driver.getconfig(facts)
+                    objects = @driver.getconfig(facts, "yaml")
 
                     if objects == ""
                         raise Puppet::Error, "Could not retrieve configuration"
                     end
                 else
-                    textfacts = CGI.escape(Marshal::dump(facts))
+                    textfacts = CGI.escape(YAML.dump(facts))
 
                     # error handling for this is done in the network client
-                    textobjects = @driver.getconfig(textfacts)
+                    textobjects = @driver.getconfig(textfacts, "yaml")
 
                     unless textobjects == ""
                         begin
@@ -330,23 +325,26 @@ module Puppet
                         end
                     end
 
+                    cachefile = Puppet[:localconfig] + ".yaml"
                     if @cache
                         if textobjects == ""
-                            if FileTest.exists?(Puppet[:localconfig])
-                                textobjects = File.read(Puppet[:localconfig])
+                            if FileTest.exists?(cachefile)
+                                textobjects = File.read(cachefile)
                             else
                                 raise Puppet::Error.new(
                                     "Cannot connect to server and there is no cached configuration"
                                 )
                             end
                         else
-                            # we store the config so that if we can't connect next time, we
-                            # can just run against the most recently acquired copy
+                            # We store the config so that if we can't connect
+                            # next time, we can just run against the most
+                            # recently acquired copy.
+                            Puppet.info "Caching configuration at %s" % cachefile
                             confdir = File.dirname(Puppet[:localconfig])
                             unless FileTest.exists?(confdir)
                                 Puppet.recmkdir(confdir, 0770)
                             end
-                            File.open(Puppet[:localconfig], "w", 0660) { |f|
+                            File.open(cachefile, "w", 0660) { |f|
                                 f.print textobjects
                             }
                         end
@@ -355,17 +353,54 @@ module Puppet
                     end
 
                     begin
-                        objects = Marshal::load(textobjects)
+                        objects = YAML.load(textobjects)
                     rescue => detail
-                        raise Puppet::Error.new("Could not understand configuration")
+                        raise Puppet::Error,
+                            "Could not understand configuration: %s" %
+                            detail.to_s
                     end
                 end
-                if objects.is_a?(Puppet::TransBucket)
-                    @objects = objects
-                else
+
+                unless objects.is_a?(Puppet::TransBucket)
                     raise NetworkClientError,
                         "Invalid returned objects of type %s" % objects.class
                 end
+
+                if classes = objects.classes
+                    self.setclasses(classes)
+                else
+                    Puppet.info "No classes"
+                end
+
+                # Clear all existing objects, so we can recreate our stack.
+                @objects = nil
+                if defined? @objects
+                    Puppet::Type.allclear
+                end
+
+                # Now convert the objects to real Puppet objects
+                @objects = objects.to_type
+
+                if @objects.nil?
+                    raise Puppet::Error, "Configuration could not be processed"
+                end
+                #@objects = objects
+
+                # and perform any necessary final actions before we evaluate.
+                Puppet::Type.finalize
+
+                return @objects
+            end
+
+            def setclasses(ary)
+                begin
+                    File.open(Puppet[:classfile], "w") { |f|
+                        f.puts ary.join("\n")
+                    }
+                rescue => detail
+                    Puppet.err "Could not create class file %s: %s" %
+                        [Puppet[:classfile], detail]
+                end
             end
         end
 
diff --git a/lib/puppet/log.rb b/lib/puppet/log.rb
index 7d4b750c7..6b7aa506d 100644
--- a/lib/puppet/log.rb
+++ b/lib/puppet/log.rb
@@ -216,9 +216,9 @@ module Puppet # :nodoc:
                         begin
                             #puts "would have sent %s" % msg
                             #puts "would have sent %s" %
-                            #    CGI.escape(Marshal::dump(msg))
+                            #    CGI.escape(YAML.dump(msg))
                             begin
-                                tmp = CGI.escape(Marshal::dump(msg))
+                                tmp = CGI.escape(YAML.dump(msg))
                             rescue => detail
                                 puts "Could not dump: %s" % detail.to_s
                                 return
diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index 9d6a8504c..bb15b3320 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -150,7 +150,11 @@ module Puppet
             if self.is_a?(Puppet::State)
                 return self.should
             else
-                return @value
+                if defined? @value
+                    return @value
+                else
+                    return nil
+                end
             end
         end
 
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index 344863ec0..96555faa1 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -61,6 +61,9 @@ module Puppet
 
                         # We've already evaluated the AST, in this case
                         retval = @scope.evalnode(names, facts)
+                        if classes = @scope.classlist
+                            retval.classes = classes
+                        end
                         return retval
                     else
                         # We've already evaluated the AST, in this case
@@ -68,7 +71,11 @@ module Puppet
                         @scope.interp = self
                         @scope.type = "puppet"
                         @scope.name = "top"
-                        return @scope.evaluate(@ast, facts, @classes)
+                        retval = @scope.evaluate(@ast, facts, @classes)
+                        if classes = @scope.classlist
+                            retval.classes = classes + @classes
+                        end
+                        return retval
                     end
                     #@ast.evaluate(@scope)
                 rescue Puppet::DevError, Puppet::Error, Puppet::ParseError => except
diff --git a/lib/puppet/parser/scope.rb b/lib/puppet/parser/scope.rb
index a2d05e7d4..598c67b5e 100644
--- a/lib/puppet/parser/scope.rb
+++ b/lib/puppet/parser/scope.rb
@@ -149,6 +149,14 @@ module Puppet
                 @level == 1
             end
 
+            # Return a list of all of the defined classes.
+            def classlist
+                unless defined? @classtable
+                    raise Puppet::DevError, "Scope did not receive class table"
+                end
+                return @classtable.keys
+            end
+
             # Yield each child scope in turn
             def each
                 @children.reject { |child|
@@ -714,7 +722,7 @@ module Puppet
 
                     # it'd be nice not to have to do this...
                     results.each { |result|
-                        #Puppet.debug "Result type is %s" % result.class
+                        #Puppet.warning "Result type is %s" % result.class
                         bucket.push(result)
                     }
                     if defined? @type
diff --git a/lib/puppet/server/logger.rb b/lib/puppet/server/logger.rb
index d5feb0320..ae57370d8 100755
--- a/lib/puppet/server/logger.rb
+++ b/lib/puppet/server/logger.rb
@@ -1,3 +1,5 @@
+require 'yaml'
+
 module Puppet
 class Server # :nodoc:
     class LoggerError < RuntimeError; end
@@ -18,11 +20,11 @@ class Server # :nodoc:
             # if the client is set, then we're not local
             if client
                 begin
-                    message = Marshal::load(CGI.unescape(message))
+                    message = YAML.load(CGI.unescape(message))
                     #message = message
                 rescue => detail
                     raise XMLRPC::FaultException.new(
-                        1, "Could not unMarshal log message from %s" % client
+                        1, "Could not unYAML log message from %s" % client
                     )
                 end
             end
diff --git a/lib/puppet/server/master.rb b/lib/puppet/server/master.rb
index e86663d0f..8d4ddbfde 100644
--- a/lib/puppet/server/master.rb
+++ b/lib/puppet/server/master.rb
@@ -3,6 +3,7 @@ require 'puppet'
 require 'puppet/parser/interpreter'
 require 'puppet/sslcertificates'
 require 'xmlrpc/server'
+require 'yaml'
 
 module Puppet
 class Server
@@ -57,7 +58,7 @@ class Server
             end
         end
 
-        def getconfig(facts, client = nil, clientip = nil)
+        def getconfig(facts, format = "marshal", client = nil, clientip = nil)
             if @local
                 # we don't need to do anything, since we should already
                 # have raw objects
@@ -66,11 +67,26 @@ class Server
                 Puppet.debug "Our client is remote"
 
                 # XXX this should definitely be done in the protocol, somehow
-                begin
-                    facts = Marshal::load(CGI.unescape(facts))
-                rescue => detail
+                case format
+                when "marshal":
+                    begin
+                        facts = Marshal::load(CGI.unescape(facts))
+                    rescue => detail
+                        raise XMLRPC::FaultException.new(
+                            1, "Could not rebuild facts"
+                        )
+                    end
+                when "yaml":
+                    begin
+                        facts = YAML.load(CGI.unescape(facts))
+                    rescue => detail
+                        raise XMLRPC::FaultException.new(
+                            1, "Could not rebuild facts"
+                        )
+                    end
+                else
                     raise XMLRPC::FaultException.new(
-                        1, "Could not rebuild facts"
+                        1, "Unavailable config format %s" % format
                     )
                 end
             end
@@ -95,7 +111,18 @@ class Server
             if @local
                 return retobjects
             else
-                return CGI.escape(Marshal::dump(retobjects))
+                str = nil
+                case format
+                when "marshal":
+                    str = Marshal::dump(retobjects)
+                when "yaml":
+                    str = YAML.dump(retobjects)
+                else
+                    raise XMLRPC::FaultException.new(
+                        1, "Unavailable config format %s" % format
+                    )
+                end
+                return CGI.escape(str)
             end
         end
     end
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index e81401c41..129e80bac 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -1,17 +1,32 @@
 require 'puppet'
+require 'yaml'
 
 module Puppet
     # The transportable objects themselves.  Basically just a hash with some
-    # metadata and a few extra methods.
-    class TransObject < Hash
+    # metadata and a few extra methods.  I used to have the object actually
+    # be a subclass of Hash, but I could never correctly dump them using
+    # YAML.
+    class TransObject
+        include Enumerable
         attr_accessor :type, :name, :file, :line
 
         attr_writer :tags
 
+        %w{has_key? include? length delete empty? << [] []=}.each { |method|
+            define_method(method) do |*args|
+                @params.send(method, *args)
+            end
+        }
+
+        def each
+            @params.each { |p,v| yield p, v }
+        end
+
         def initialize(name,type)
-            self[:name] = name
             @type = type
             @name = name
+            @params = {"name" => name}
+            #Puppet.info @params.inspect
             #self.class.add(self)
         end
 
@@ -23,10 +38,20 @@ module Puppet
             return @tags
         end
 
+        def to_hash
+            @params.dup
+        end
+
         def to_s
             return "%s(%s) => %s" % [@type,self[:name],super]
         end
 
+        def to_yaml_properties
+            instance_variables
+            #%w{ @type @name @file @line @tags }.find_all { |v|
+            #}
+        end
+
         def to_type(parent = nil)
             if parent
                 self[:parent] = parent
@@ -34,34 +59,55 @@ module Puppet
             retobj = nil
             if type = Puppet::Type.type(self.type)
                 unless retobj = type.create(self)
+                    Puppet.info "Could not create object"
                     return nil
                 end
-                retobj.file = @file
-                retobj.line = @line
+                #retobj.file = @file
+                #retobj.line = @line
             else
                 raise Puppet::Error.new("Could not find object type %s" % self.type)
             end
 
-            if defined? @tags and @tags
-                #Puppet.debug "%s(%s) tags: %s" % [@type, @name, @tags.join(" ")]
-                retobj.tags = @tags
-            end
+            #if defined? @tags and @tags
+            #    #Puppet.debug "%s(%s) tags: %s" % [@type, @name, @tags.join(" ")]
+            #    retobj.tags = @tags
+            #end
 
             if parent
                 parent.push retobj
             end
 
+            Puppet.info retobj.class
+
             return retobj
         end
     end
-    #------------------------------------------------------------
 
-    #------------------------------------------------------------
-    # just a linear container for objects
-    class TransBucket < Array
-        attr_accessor :name, :type, :file, :line
+    # Just a linear container for objects.  Behaves mostly like an array, except
+    # that YAML will correctly dump them even with their instance variables.
+    class TransBucket
+        include Enumerable
+
+        attr_accessor :name, :type, :file, :line, :classes
+
+        %w{delete shift include? length empty? << []}.each { |method|
+            define_method(method) do |*args|
+                #Puppet.warning "Calling %s with %s" % [method, args.inspect]
+                @children.send(method, *args)
+                #Puppet.warning @params.inspect
+            end
+        }
+
+        def each
+            @children.each { |c| yield c }
+        end
+
+        def initialize
+            @children = []
+        end
 
         def push(*args)
+            #Puppet.warning "calling push"
             args.each { |arg|
                 case arg
                 when Puppet::TransBucket, Puppet::TransObject
@@ -72,7 +118,12 @@ module Puppet
                             arg.class
                 end
             }
-            super
+            @children += args
+            #Puppet.warning @children.inspect
+        end
+
+        def to_yaml_properties
+            instance_variables
         end
 
         def to_type(parent = nil)
@@ -103,6 +154,7 @@ module Puppet
                 hash[:parent] = parent
             end
             container = Puppet.type(:component).create(hash)
+            #Puppet.info container.inspect
 
             if parent
                 parent.push container
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index dfef5d07a..2623fd6c5 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -95,6 +95,19 @@ class Type < Puppet::Element
         end
     end
 
+    def inspect
+        str = "Type(%s)" % self.name
+        if defined? @states
+            str += " States(" + @states.inspect + ")"
+        end
+        if defined? @parameters
+            str += " Parameters(" + @parameters.inspect + ")"
+        end
+        if defined? @metaparams
+            str += " Metaparams(" + @metaparams.inspect + ")"
+        end
+    end
+
     # Create @@typehash from @@typeary.  This is meant to be run
     # multiple times -- whenever it is discovered that the two
     # objects have differents lengths.
@@ -531,7 +544,8 @@ class Type < Puppet::Element
         when @@metaparamhash.include?(name): return :meta
         when @validstates.include?(name): return :state
         else
-            raise Puppet::DevError, "Invalid parameter %s" % [name]
+            raise Puppet::DevError, "Invalid attribute '%s' for class '%s'" %
+                [name, self.name]
         end
     end
 
@@ -1042,6 +1056,21 @@ class Type < Puppet::Element
             self.initvars
         end
 
+        # If we got passed a transportable object, we just pull a bunch of info
+        # directly from it.
+        if hash.is_a?(Puppet::TransObject)
+            #self[:name] = hash[:name]
+            [:file, :line, :tags].each { |getter|
+                if hash.respond_to?(getter)
+                    setter = getter.to_s + "="
+                    if val = hash.send(getter)
+                        self.send(setter, val)
+                    end
+                end
+            }
+            hash = hash.to_hash
+        end
+
         # Before anything else, set our parent if it was included
         if hash.include?(:parent)
             @parent = hash[:parent]
@@ -1217,9 +1246,11 @@ class Type < Puppet::Element
 
     # fix any namevar => param translations
     def argclean(hash)
-        # 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
+        # We have to set the name of our object before anything else,
+        # because it might be used in creating the other states.  We dup and
+        # then convert to a hash here because TransObjects behave strangely
+        # here.
+        hash = hash.dup.to_hash
 
         if hash.include?(:parent)
             hash.delete(:parent)
diff --git a/lib/puppet/type/tidy.rb b/lib/puppet/type/tidy.rb
index 59d51e680..1603dba34 100755
--- a/lib/puppet/type/tidy.rb
+++ b/lib/puppet/type/tidy.rb
@@ -151,6 +151,10 @@ module Puppet
             end
         end
 
+        # Erase PFile's validate method
+        validate do
+        end
+
         @depthfirst = true
 
         def initialize(hash)
diff --git a/test/executables/puppetmasterd.rb b/test/executables/puppetmasterd.rb
index 389141ed0..e1f48c11b 100755
--- a/test/executables/puppetmasterd.rb
+++ b/test/executables/puppetmasterd.rb
@@ -80,8 +80,6 @@ class TestPuppetMasterD < Test::Unit::TestCase
         }
 
         objects = nil
-        assert_instance_of(Puppet::TransBucket, retval,
-            "Retrieved non-transportable object")
         stopmasterd
         sleep(1)
     end
@@ -135,16 +133,16 @@ class TestPuppetMasterD < Test::Unit::TestCase
         Facter.each { |p,v|
             facts[p] = v
         }
-        textfacts = CGI.escape(Marshal::dump(facts))
+        textfacts = CGI.escape(YAML.dump(facts))
         assert_nothing_raised() {
             #Puppet.notice "calling status"
             #retval = client.call("status.status", "")
-            retval = client.call("puppetmaster.getconfig", textfacts)
+            retval = client.call("puppetmaster.getconfig", textfacts, "yaml")
         }
 
         objects = nil
         assert_nothing_raised {
-            Marshal::load(CGI.unescape(retval))
+            YAML.load(CGI.unescape(retval))
         }
         #stopmasterd
     end
diff --git a/test/language/ast.rb b/test/language/ast.rb
index 6aa89c225..3dc5fd582 100755
--- a/test/language/ast.rb
+++ b/test/language/ast.rb
@@ -289,11 +289,11 @@ class TestAST < Test::Unit::TestCase
 
         # And now verify that we got both the top and node objects
         assert_nothing_raised("Could not find top-declared object") {
-            assert_equal("/testing", objects[0][:name])
+            assert_equal("/testing", objects[0]["name"])
         }
 
         assert_nothing_raised("Could not find node-declared object") {
-            assert_equal("/%s" % name, objects[1][0][:name])
+            assert_equal("/%s" % name, objects[1][0]["name"])
         }
     end
 
@@ -334,7 +334,7 @@ class TestAST < Test::Unit::TestCase
         assert(objects, "Could not retrieve objects")
 
         assert_nothing_raised("Could not find top-declared object") {
-            assert_equal("/%s" % klassname, objects[0][0][:name])
+            assert_equal("/%s" % klassname, objects[0][0]["name"])
         }
     end
 
@@ -391,19 +391,19 @@ class TestAST < Test::Unit::TestCase
             # And now verify that we got the subnode file
             assert_nothing_raised("Could not find basenode file") {
                 base = inner[0]
-                assert_equal("/basenode", base[:name])
+                assert_equal("/basenode", base["name"])
             }
 
             # and the parent node file
             assert_nothing_raised("Could not find subnode file") {
                 sub = inner[1]
-                assert_equal("/subnode", sub[:name])
+                assert_equal("/subnode", sub["name"])
             }
 
             inner.each { |obj|
                 %w{basenode subnode}.each { |tag|
                     assert(obj.tags.include?(tag),
-                        "%s did not include %s tag" % [obj[:name], tag]
+                        "%s did not include %s tag" % [obj["name"], tag]
                     )
                 }
             }
diff --git a/test/language/snippets.rb b/test/language/snippets.rb
index 2dc671be1..7ca896ef4 100755
--- a/test/language/snippets.rb
+++ b/test/language/snippets.rb
@@ -196,8 +196,9 @@ class TestSnippets < Test::Unit::TestCase
         @@tmpfiles += files
 
         files.each { |file|
-            assert(FileTest.exists?(file))
-            assert(File.stat(file).mode & 007777 == 0755)
+            assert(FileTest.exists?(file), "File %s does not exist" % file)
+            assert(File.stat(file).mode & 007777 == 0755,
+                "File %s is the incorrect mode" % file)
             @@tmpfiles << file
         }
 
diff --git a/test/language/transportable.rb b/test/language/transportable.rb
new file mode 100755
index 000000000..0e04774a5
--- /dev/null
+++ b/test/language/transportable.rb
@@ -0,0 +1,77 @@
+#!/usr/bin/ruby
+
+if __FILE__ == $0
+    $:.unshift '../../lib'
+    $:.unshift '..'
+    $puppetbase = "../.."
+end
+
+require 'puppet'
+require 'puppet/transportable'
+require 'test/unit'
+require 'puppettest'
+require 'yaml'
+
+class TestTransportable < Test::Unit::TestCase
+	include TestPuppet
+
+    def mkobj(file = "/etc/passwd")
+        obj = nil
+        assert_nothing_raised {
+            obj = Puppet::TransObject.new("file", file)
+            obj["owner"] = "root"
+            obj["mode"] = "644"
+        }
+
+        return obj
+    end
+
+    def mkbucket(*objects)
+        bucket = nil
+        assert_nothing_raised {
+            bucket = Puppet::TransBucket.new
+            bucket.name = "yayname"
+            bucket.type = "yaytype"
+        }
+
+        objects.each { |o| bucket << o }
+
+        return bucket
+    end
+
+    def test_yamldumpobject
+        obj = mkobj
+        obj.to_yaml_properties
+        str = nil
+        assert_nothing_raised {
+            str = YAML.dump(obj)
+        }
+
+        newobj = nil
+        assert_nothing_raised {
+            newobj = YAML.load(str)
+        }
+
+        assert(newobj.name, "Object has no name")
+        assert(newobj.type, "Object has no type")
+    end
+
+    def test_yamldumpbucket
+        objects = %w{/etc/passwd /etc /tmp /var /dev}.collect { |d|
+            mkobj(d)
+        }
+        bucket = mkbucket(*objects)
+        str = nil
+        assert_nothing_raised {
+            str = YAML.dump(bucket)
+        }
+
+        newobj = nil
+        assert_nothing_raised {
+            newobj = YAML.load(str)
+        }
+
+        assert(newobj.name, "Bucket has no name")
+        assert(newobj.type, "Bucket has no type")
+    end
+end
diff --git a/test/other/metrics.rb b/test/other/metrics.rb
index 36a12eb1f..32d73cf06 100644
--- a/test/other/metrics.rb
+++ b/test/other/metrics.rb
@@ -18,6 +18,7 @@ end
 
 if $haverrd
     class TestMetric < Test::Unit::TestCase
+        include TestPuppet
 
         def gendata
             totalmax = 1000
@@ -52,13 +53,8 @@ if $haverrd
         end
 
         def setup
-            Puppet[:rrddir] = File.join(Puppet[:puppetvar], "rrdtesting")
+            super
             Puppet[:rrdgraph] = true
-            Puppet[:loglevel] = :debug if __FILE__ == $0
-        end
-
-        def teardown
-            system("rm -rf %s" % Puppet[:rrddir])
         end
 
         def test_fakedata
diff --git a/test/puppet/defaults.rb b/test/puppet/defaults.rb
index e1d0124d2..9c16578e7 100755
--- a/test/puppet/defaults.rb
+++ b/test/puppet/defaults.rb
@@ -5,11 +5,13 @@ if __FILE__ == $0
 end
 
 require 'puppet'
+require 'puppettest'
 require 'test/unit'
 
 # $Id$
 
 class TestPuppetDefaults < Test::Unit::TestCase
+    include TestPuppet
     @@dirs = %w{rrddir puppetconf puppetvar logdir statedir}
     @@files = %w{logfile checksumfile manifest masterlog}
     @@normals = %w{puppetport masterport server}
@@ -77,7 +79,7 @@ class TestPuppetDefaults < Test::Unit::TestCase
             value = Puppet[param]
 
             unless value !~ notval
-                assert_nothing_raised { raise "%s is in wrong dir: %s" %
+                assert_nothing_raised { raise "%s is incorrectly set to %s" %
                     [param,value] }
             end
         }
diff --git a/test/puppet/utiltest.rb b/test/puppet/utiltest.rb
index a9fd2f719..161a831d8 100755
--- a/test/puppet/utiltest.rb
+++ b/test/puppet/utiltest.rb
@@ -12,6 +12,8 @@ class TestPuppetUtil < Test::Unit::TestCase
     include TestPuppet
     unless Process.uid == 0
         $stderr.puts "Run as root to perform Utility tests"
+        def test_nothing
+        end
     else
 
     def mknverify(file, user, group = nil, id = false)
diff --git a/test/puppettest.rb b/test/puppettest.rb
index 18426aa2f..9309a54b9 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -59,21 +59,21 @@ module TestPuppet
     def spin
         # Just disable spin, unless we really need it
         return
-        if Puppet[:debug]
-            return
-        end
-        @modes = %w{| / - \\}
-        unless defined? @mode
-            @mode = 0
-        end
-
-        $stderr.print "%s" % @modes[@mode]
-        if @mode == @modes.length - 1
-            @mode = 0
-        else
-            @mode += 1
-        end
-        $stderr.flush
+#        if Puppet[:debug]
+#            return
+#        end
+#        @modes = %w{| / - \\}
+#        unless defined? @mode
+#            @mode = 0
+#        end
+#
+#        $stderr.print "%s" % @modes[@mode]
+#        if @mode == @modes.length - 1
+#            @mode = 0
+#        else
+#            @mode += 1
+#        end
+#        $stderr.flush
     end
 
     # stop any services that might be hanging around
diff --git a/test/server/logger.rb b/test/server/logger.rb
index dfaf50438..7872ed172 100644
--- a/test/server/logger.rb
+++ b/test/server/logger.rb
@@ -55,7 +55,7 @@ class TestLogger < Test::Unit::TestCase
         }
 
         assert_nothing_raised {
-            msg = CGI.escape(Marshal::dump(msg))
+            msg = CGI.escape(YAML.dump(msg))
         }
         assert_nothing_raised {
             logger.addlog(msg, "localhost", "127.0.0.1")
@@ -77,7 +77,7 @@ class TestLogger < Test::Unit::TestCase
             )
         }
 
-        msg = CGI.escape(Marshal::dump(msg))
+        msg = CGI.escape(YAML.dump(msg))
 
         assert_nothing_raised {
             client.addlog(msg, "localhost", "127.0.0.1")
@@ -140,7 +140,7 @@ class TestLogger < Test::Unit::TestCase
             :warning => "XMLRPC2",
             :err => "XMLRPC3"
         }.each { |level, str|
-            msg = CGI.escape(Marshal::dump(Puppet::Log.create(
+            msg = CGI.escape(YAML.dump(Puppet::Log.create(
                 :level => level,
                 :message => str
             )))
diff --git a/test/server/server.rb b/test/server/server.rb
index be9923b40..89ba01b4d 100644
--- a/test/server/server.rb
+++ b/test/server/server.rb
@@ -23,10 +23,6 @@ end
 
 class TestServer < Test::Unit::TestCase
 	include ServerTest
-    def teardown
-        super
-        #print "\n\n\n\n" if Puppet[:debug]
-    end
 
     # test that we can connect to the server
     # we have to use fork here, because we apparently can't use threads
@@ -120,9 +116,8 @@ class TestServer < Test::Unit::TestCase
         assert_nothing_raised() {
             retval = client.getconfig
         }
-        assert_instance_of(Puppet::TransBucket, retval,
-            "Server returned something other than a TransBucket")
 
+        # Try it again, just for kicks
         assert_nothing_raised() {
             retval = client.getconfig
         }