Fixing #372 and #374. All is not perfect, since OS X still cannot set UID, but it is much better. There is still plenty of bug-fixing to do on other platforms, I expect.

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

9 files changed, 98 insertions, 52 deletions

diff --git a/test/data/providers/package/testpackages.yaml b/test/data/providers/package/testpackages.yaml
index 47998ed33..8d8935eb9 100644
--- a/test/data/providers/package/testpackages.yaml
+++ b/test/data/providers/package/testpackages.yaml
@@ -57,6 +57,7 @@
   :files:
   - /Users/luke/Documents/Puppet/pkgtesting.pkg
   :name: pkgtesting
+  :cleanup: rm -rf /Library/Receipts/pkgtesting.pkg/ /tmp/file
 - :provider: :gem
   :name: wxrubylayouts
   :versions:
diff --git a/test/other/transactions.rb b/test/other/transactions.rb
index 2ae4b1b13..8c67b5b56 100755
--- a/test/other/transactions.rb
+++ b/test/other/transactions.rb
@@ -521,7 +521,9 @@ class TestTransactions < Test::Unit::TestCase
         
         # Make sure all of the components are gone
         comps = graph.vertices.find_all { |v| v.is_a?(Puppet::Type::Component)}
-        assert(comps.empty?, "Deps graph still contains components")
+        assert(comps.empty?, "Deps graph still contains components %s" % comps.inspect)
+        
+        assert_equal([], comps, "Deps graph still contains components")
         
         # It must be reversed because of how topsort works
         sorted = graph.topsort.reverse
diff --git a/test/providers/package.rb b/test/providers/package.rb
index 6b175c44a..6de88eace 100755
--- a/test/providers/package.rb
+++ b/test/providers/package.rb
@@ -118,6 +118,10 @@ class TestPackageProvider < Test::Unit::TestCase
                 return
             end
         end
+        
+        if cleancmd = hash[:cleanup]
+            hash.delete(:cleanup)
+        end
 
         pkg = nil
         assert_nothing_raised(
@@ -141,8 +145,14 @@ class TestPackageProvider < Test::Unit::TestCase
         end
 
         cleanup do
-            pkg[:ensure] = :absent
-            assert_apply(pkg)
+            if pkg.provider.respond_to?(:uninstall)
+                pkg[:ensure] = :absent
+                assert_apply(pkg)
+            else
+                if cleancmd
+                    system(cleancmd)
+                end
+            end
         end
 
         assert_nothing_raised("Could not install package") do
@@ -175,11 +185,13 @@ class TestPackageProvider < Test::Unit::TestCase
         end
 
         # Now remove the package
-        assert_nothing_raised do
-            provider.uninstall
-        end
+        if provider.respond_to?(:uninstall)
+            assert_nothing_raised do
+                provider.uninstall
+            end
 
-        assert_absent(provider)
+            assert_absent(provider)
+        end
     end
 
     # Now create a separate test method for each package
diff --git a/test/providers/provider.rb b/test/providers/provider.rb
index bde265fff..61090d5db 100755
--- a/test/providers/provider.rb
+++ b/test/providers/provider.rb
@@ -90,7 +90,7 @@ class TestProvider < Test::Unit::TestCase
         inst = provider.new(nil)
         assert_nothing_raised do
             [provider, inst].each do |thing|
-                out = thing.echo "some text"
+                out = thing.echo "some", "text"
                 assert_equal("some text\n", out)
             end
         end
@@ -167,12 +167,12 @@ class TestProvider < Test::Unit::TestCase
         inst = provider.new(nil)
 
         assert_raise(Puppet::ExecutionFailure) do
-            inst.cmd "some arguments"
+            inst.cmd "some", "arguments"
         end
 
         out = nil
         begin
-            inst.cmd "some arguments"
+            inst.cmd "some", "arguments"
         rescue Puppet::ExecutionFailure => detail
             out = detail.to_s
         end
@@ -180,10 +180,8 @@ class TestProvider < Test::Unit::TestCase
         assert(out =~ /A Failure/,
                "Did not receive command output on failure")
 
-        assert(out =~ /Could not execute/,
+        assert(out =~ /Execution of/,
                "Did not receive info wrapper on failure")
-
-
     end
 end
 
diff --git a/test/providers/user.rb b/test/providers/user.rb
index 25579ece0..40d3828cc 100755
--- a/test/providers/user.rb
+++ b/test/providers/user.rb
@@ -29,7 +29,6 @@ class TestUserProvider < Test::Unit::TestCase
             end
         }
         super
-        #Puppet.type(:user).clear
     end
 
     case Facter["operatingsystem"].value
@@ -51,20 +50,17 @@ class TestUserProvider < Test::Unit::TestCase
                 st.name == param
             }
 
-            output = %x{nireport / /users name #{state.netinfokey}}
-            output.split("\n").each { |line|
-                if line =~ /^(\w+)\s+(.+)$/
-                    username = $1
-                    id = $2.sub(/\s+$/, '')
-                    if username == user.name
-                        if id =~ /^[-0-9]+$/
-                            return Integer(id)
-                        else
-                            return id
-                        end
+            prov = Puppet::Type.type(:user).defaultprovider
+            output = prov.report(param)
+            # output = %x{nireport / /users name #{prov.netinfokey(param)}}
+            output.each { |hash|
+                if hash[:name] == user.name
+                    val = hash[param]
+                    if val =~ /^[-0-9]+$/
+                        return Integer(val)
+                    else
+                        return val
                     end
-                else
-                    raise "Could not match %s" % line
                 end
             }
 
@@ -125,8 +121,8 @@ class TestUserProvider < Test::Unit::TestCase
         case param
         when :name: name
         when :ensure: :present
-        when :comment: "Puppet Testing User %s" % name
-        when :gid: nonrootgroup.name
+        when :comment: "Puppet's Testing User %s" % name # use a single quote a la #375
+        when :gid: nonrootgroup.gid
         when :shell: findshell()
         when :home: "/home/%s" % name
         else
@@ -222,11 +218,12 @@ class TestUserProvider < Test::Unit::TestCase
     def attrtest_comment(user)
         old = user.comment
 
+        newname = "Billy O'Neal" # use a single quote, a la #372
         assert_nothing_raised {
-            user.comment = "A different comment"
+            user.comment = newname
         }
 
-        assert_equal("A different comment", current?(:comment, user),
+        assert_equal(newname, current?(:comment, user),
             "Comment was not changed")
 
         assert_nothing_raised {
@@ -423,7 +420,7 @@ class TestUserProvider < Test::Unit::TestCase
                 user.create
             }
 
-            assert_equal("Puppet Testing User pptest",
+            assert_equal("Puppet's Testing User pptest",
                  user.comment,
                 "Comment was not set")
 
@@ -453,14 +450,13 @@ class TestUserProvider < Test::Unit::TestCase
             assert_nothing_raised {
                 user.create
             }
-            assert_equal("Puppet Testing User pptest", user.comment,
+            assert_equal("Puppet's Testing User pptest", user.comment,
                 "Comment was not set")
 
             tests = Puppet::Type.type(:user).validstates
 
             just = nil
             tests.each { |test|
-                next unless test == :groups
                 if self.respond_to?("attrtest_%s" % test)
                     self.send("attrtest_%s" % test, user)
                 else
diff --git a/test/puppet/tc_suidmanager.rb b/test/puppet/tc_suidmanager.rb
index 674dfff8e..5561992e3 100755
--- a/test/puppet/tc_suidmanager.rb
+++ b/test/puppet/tc_suidmanager.rb
@@ -97,7 +97,7 @@ class TestSUIDManager < Test::Unit::TestCase
             else
                 uid = Process.uid
             end
-            cmd = "/bin/echo $EUID"
+            cmd = [%{/bin/echo $EUID}]
             output = Puppet::SUIDManager.run_and_capture(cmd, uid)[0].chomp
             assert_equal(uid.to_s, output)
         end
diff --git a/test/rails/rails.rb b/test/rails/rails.rb
index c0f902d65..1342437bd 100755
--- a/test/rails/rails.rb
+++ b/test/rails/rails.rb
@@ -17,16 +17,6 @@ class TestRails < Test::Unit::TestCase
     include PuppetTest::ResourceTesting
     include PuppetTest::RailsTesting
 
-    def setup
-        super
-        railsinit
-    end
-
-    def teardown
-        railsteardown
-        super
-    end
-
     def test_includerails
         assert_nothing_raised {
             require 'puppet/rails'
diff --git a/test/types/host.rb b/test/types/host.rb
index 580ecbf3b..18d87db77 100755
--- a/test/types/host.rb
+++ b/test/types/host.rb
@@ -12,7 +12,7 @@ class TestHost < Test::Unit::TestCase
 
     def setup
         super
-        @hosttype = Puppet.type(:host)
+        @hosttype = Puppet::Type.type(:host)
 
         @provider = @hosttype.defaultprovider
 
@@ -68,6 +68,10 @@ class TestHost < Test::Unit::TestCase
     if Facter.value(:operatingsystem) != "Darwin" or Process.uid == 0
     def test_simplehost
         host = nil
+        # We want to actually use the netinfo provider on darwin
+        if Facter.value(:operatingsystem) == "Darwin"
+            Puppet::Type.type(:host).defaultprovider = nil
+        end
 
         assert_nothing_raised {
             host = Puppet.type(:host).create(
@@ -76,6 +80,7 @@ class TestHost < Test::Unit::TestCase
             )
         }
 
+        host.retrieve
         assert_events([:host_created], host)
 
         assert_nothing_raised { host.retrieve }
@@ -118,12 +123,8 @@ class TestHost < Test::Unit::TestCase
 
         host.retrieve
 
-        if Facter.value(:operatingsystem) == "Darwin"
-            # Netinfo can't handle arrays right now
-            assert_equal(%w{madstop}, host.is(:alias))
-        else
-            assert_equal(%w{madstop kirby yayness}, host.is(:alias))
-        end
+        assert_equal(%w{madstop kirby yayness}, host.is(:alias))
+        
         host[:ensure] = :absent
         assert_events([:host_removed], host)
     end
diff --git a/test/util/utiltest.rb b/test/util/utiltest.rb
index ffb2ef347..74a29d56f 100755
--- a/test/util/utiltest.rb
+++ b/test/util/utiltest.rb
@@ -261,6 +261,52 @@ class TestPuppetUtil < Test::Unit::TestCase
 
         assert_equal(nil, ret)
     end
+    
+    def test_execute
+        command = tempfile()
+        File.open(command, "w") { |f|
+            f.puts %{#!/bin/sh\n/bin/echo "$1">&1; echo "$2">&2}
+        }
+        File.chmod(0755, command)
+        output = nil
+        assert_nothing_raised do
+            output = Puppet::Util.execute([command, "yaytest", "funtest"])
+        end
+        assert_equal("yaytest\nfuntest\n", output)
+        
+        # Now try it with a single quote
+        assert_nothing_raised do
+            output = Puppet::Util.execute([command, "yay'test", "funtest"])
+            # output = Puppet::Util.execute(command)
+            
+        end
+        assert_equal("yay'test\nfuntest\n", output)
+        
+        # Now test that we correctly fail if the command returns non-zero
+        assert_raise(Puppet::ExecutionFailure) do
+            out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"])
+        end
+        
+        # And that we can tell it not to fail
+        assert_nothing_raised() do
+            out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"], false)
+        end
+        
+        if Process.uid == 0
+            # Make sure we correctly set our uid and gid
+            user = nonrootuser
+            group = nonrootgroup
+            file = tempfile()
+            assert_nothing_raised do
+                Puppet::Util.execute(["touch", file], true, user.name, group.name)
+            end
+            assert(FileTest.exists?(file), "file was not created")
+            assert_equal(user.uid, File.stat(file).uid, "uid was not set correctly")
+            
+            # We can't really check the gid, because it just behaves too inconsistently everywhere.
+            # assert_equal(group.gid, File.stat(file).gid, "gid was not set correctly")
+        end
+    end
 end
 
 # $Id$