Lots of refactoring, and added the capture_stderr method

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

28 files changed, 271 insertions, 123 deletions

diff --git a/bin/puppetdoc b/bin/puppetdoc
index 04af5b35e..00106ff72 100755
--- a/bin/puppetdoc
+++ b/bin/puppetdoc
@@ -143,9 +143,11 @@ types.sort { |a,b|
     #puts "%s States\n'''''''''''''''''''''''''''''''" % name.to_s.capitalize
     type.validstates.sort { |a,b|
         a.to_s <=> b.to_s
-    }.reject { |sname,state|
+    }.reject { |sname|
+        state = type.statebyname(sname)
         state.nodoc
-    }.each { |sname,state|
+    }.each { |sname|
+        state = type.statebyname(sname)
         docs[sname] = state.doc.gsub(/\n\s*/,' ')
         #puts "- **%s**" % sname
         #puts tab(1) + state.doc.gsub(/\n\s*/,' ')
diff --git a/lib/puppet/element.rb b/lib/puppet/element.rb
index 3991160db..4d1ad8bac 100644
--- a/lib/puppet/element.rb
+++ b/lib/puppet/element.rb
@@ -25,19 +25,7 @@ class Puppet::Element
         }
     }
 
-    # create instance methods for each of the log levels, too
-    Puppet::Log.eachlevel { |level|
-        define_method(level,proc { |args|
-            if args.is_a?(Array)
-                args = args.join(" ")
-            end
-            Puppet::Log.create(
-                :level => level,
-                :source => self,
-                :message => args
-            )
-        })
-    }
+    Puppet::Util.logmethods(self, true)
 
     # for testing whether we should actually do anything
     def noop
@@ -66,8 +54,11 @@ class Puppet::Element
                 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]
+                    if self.is_a?(Puppet::Type::Component)
+                        @path = [self.name]
+                    else
+                        @path = [self.class.name.to_s + "=" + self.name]
+                    end
                 end
             end
         end
diff --git a/lib/puppet/log.rb b/lib/puppet/log.rb
index 6120df0ac..9fff07225 100644
--- a/lib/puppet/log.rb
+++ b/lib/puppet/log.rb
@@ -183,7 +183,7 @@ module Puppet # :nodoc:
                         ] + RESET
                     else
                         puts @colors[msg.level] + "%s: %s: %s" % [
-                            msg.source, msg.level, msg.to_s
+                            msg.level, msg.source, msg.to_s
                         ] + RESET
                     end
                 when Puppet::Client::LogClient
@@ -294,9 +294,13 @@ module Puppet # :nodoc:
             Log.newmessage(self)
 		end
 
+        # If they pass a source in to us, we make sure it is a string, and
+        # we retrieve any tags we can.
         def source=(source)
-            # We can't store the actual source, we just store the path
-            if source.respond_to?(:path)
+            # We can't store the actual source, we just store the path.  This
+            # is a bit of a stupid hack, specifically testing for elements, but
+            # eh.
+            if source.is_a?(Puppet::Element) and source.respond_to?(:path)
                 @source = source.path
             else
                 @source = source.to_s
diff --git a/lib/puppet/server/authstore.rb b/lib/puppet/server/authstore.rb
index 075f1f9a1..9c8f700dc 100755
--- a/lib/puppet/server/authstore.rb
+++ b/lib/puppet/server/authstore.rb
@@ -19,6 +19,8 @@ class Server
             :name => [:hostname, :domain]
         }
 
+        Puppet::Util.logmethods(self, false)
+
         def allow(pattern)
             # a simple way to allow anyone at all to connect
             if pattern == "*"
@@ -66,7 +68,7 @@ class Server
                 }
             }
 
-            Puppet.info "Defaulting to false for %s" % name
+            self.info "Defaulting to false for %s" % name
             # default to false
             return false
         end
diff --git a/lib/puppet/server/fileserver.rb b/lib/puppet/server/fileserver.rb
index f792c7cd3..92bc129ec 100755
--- a/lib/puppet/server/fileserver.rb
+++ b/lib/puppet/server/fileserver.rb
@@ -19,6 +19,8 @@ class Server
             iface.add_method("string retrieve(string)")
         }
 
+        # Run 'retrieve' on a file.  This gets the actual parameters, so
+        # we can pass them to the client.
         def check(dir)
             unless FileTest.exists?(dir)
                 Puppet.notice "File source %s does not exist" % dir
@@ -41,17 +43,19 @@ class Server
             return obj
         end
 
+        # Describe a given file.  This returns all of the manageable aspects
+        # of that file.
         def describe(file, client = nil, clientip = nil)
             readconfig
             mount, path = splitpath(file)
 
-            unless @mounts[mount].allowed?(client, clientip)
+            unless mount.allowed?(client, clientip)
                 raise Puppet::Server::AuthorizationError, "Cannot access %s" % mount
             end
 
             sdir = nil
             unless sdir = subdir(mount, path)
-                Puppet.notice "Could not find subdirectory %s" %
+                mount.notice "Could not find subdirectory %s" %
                     "//%s/%s" % [mount, path]
                 return ""
             end
@@ -65,13 +69,13 @@ class Server
             CHECKPARAMS.each { |check|
                 if state = obj.state(check)
                     unless state.is
-                        Puppet.notice "Manually retrieving info for %s" % check
+                        mount.notice "Manually retrieving info for %s" % check
                         state.retrieve
                     end
                     desc << state.is
                 else
                     if check == "checksum" and obj.state(:type).is == "file"
-                        Puppet.notice "File %s does not have data for %s" %
+                        mount.notice "File %s does not have data for %s" %
                             [obj.name, check]
                     end
                     desc << nil
@@ -81,6 +85,7 @@ class Server
             return desc.join("\t")
         end
 
+        # Deal with ignore parameters.
         def handleignore(children, path, ignore)            
             ignore.each { |ignore|                
                 Dir.glob(File.join(path,ignore), File::FNM_DOTMATCH) { |match|
@@ -90,6 +95,7 @@ class Server
             return children
         end  
 
+        # Create a new fileserving module.
         def initialize(hash = {})
             @mounts = {}
             @files = {}
@@ -129,18 +135,19 @@ class Server
             end
         end
 
+        # List a specific directory's contents.
         def list(dir, recurse = false, ignore = false, client = nil, clientip = nil)
             readconfig
             mount, path = splitpath(dir)
 
-            unless @mounts[mount].allowed?(client, clientip)
+            unless mount.allowed?(client, clientip)
                 raise Puppet::Server::AuthorizationError, "Cannot access %s" % mount
             end
 
             subdir = nil
             unless subdir = subdir(mount, path)
-                Puppet.notice "Could not find subdirectory %s" %
-                    "//%s/%s" % [mount, path]
+                mount.notice "Could not find subdirectory %s" %
+                    "%s:%s" % [mount, path]
                 return ""
             end
 
@@ -149,12 +156,11 @@ class Server
                 return ""
             end
 
-            #rmdir = File.dirname(File.join(@mounts[mount], path))
-            rmdir = nameswap(dir, mount)
-            desc = reclist(rmdir, subdir, recurse, ignore)
+            rmdir = expand_mount(dir, mount)
+            desc = reclist(mount, rmdir, subdir, recurse, ignore)
 
             if desc.length == 0
-                Puppet.notice "Got no information on //%s/%s" %
+                mount.notice "Got no information on //%s/%s" %
                     [mount, path]
                 return ""
             end
@@ -164,6 +170,7 @@ class Server
             }.join("\n")
         end
 
+        # Mount a new directory with a name.
         def mount(path, name)
             if @mounts.include?(name)
                 if @mounts[name] != path
@@ -178,7 +185,7 @@ class Server
             if FileTest.directory?(path)
                 if FileTest.readable?(path)
                     @mounts[name] = Mount.new(name, path)
-                    Puppet.info "Mounted %s at %s" % [path, name]
+                    @mounts[name].info "Mounted"
                 else
                     raise FileServerError, "%s is not readable" % path
                 end
@@ -187,6 +194,7 @@ class Server
             end
         end
 
+        # Read the configuration file.
         def readconfig
             return if @noreadconfig
 
@@ -237,8 +245,7 @@ class Server
                             when "allow":
                                 value.split(/\s*,\s*/).each { |val|
                                     begin
-                                        Puppet.info "Allowing %s access to %s" %
-                                            [val, mount.name]
+                                        mount.info "Allowing %s access" % val
                                         mount.allow(val)
                                     rescue AuthStoreError => detail
                                         raise FileServerError, "%s at line %s of %s" %
@@ -248,8 +255,7 @@ class Server
                             when "deny":
                                 value.split(/\s*,\s*/).each { |val|
                                     begin
-                                        Puppet.info "Denying %s access to %s" %
-                                            [val, mount.name]
+                                        mount.info "Denying %s access" % val
                                         mount.deny(val)
                                     rescue AuthStoreError => detail
                                         raise FileServerError, "%s at line %s of %s" %
@@ -289,24 +295,21 @@ class Server
             @configstatted = Time.now
         end
 
+        # Retrieve a file from the local disk and pass it to the remote
+        # client.
         def retrieve(file, client = nil, clientip = nil)
             readconfig
             mount, path = splitpath(file)
 
-            unless (@mounts.include?(mount))
-                raise Puppet::Server::FileServerError,
-                    "FileServer module '%s' not mounted" % mount
-            end
-
-            unless @mounts[mount].allowed?(client, clientip)
+            unless mount.allowed?(client, clientip)
                 raise Puppet::Server::AuthorizationError, "Cannot access %s" % mount
             end
 
             fpath = nil
             if path
-                fpath = File.join(@mounts[mount].path, path)
+                fpath = File.join(mount.path, path)
             else
-                fpath = @mounts[mount].path
+                fpath = mount.path
             end
 
             unless FileTest.exists?(fpath)
@@ -324,24 +327,26 @@ class Server
 
         private
 
-        def nameswap(name, mount)
-            name.sub(/\/#{mount}/, @mounts[mount].path).gsub(%r{//}, '/').sub(
+        # Convert from the '/mount/path' form to the real path on disk.
+        def expand_mount(name, mount)
+            # Note that the user could have passed a path with multiple /'s
+            # in it, and we are likely to result in multiples, so we have to
+            # get rid of all of them.
+            name.sub(/\/#{mount.name}/, mount.path).gsub(%r{/+}, '/').sub(
                 %r{/$}, ''
             )
-            #Puppet.info "Swapped %s to %s" % [name, newname]
-            #newname
         end
 
-        # recursive listing function
-        def reclist(root, path, recurse, ignore)
-            #desc = [obj.name.sub(%r{#{root}/?}, '')]
+        # Recursively list the directory.
+        def reclist(mount, root, path, recurse, ignore)
+            # Take out the root of the path.
             name = path.sub(root, '')
             if name == ""
                 name = "/"
             end
 
             if name == path
-                raise Puppet::FileServerError, "Could not match %s in %s" %
+                raise FileServerError, "Could not match %s in %s" %
                     [root, path]
             end
 
@@ -362,7 +367,7 @@ class Server
                     end  
                     children.each { |child|
                         next if child =~ /^\.\.?$/
-                        reclist(root, File.join(path, child), recurse, ignore).each { |cobj|
+                        reclist(mount, root, File.join(path, child), recurse, ignore).each { |cobj|
                             ary << cobj
                         }
                     }
@@ -372,6 +377,7 @@ class Server
             return ary.reject { |c| c.nil? }
         end
 
+        # Split the path into the separate mount point and path.
         def splitpath(dir)
             # the dir is based on one of the mounts
             # so first retrieve the mount path
@@ -389,6 +395,9 @@ class Server
                     raise FileServerError,
                         "Fileserver error: Mount '%s' does not have a path set" % mount
                 end
+
+                # And now replace the name with the actual object.
+                mount = @mounts[mount]
             else
                 raise FileServerError, "Fileserver error: Invalid path '%s'" % dir
             end
@@ -399,8 +408,9 @@ class Server
             return mount, path
         end
 
+        # Retrieve a specific directory relative to a mount point.
         def subdir(mount, dir)
-            basedir = @mounts[mount].path
+            basedir = mount.path
 
             dirname = nil
             if dir
@@ -409,12 +419,18 @@ class Server
                 dirname = basedir
             end
 
-            return dirname
+            dirname
         end
 
+        # A simple class for wrapping mount points.  Instances of this class
+        # don't know about the enclosing object; they're mainly just used for
+        # authorization.
         class Mount < AuthStore
             attr_reader :path, :name
 
+            Puppet::Util.logmethods(self, true)
+
+            # Create out orbject.  It must have a name.
             def initialize(name, path = nil)
                 unless name =~ %r{^\w+$}
                     raise FileServerError, "Invalid name format '%s'" % name
@@ -430,6 +446,7 @@ class Server
                 super()
             end
 
+            # Set the path.
             def path=(path)
                 unless FileTest.exists?(path)
                     raise FileServerError, "%s does not exist" % path
@@ -438,7 +455,11 @@ class Server
             end
 
             def to_s
-                @path
+                if @path
+                    @name + ":" + @path
+                else
+                    @name
+                end
             end
 
             # Verify our configuration is valid.  This should really check to
diff --git a/lib/puppet/storage.rb b/lib/puppet/storage.rb
index dfcb5fe55..8f2d12de6 100644
--- a/lib/puppet/storage.rb
+++ b/lib/puppet/storage.rb
@@ -31,6 +31,7 @@ module Puppet
                 Puppet.info "Statefile %s does not exist" % Puppet[:checksumfile]
                 return
             end
+            begin
             #Puppet.debug "Loading statefile %s" % Puppet[:checksumfile]
 			File.open(Puppet[:checksumfile]) { |file|
 				file.each { |line|
@@ -46,6 +47,9 @@ module Puppet
                     end
 				}
 			}
+            rescue => detail
+                Puppet.err "Could not read %s" % Puppet[:checksumfile]
+            end
 
             #Puppet.debug "Loaded state is %s" % @@state.inspect
 		end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 5282076d5..a71104fc0 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -349,6 +349,14 @@ class Type < Puppet::Element
         return @validstates.keys
     end
 
+    # Return the state class associated with a name
+    def self.statebyname(name)
+        unless @validstates.length == @states.length
+            self.buildstatehash
+        end
+        @validstates[name]
+    end
+
     # does the name reflect a valid parameter?
     def self.validparameter?(name)
         unless defined? @parameters
diff --git a/lib/puppet/type/exec.rb b/lib/puppet/type/exec.rb
index fff3aeee2..49f3e8c4c 100755
--- a/lib/puppet/type/exec.rb
+++ b/lib/puppet/type/exec.rb
@@ -92,11 +92,20 @@ module Puppet
                         # handlers correctly
                         Puppet::Util.asuser(@parent[:user], @parent[:group]) {
                             # capture both stdout and stderr
-                            @output = %x{#{self.parent[:command]} 2>&1}
+
+                            stderr = Puppet::Util.capture_stderr {
+                                @output = %x{#{self.parent[:command]}}
+                            }
+
+                            if stderr != ""
+                                stderr.split(/\n/).each { |line|
+                                    self.send(:err, line)
+                                }
+                            end
                         }
                         status = $?
 
-                        loglevel = :info
+                        loglevel = @parent[:loglevel]
                         if status.exitstatus.to_s != self.should.to_s
                             err("%s returned %s" %
                                 [self.parent[:command],status.exitstatus])
@@ -157,7 +166,17 @@ module Puppet
             @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 *creates* parameter."
+                commands is to use the *creates* parameter.
+
+                It is worth nothing that ``exec`` is special, in that it is not
+                currently considered an error to have multiple ``exec`` instances
+                with the same name.  This was done purely because it had to be this
+                way in order to get certain functionality, but it complicates things.
+                In particular, you will not be able to use ``exec`` instances that
+                share their commands with other instances as a dependency, since
+                Puppet has no way of knowing which instance you mean.
+
+                It is recommended to avoid duplicate names whenever possible."
             @name = :exec
             @namevar = :command
 
diff --git a/lib/puppet/type/pfile.rb b/lib/puppet/type/pfile.rb
index ff097fe64..3d5a87011 100644
--- a/lib/puppet/type/pfile.rb
+++ b/lib/puppet/type/pfile.rb
@@ -471,7 +471,7 @@ module Puppet
                 end
 
                 unless stat = self.stat(true)
-                    self.debug "File does not exist" % self.name
+                    self.debug "File does not exist"
                     @states.each { |name,state|
                         # We've already retreived the source, and we don't
                         # want to overwrite whatever it did.  This is a bit
diff --git a/lib/puppet/type/service.rb b/lib/puppet/type/service.rb
index a89d56e12..24b5a2492 100644
--- a/lib/puppet/type/service.rb
+++ b/lib/puppet/type/service.rb
@@ -73,8 +73,7 @@ module Puppet
 
             def retrieve
                 self.is = @parent.status
-                self.debug "Running value is '%s'" %
-                    [self.parent.name,self.is]
+                self.debug "Running value is '%s'" % self.is
             end
 
             def sync
@@ -201,13 +200,14 @@ module Puppet
                 end
                 ps = Facter["ps"].value
                 unless ps and ps != ""
-                    raise Puppet::Error, "You must upgrade Facter"
+                    raise Puppet::Error,
+                        "You must upgrade Facter to a version that includes 'ps'"
                 end
                 regex = Regexp.new(self[:pattern])
                 IO.popen(ps) { |table|
                     table.each { |line|
                         if regex.match(line)
-                            ary = line.split(/\s+/)
+                            ary = line.sub(/^\s+/, '').split(/\s+/)
                             return ary[1]
                         end
                     }
@@ -230,6 +230,7 @@ module Puppet
                 end
 
                 # Extend the object with the service type
+                #self.info "extending with %s" % type
                 self.extend(type)
 
                 super
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 50f488905..6169bde81 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -85,6 +85,43 @@ module Util
         return retval
     end
 
+    # Capture stderr of a block
+    def self.capture_stderr
+        require 'stringio'
+        begin
+            $stderr = StringIO.new
+            yield
+            $stderr.rewind && $stderr.read
+        ensure
+            $stderr = STDERR
+        end
+    end
+
+    # Create instance methods for each of the log levels.  This allows
+    # the messages to be a little richer.  Most classes will be calling this
+    # method.
+    def self.logmethods(klass, useself = true)
+        Puppet::Log.eachlevel { |level|
+            klass.send(:define_method, level, proc { |args|
+                if args.is_a?(Array)
+                    args = args.join(" ")
+                end
+                if useself
+                    Puppet::Log.create(
+                        :level => level,
+                        :message => args
+                    )
+                else
+                    Puppet::Log.create(
+                        :level => level,
+                        :source => self,
+                        :message => args
+                    )
+                end
+            })
+        }
+    end
+
     # XXX this should all be done using puppet objects, not using
     # normal mkdir
     def self.recmkdir(dir,mode = 0755)
diff --git a/test/other/storage.rb b/test/other/storage.rb
new file mode 100755
index 000000000..aa010b7f3
--- /dev/null
+++ b/test/other/storage.rb
@@ -0,0 +1,39 @@
+if __FILE__ == $0
+    $:.unshift '..'
+    $:.unshift '../../lib'
+    $puppetbase = "../.."
+end
+
+require 'puppet'
+require 'puppettest'
+require 'test/unit'
+
+class TestParsedFile < Test::Unit::TestCase
+	include TestPuppet
+
+    def test_storeandretrieve
+        hash = {:a => :b, :c => :d}
+
+        state = nil
+        assert_nothing_raised {
+            state = Puppet::Storage.state(hash)
+        }
+
+        assert(!state.include?("name"))
+
+        assert_nothing_raised {
+            state["name"] = hash
+        }
+
+        assert_nothing_raised {
+            Puppet::Storage.store
+        }
+        assert_nothing_raised {
+            state = Puppet::Storage.state(hash)
+        }
+
+        assert_equal(state["name"], hash)
+    end
+end
+
+# $Id$
diff --git a/test/other/transactions.rb b/test/other/transactions.rb
index 738337dc4..c91c1456c 100644
--- a/test/other/transactions.rb
+++ b/test/other/transactions.rb
@@ -25,12 +25,12 @@ class TestTransactions < Test::Unit::TestCase
     end
 
     def setup
+        super
         @groups = %x{groups}.chomp.split(/ /)
         unless @groups.length > 1
             p @groups
             raise "You must be a member of more than one group to test this"
         end
-        super
     end
 
     def teardown
diff --git a/test/puppet/utiltest.rb b/test/puppet/utiltest.rb
index 64eb07d17..e18971e59 100755
--- a/test/puppet/utiltest.rb
+++ b/test/puppet/utiltest.rb
@@ -8,13 +8,15 @@ require 'puppet'
 require 'puppettest'
 require 'test/unit'
 
-unless Process.uid == 0
-    $stderr.puts "Run as root to perform Utility tests"
-else
 class TestPuppetUtil < Test::Unit::TestCase
     include TestPuppet
-
+    unless Process.uid == 0
+        $stderr.puts "Run as root to perform Utility tests"
+    else
     def mknverify(file, user, group = nil, id = false)
+        if File.exists?(file)
+            File.unlink(file)
+        end
         args = []
         unless user or group
             args << nil
@@ -60,15 +62,27 @@ class TestPuppetUtil < Test::Unit::TestCase
                 system("touch %s" % file)
             }
         }
-        assert(File.exists?(file), "File does not exist")
-        assert_equal(File.stat(file).uid, uid,
-            "File is owned by %s instead of %s" %
-            [File.stat(file).uid, uid]
-        )
-        assert_equal(File.stat(file).gid, gid,
-            "File group is %s instead of %s" %
-            [File.stat(file).gid, gid]
-        )
+        if uid == 0
+            #Puppet.warning "Not testing user"
+        else
+            #Puppet.warning "Testing user %s" % uid
+            assert(File.exists?(file), "File does not exist")
+            assert_equal(File.stat(file).uid, uid,
+                "File is owned by %s instead of %s" %
+                [File.stat(file).uid, uid]
+            )
+            #system("ls -l %s" % file)
+        end
+        if gid == 0
+            #Puppet.warning "Not testing group"
+        else
+            #Puppet.warning "Testing group %s" % gid
+            assert_equal(File.stat(file).gid, gid,
+                "File group is %s instead of %s" %
+                [File.stat(file).gid, gid]
+            )
+            #system("ls -l %s" % file)
+        end
         assert_nothing_raised {
             File.unlink(file)
         }
@@ -99,7 +113,12 @@ class TestPuppetUtil < Test::Unit::TestCase
 
         assert(Process.euid == 0, "UID did not get reset")
     end
-end
+    end
+
+    def test_capturestderr
+        str = Puppet::Util.capture_stderr { $stderr.puts("hello world") }
+        assert_equal("hello world\n", str)
+    end
 end
 
 # $Id$
diff --git a/test/puppettest.rb b/test/puppettest.rb
index f45356740..20bfb439d 100644
--- a/test/puppettest.rb
+++ b/test/puppettest.rb
@@ -34,7 +34,7 @@ module TestPuppet
             Dir.mkdir(@configpath)
         end
 
-        @@tmpfiles = [@configpath]
+        @@tmpfiles = [@configpath, tmpdir()]
         @@tmppids = []
 
         if $0 =~ /.+\.rb/
@@ -128,6 +128,13 @@ module TestPuppet
             else
                 "/tmp"
             end
+
+            @tmpdir = File.join(@tmpdir, "puppettesting")
+
+            unless File.exists?(@tmpdir)
+                FileUtils.mkdir_p(@tmpdir)
+                File.chmod(01777, @tmpdir)
+            end
         end
         @tmpdir
     end
@@ -303,7 +310,7 @@ module ExeTest
             %x{#{ps}}.chomp.split(/\n/).each { |line|
                 if line =~ /ruby.+puppetmasterd/
                     next if line =~ /\.rb/ # skip the test script itself
-                    ary = line.split(/\s+/)
+                    ary = line.sub(/^\s+/, '').split(/\s+/)
                     runningpid = ary[1].to_i
                 end
             }
diff --git a/test/server/authstore.rb b/test/server/authstore.rb
index c7dfd0978..1875dee07 100755
--- a/test/server/authstore.rb
+++ b/test/server/authstore.rb
@@ -15,14 +15,6 @@ require 'puppettest.rb'
 
 class TestAuthStore < Test::Unit::TestCase
 	include TestPuppet
-    def setup
-        if __FILE__ == $0
-            Puppet[:loglevel] = :debug
-        end
-
-        super
-    end
-
     def mkstore
         store = nil
         assert_nothing_raised {
diff --git a/test/server/bucket.rb b/test/server/bucket.rb
index 5d45eedb2..ac418484e 100644
--- a/test/server/bucket.rb
+++ b/test/server/bucket.rb
@@ -21,8 +21,10 @@ class TestBucket < Test::Unit::TestCase
         # iterate across all of the files
         files.each { |file|
             spin
+            tempdir = tempfile()
+            Dir.mkdir(tempdir)
             name = File.basename(file)
-            tmppath = File.join(tmpdir,name)
+            tmppath = File.join(tempdir,name)
             @@tmpfiles << tmppath
 
             # copy the files to our tmp directory so we can modify them...
diff --git a/test/server/fileserver.rb b/test/server/fileserver.rb
index 6712c30b7..f6037f17d 100755
--- a/test/server/fileserver.rb
+++ b/test/server/fileserver.rb
@@ -226,7 +226,7 @@ class TestFileServer < Test::Unit::TestCase
 
     # verify we can mount /, which is what local file servers will
     # normally do
-    def test_mountroot
+    def test_zmountroot
         server = nil
         assert_nothing_raised {
             server = Puppet::Server::FileServer.new(
@@ -247,6 +247,11 @@ class TestFileServer < Test::Unit::TestCase
         }
 
         assert(list =~ pattern)
+        assert_nothing_raised {
+            list = server.list("/root" + testdir, true, false)
+        }
+
+        assert(list =~ pattern)
     end
 
     # verify that we're correctly recursing the right number of levels
@@ -688,6 +693,17 @@ class TestFileServer < Test::Unit::TestCase
         list = nil
     end
 
+    # Verify that we get converted to the right kind of string
+    def test_mountstring
+        mount = nil
+        name = "yaytest"
+        path = tmpdir()
+        assert_nothing_raised {
+            mount = Puppet::Server::FileServer::Mount.new(name, path)
+        }
+
+        assert_equal(name + ":" + path, mount.to_s)
+    end
 end
 
 # $Id$
diff --git a/test/test b/test/test
index 15f9ad5f9..3f2bce2ae 100755
--- a/test/test
+++ b/test/test
@@ -6,6 +6,8 @@
 $:.unshift '.'
 $:.unshift '../lib'
 
+$puppetbase = ".."
+
 require 'puppettest.rb'
 require 'getoptlong'
 
@@ -26,8 +28,6 @@ result.each { |opt,arg|
 	end
 }
 
-$puppetbase = ".."
-
 suites = nil
 
 if ARGV.length != 0
diff --git a/test/types/component.rb b/test/types/component.rb
index d52123ae0..ea1c797ac 100755
--- a/test/types/component.rb
+++ b/test/types/component.rb
@@ -13,15 +13,8 @@ require 'test/unit'
 class TestComponent < Test::Unit::TestCase
 	include TestPuppet
     def setup
-        @@used = {}
-        super
-    end
-
-    def teardown
-        assert_nothing_raised() {
-            Puppet::Type.allclear
-        }
         super
+        @@used = {}
     end
 
     def randnum(limit)
diff --git a/test/types/cron.rb b/test/types/cron.rb
index f26dd69ef..bcd5be93c 100755
--- a/test/types/cron.rb
+++ b/test/types/cron.rb
@@ -15,6 +15,7 @@ require 'facter'
 class TestExec < Test::Unit::TestCase
 	include TestPuppet
     def setup
+        super
         # retrieve the user name
         id = %x{id}.chomp
         if id =~ /uid=\d+\(([^\)]+)\)/
@@ -27,8 +28,6 @@ class TestExec < Test::Unit::TestCase
         end
         # god i'm lazy
         @crontype = Puppet::Type::Cron
-
-        super
     end
 
     # Back up the user's existing cron tab if they have one.
diff --git a/test/types/exec.rb b/test/types/exec.rb
index 56b2439ad..bbe1ba3b6 100755
--- a/test/types/exec.rb
+++ b/test/types/exec.rb
@@ -220,6 +220,7 @@ class TestExec < Test::Unit::TestCase
             }
 
             if user
+                #Puppet.warning "Using user %s" % user.name
                 if id
                     # convert to a string, because that's what the object expects
                     args[:user] = user.uid.to_s
@@ -229,6 +230,7 @@ class TestExec < Test::Unit::TestCase
             end
 
             if group
+                #Puppet.warning "Using group %s" % group.name
                 if id
                     args[:group] = group.gid.to_s
                 else
diff --git a/test/types/fileignoresource.rb b/test/types/fileignoresource.rb
index f3d258381..ca89803d1 100644
--- a/test/types/fileignoresource.rb
+++ b/test/types/fileignoresource.rb
@@ -17,17 +17,17 @@ class TestFileIgnoreSources < Test::Unit::TestCase
 	include FileTesting
    
     def setup
+        super
         begin
             initstorage
         rescue
             system("rm -rf %s" % Puppet[:checksumfile])
         end
-        super
     end
 
     def teardown
-        clearstorage
         super
+        clearstorage
     end
 
 #This is not needed unless using md5 (correct me if I'm wrong)
diff --git a/test/types/group.rb b/test/types/group.rb
index 68e7f08b8..215229063 100755
--- a/test/types/group.rb
+++ b/test/types/group.rb
@@ -14,8 +14,8 @@ require 'test/unit'
 class TestGroup < Test::Unit::TestCase
 	include TestPuppet
     def setup
-        @@tmpgroups = []
         super
+        @@tmpgroups = []
     end
 
     def teardown
diff --git a/test/types/package.rb b/test/types/package.rb
index 1936c8bbc..69fb16d5e 100644
--- a/test/types/package.rb
+++ b/test/types/package.rb
@@ -30,9 +30,9 @@ end
 class TestPackages < Test::Unit::TestCase
 	include FileTesting
     def setup
+        super
         #@list = Puppet::Type::Package.getpkglist
         Puppet::Type::Package.clear
-        super
     end
 
     # These are packages that we're sure will be installed
diff --git a/test/types/query.rb b/test/types/query.rb
index 1793e1e17..86aadf678 100644
--- a/test/types/query.rb
+++ b/test/types/query.rb
@@ -5,21 +5,13 @@ if __FILE__ == $0
 end
 
 require 'puppet'
+require 'puppettest'
 require 'test/unit'
 
 # $Id$
 
 class TestQuery < Test::Unit::TestCase
-    def setup
-        Puppet[:loglevel] = :debug if __FILE__ == $0
-    end
-
-    def teardown
-        assert_nothing_raised() {
-            Puppet::Type.allclear
-        }
-    end
-
+    include TestPuppet
     # hmmm
     # this is complicated, because we store references to the created
     # objects in a central store
diff --git a/test/types/service.rb b/test/types/service.rb
index 5d99d662d..99384cf0b 100644
--- a/test/types/service.rb
+++ b/test/types/service.rb
@@ -14,16 +14,15 @@ class TestService < Test::Unit::TestCase
     # this is complicated, because we store references to the created
     # objects in a central store
     def setup
+        super
         sleeper = nil
         script = File.join($puppetbase,"examples/root/etc/init.d/sleeper")
         @status = script + " status"
-
-        super
     end
 
     def teardown
-        stopservices
         super
+        stopservices
     end
 
     def mksleeper(hash = {})
diff --git a/test/types/user.rb b/test/types/user.rb
index 1869b3d08..4408cfa4b 100755
--- a/test/types/user.rb
+++ b/test/types/user.rb
@@ -14,9 +14,8 @@ require 'test/unit'
 class TestUser < Test::Unit::TestCase
 	include TestPuppet
     def setup
-        @@tmpusers = []
-        Puppet[:loglevel] = :debug if __FILE__ == $0
         super
+        @@tmpusers = []
     end
 
     def teardown