Users and groups now nearly work on normal machines and on os x, and I think I have a decent platform for expansion to some of the other important elements like hosts (probably most important after users and groups). Some tests are still failing on os x, because netinfo sucks, but I will hopefully be able to figure out a solution soon. Stupid OS X and NetInfo.

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

10 files changed, 323 insertions, 145 deletions

diff --git a/lib/puppet/statechange.rb b/lib/puppet/statechange.rb
index 0dd6aa858..cd5195616 100644
--- a/lib/puppet/statechange.rb
+++ b/lib/puppet/statechange.rb
@@ -121,7 +121,7 @@ module Puppet
                     self
             end
             unless @state.insync?
-                Puppet.info "Rolling %s backward" % self
+                Puppet.info "Backing %s" % self
                 return self.go
             else
                 Puppet.debug "rollback is already in sync: %s vs. %s" %
@@ -141,7 +141,9 @@ module Puppet
 		#---------------------------------------------------------------
 
         def to_s
-            return "change %s.%s" % [@transaction.object_id, self.object_id]
+            return "change %s.%s(%s)" %
+                [@transaction.object_id, self.object_id, @state.change_to_s]
+            #return "change %s.%s" % [@transaction.object_id, self.object_id]
         end
 	end
 end
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 2b8d93bbf..b44393efd 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -58,8 +58,13 @@ class Transaction
                     #@@changed.push change.state.parent
                 rescue => detail
                     Puppet.err("%s failed: %s" % [change,detail])
+                    if Puppet[:debug] and detail.respond_to?(:stack)
+                        puts detail.stack
+                    end
                     next
-                    # FIXME this should support using onerror to determine behaviour
+                    # FIXME this should support using onerror to determine
+                    # behaviour; or more likely, the client calling us
+                    # should do so
                 end
 
                 if events.nil?
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 49190596a..a9ae147ac 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1065,7 +1065,7 @@ class Type < Puppet::Element
 
         states.each { |state|
             unless state.insync?
-                #Puppet.debug("%s is not in sync" % state)
+                Puppet.debug("%s is not in sync" % state)
                 insync = false
             end
         }
diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb
index c4421e57f..eb15907af 100755
--- a/lib/puppet/type/group.rb
+++ b/lib/puppet/type/group.rb
@@ -25,10 +25,23 @@ module Puppet
                 :gid
             end
 
+            def autogen
+                highest = 0
+                Etc.group { |group|
+                    if group.gid > highest
+                        unless group.gid > 65000
+                            highest = group.gid
+                        end
+                    end
+                }
+
+                return highest + 1
+            end
+
             def should=(gid)
                 case gid
                 when String
-                    if gid =~ /^[0-9]+$/
+                    if gid =~ /^[-0-9]+$/
                         gid = Integer(gid)
                     else
                         raise Puppet::Error, "Invalid GID %s" % gid
@@ -41,6 +54,11 @@ module Puppet
                     unless gid == :auto or gid == :notfound
                         raise Puppet::DevError, "Invalid GID %s" % gid
                     end
+                    if gid == :auto
+                        unless self.class.autogen?
+                            gid = autogen
+                        end
+                    end
                 end
 
                 Puppet.info "Setting gid to %s" % gid
diff --git a/lib/puppet/type/nameservice/netinfo.rb b/lib/puppet/type/nameservice/netinfo.rb
index bd0fa2dda..35d7348d8 100644
--- a/lib/puppet/type/nameservice/netinfo.rb
+++ b/lib/puppet/type/nameservice/netinfo.rb
@@ -8,13 +8,6 @@ module Puppet
     class Type
         # Return the NetInfo directory in which a given object type is stored.
         # Defaults to the type's name if @netinfodir is unset.
-        def self.netinfodir
-            if defined? @netinfodir and @netinfodir
-                return @netinfodir
-            else
-                return @name
-            end
-        end
     end
 
     module NameService
@@ -47,6 +40,7 @@ module Puppet
                 if output == ""
                     return false
                 else
+                    #Puppet.debug "%s exists: %s" % [obj.name, output]
                     return true
                 end
             end
@@ -60,17 +54,21 @@ module Puppet
             end
 
             class NetInfoState < POSIX::POSIXState
+                def self.allatonce?
+                    false
+                end
+
                 def self.netinfokey
                     if defined? @netinfokey and @netinfokey
                         return @netinfokey
                     else
-                        return @name
+                        return self.name
                     end
                 end
 
                 def retrieve
                     dir = @parent.class.netinfodir
-                    cmd = ["nireport", "/" "/%s" % dir, "name"]
+                    cmd = ["nireport", "/", "/%s" % dir, "name"]
 
                     if key = self.class.netinfokey
                         cmd << key.to_s
@@ -79,16 +77,22 @@ module Puppet
                             "Could not find netinfokey for state %s" %
                             self.class.name
                     end
+                    Puppet.debug "Executing %s" % cmd.join(" ").inspect
 
                     output = %x{#{cmd.join(" ")} 2>&1}.split("\n").each { |line|
-                        name, value = line.chomp.split(/\s+/)
-
-                        if name == @parent.name
-                            if value =~ /^\d+$/
-                                @is = Integer(value)
-                            else
-                                @is = value
+                        if line =~ /^(\w+)\s+(.+)$/
+                            name = $1
+                            value = $2.sub(/\s+$/, '')
+
+                            if name == @parent.name
+                                if value =~ /^[-0-9]+$/
+                                    @is = Integer(value)
+                                else
+                                    @is = value
+                                end
                             end
+                        else
+                            raise Puppet::DevError, "Could not match %s" % line
                         end
                     }
 
@@ -108,7 +112,11 @@ module Puppet
                     cmd << "/" << "/%s/%s" %
                         [@parent.class.netinfodir, @parent.name]
 
-                    cmd.join(" ")
+                    #if arg == "-create"
+                    #    return [cmd.join(" "), self.modifycmd].join(";")
+                    #else
+                        return cmd.join(" ")
+                    #end
                 end
 
                 def deletecmd
@@ -132,7 +140,33 @@ module Puppet
                 end
             end
 
-            class GroupGID < NetInfoState; end
+            class GroupGID < NetInfoState
+            end
+
+            class UserUID       < NetInfoState
+            end
+
+            class UserGID       < NetInfoState
+            end
+
+            class UserComment   < NetInfoState
+                @netinfokey = "realname"
+            end
+
+            class UserHome      < NetInfoState
+            end
+
+            class UserShell     < NetInfoState
+            end
+
+            class UserLocked    < NetInfoState
+            end
+
+            class UserExpire    < NetInfoState
+            end
+
+            class UserInactive  < NetInfoState
+            end
         end
     end
 end
diff --git a/lib/puppet/type/nameservice/objectadd.rb b/lib/puppet/type/nameservice/objectadd.rb
index 02b43f669..0a6842955 100644
--- a/lib/puppet/type/nameservice/objectadd.rb
+++ b/lib/puppet/type/nameservice/objectadd.rb
@@ -34,6 +34,10 @@ module Puppet
             end
 
             class ObjectAddGroup < POSIX::POSIXState
+                def self.allatonce?
+                    true
+                end
+
                 def addcmd
                     cmd = ["groupadd"]
                     if gid = @parent.should(:gid)
@@ -62,9 +66,14 @@ module Puppet
 
             class GroupGID       < ObjectAddGroup
                 @objectaddflag = "-g"
+                @autogen = true
             end
 
             class ObjectAddUser < POSIX::POSIXState
+                def self.allatonce?
+                    true
+                end
+
                 def addcmd
                     cmd = ["useradd"]
                     @parent.eachstate { |state|
@@ -98,10 +107,12 @@ module Puppet
             end
             class UserUID       < ObjectAddUser
                 @objectaddflag = "-u"
+                @autogen = true
             end
 
             class UserGID       < ObjectAddUser
                 @objectaddflag = "-g"
+                @autogen = true
             end
 
             class UserComment   < ObjectAddUser
@@ -110,10 +121,12 @@ module Puppet
 
             class UserHome      < ObjectAddUser
                 @objectaddflag = "-d"
+                @autogen = true
             end
 
             class UserShell     < ObjectAddUser
                 @objectaddflag = "-s"
+                @autogen = true
             end
 
             class UserLocked    < ObjectAddUser
@@ -121,10 +134,12 @@ module Puppet
 
             class UserExpire    < ObjectAddUser
                 @objectaddflag = "-e"
+                @autogen = true
             end
 
             class UserInactive  < ObjectAddUser
                 @objectaddflag = "-f"
+                @autogen = true
             end
         end
     end
diff --git a/lib/puppet/type/nameservice/posix.rb b/lib/puppet/type/nameservice/posix.rb
index caa125f31..33d364dda 100644
--- a/lib/puppet/type/nameservice/posix.rb
+++ b/lib/puppet/type/nameservice/posix.rb
@@ -20,6 +20,22 @@ module Puppet
             class POSIXState < Puppet::State
                 class << self
                     attr_accessor :extender
+
+                    def autogen?
+                        if defined? @autogen
+                            return @autogen
+                        else
+                            return false
+                        end
+                    end
+
+                    def optional?
+                        if defined? @optional
+                            return @optional
+                        else
+                            return false
+                        end
+                    end
                 end
 
                 def self.doc
@@ -80,15 +96,26 @@ module Puppet
                 end
 
                 def sync
-                    obj = @parent.getinfo
-
+                    event = nil
+                    if @is == :notfound
+                        self.retrieve
+                        if @is == @should
+                            return nil
+                        end
+                    end
                     # if the object needs to be created or deleted,
                     # depend on another method to do it all at once
                     if @is == :notfound or @should == :notfound
-                        return syncname()
+                        event = syncname()
+
+                        # if the whole object is created at once, just return
+                        # an event saying so
+                        if self.class.allatonce?
+                            return event
+                        end
                     end
 
-                    if obj.nil?
+                    unless @parent.exists?
                         raise Puppet::DevError,
                             "%s %s does not exist; cannot set %s" %
                             [@parent.class.name, @parent.name, self.class.name]
@@ -102,24 +129,27 @@ module Puppet
 
                     output = %x{#{cmd} 2>&1}
 
+
                     unless $? == 0
                         raise Puppet::Error, "Could not modify %s on %s %s: %s" %
                             [self.class.name, @parent.class.name,
                                 @parent.name, output]
                     end
 
-                    return "#{@parent.class.name}_modified".intern
+                    if event
+                        return event
+                    else
+                        return "#{@parent.class.name}_modified".intern
+                    end
                 end
 
                 private
                 def syncname
-                    obj = @parent.getinfo
-                    
                     cmd = nil
                     event = nil
                     if @should == :notfound
                         # we need to remove the object...
-                        if obj.nil?
+                        unless @parent.exists?
                             # the group already doesn't exist
                             return nil
                         end
@@ -129,10 +159,8 @@ module Puppet
                         cmd = self.deletecmd
                         type = "delete"
                     else
-                        unless obj.nil?
-                            raise Puppet::DevError,
-                                "Got told to create a %s that already exists" %
-                                @parent.class.name
+                        if @parent.exists?
+                            return nil
                         end
 
                         # blah blah, define elsewhere, blah blah
@@ -148,6 +176,18 @@ module Puppet
                             [type, @parent.class.name, @parent.name, output]
                     end
 
+                    # we want object creation to show up as one event, 
+                    # not many
+                    unless self.class.allatonce?
+                        if type == "create"
+                            Puppet.info "syncing everyone"
+                            @parent.eachstate { |state|
+                                Puppet.info "syncing %s" % state.name
+                                state.sync
+                            }
+                        end
+                    end
+
                     return "#{@parent.class.name}_#{type}d".intern
                 end
             end
diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb
index 32b4bca18..43374d1f4 100755
--- a/lib/puppet/type/user.rb
+++ b/lib/puppet/type/user.rb
@@ -19,6 +19,38 @@ module Puppet
             def self.name
                 :uid
             end
+
+            def autogen
+                highest = 0
+                Etc.passwd { |user|
+                    if user.uid > highest
+                        unless user.uid > 65000
+                            highest = user.uid
+                        end
+                    end
+                }
+
+                return highest + 1
+            end
+
+            def should=(value)
+                case value
+                when String
+                    if value =~ /^[-0-9]+$/
+                        value = Integer(value)
+                    end
+                when Symbol
+                    unless value == :notfound or value == :auto
+                        raise Puppet::DevError, "Invalid UID %s" % value
+                    end
+
+                    if value == :auto
+                        value = autogen()
+                    end
+                end
+
+                @should = value
+            end
         end
 
         module UserGID
@@ -34,7 +66,7 @@ module Puppet
             def should=(gid)
                 method = :getgrgid
                 if gid.is_a?(String)
-                    if gid =~ /^[0-9]+$/
+                    if gid =~ /^[-0-9]+$/
                         gid = Integer(gid)
                     else
                         method = :getgrnam
@@ -51,6 +83,7 @@ module Puppet
                         [gid, detail]
                 end
 
+                Puppet.notice "setting gid to %s" % ginfo.gid.inspect
                 @should = ginfo.gid
             end
         end
@@ -64,6 +97,10 @@ module Puppet
                 :comment
             end
 
+            def self.optional?
+                true
+            end
+
             def self.posixmethod
                 :gecos
             end
@@ -186,6 +223,10 @@ module Puppet
 
             @netinfodir = "users"
 
+            def exists?
+                self.class.statemodule.exists?(self)
+            end
+
             def getinfo(refresh = false)
                 if @userinfo.nil? or refresh == true
                     begin
@@ -202,6 +243,19 @@ module Puppet
                 @userinfo = nil
                 super
 
+                self.class.states.each { |state|
+                    next if @states.include?(state.name)
+
+                    unless state.autogen? or state.optional?
+                        if state.method_defined?(:autogen)
+                            self[state.name] = :auto
+                        else
+                            raise Puppet::Error,
+                                "Users require a value for %s" % state.name
+                        end
+                    end
+                }
+
                 if @states.empty?
                     self[:comment] = self[:name]
                 end
diff --git a/test/types/tc_group.rb b/test/types/tc_group.rb
index afbd91cc1..b16f28eba 100755
--- a/test/types/tc_group.rb
+++ b/test/types/tc_group.rb
@@ -19,9 +19,6 @@ class TestGroup < TestPuppet
 
     def teardown
         Puppet::Type::Group.clear
-        if Facter["operatingsystem"].value == "Darwin"
-            Puppet::State::GroupNInfo.flush()
-        end
         @@tmpgroups.each { |group|
             unless missing?(group)
                 remove(group)
@@ -218,14 +215,7 @@ class TestGroup < TestPuppet
             }
 
             @@tmpgroups << name
-            case Facter["operatingsystem"].value
-            when "Darwin":
-                trans = assert_events(comp, [:group_created, :group_modified],
-                    "group")
-            else
-                trans = assert_events(comp, [:group_created],
-                    "group")
-            end
+            trans = assert_events(comp, [:group_created], "group")
 
             obj = nil
             assert_nothing_raised {
diff --git a/test/types/tc_user.rb b/test/types/tc_user.rb
index fd1e7d8e6..c3b518542 100755
--- a/test/types/tc_user.rb
+++ b/test/types/tc_user.rb
@@ -20,16 +20,95 @@ class TestUser < TestPuppet
 
     def teardown
         @@tmpusers.each { |user|
-            begin
-                obj = Etc.getpwnam(user)
-                system("userdel %s" % user)
-            rescue ArgumentError => detail
-                # no such user, so we're fine
+            unless missing?(user)
+                remove(user)
             end
         }
         super
     end
 
+    case Facter["operatingsystem"].value
+    when "Darwin":
+        def missing?(user)
+            output = %x{nidump -r /users/#{user} / 2>/dev/null}.chomp
+
+            if output == ""
+                return true
+            else
+                return false
+            end
+
+            assert_equal("", output, "User %s is present:\n%s" % [user, output])
+        end
+
+        def current?(param, name)
+            state = Puppet::Type::User.states.find { |st|
+                st.name == param
+            }
+
+            output = %x{nireport / /users name #{state.netinfokey}}
+            output.split("\n").each { |line|
+                if line =~ /^(\w+)\s+(.+)$/
+                    user = $1
+                    id = $2.sub(/\s+$/, '')
+                    if user == name
+                        if id =~ /^[-0-9]+$/
+                            return Integer(id)
+                        else
+                            return id
+                        end
+                    end
+                else
+                    raise "Could not match %s" % line
+                end
+            }
+
+            return nil
+        end
+
+        def remove(user)
+            system("niutil -destroy / /users/%s" % user)
+        end
+    else
+        def missing?(user)
+            begin
+                obj = Etc.getgrnam(user)
+                return false
+            rescue ArgumentError
+                return true
+            end
+        end
+
+        def current?(param, name)
+            state = Puppet::Type::User.states.find { |st|
+                state.name == param
+            }
+
+            assert_nothing_raised {
+                obj = Etc.getgrnam(name)
+                return obj.send(state.posixmethod)
+            }
+
+            return nil
+        end
+
+        def remove(user)
+            system("userdel %s" % user)
+        end
+    end
+
+    def findshell(old = nil)
+        %w{/bin/sh /bin/bash /sbin/sh /bin/ksh /bin/zsh /bin/csh /bin/tcsh
+            /usr/bin/sh /usr/bin/bash /usr/bin/ksh /usr/bin/zsh /usr/bin/csh
+            /usr/bin/tcsh}.find { |shell|
+                if old
+                    FileTest.exists?(shell) and shell != old
+                else
+                    FileTest.exists?(shell)
+                end
+        }
+    end
+
     def attrtest_comment(user)
         old = user.is(:comment)
         user[:comment] = "A different comment"
@@ -38,30 +117,20 @@ class TestUser < TestPuppet
 
         trans = assert_events(comp, [:user_modified], "user")
 
-        obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal("A different comment", obj.gecos, "Comment was not changed")
+        assert_equal("A different comment", current?(:comment, user[:name]),
+            "Comment was not changed")
 
         assert_rollback_events(trans, [:user_modified], "user")
 
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal(old, obj.gecos, "Comment was not reverted")
+        assert_equal(old, current?(:comment, user[:name]),
+            "Comment was not reverted")
     end
 
     def attrtest_home(user)
         obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-        old = obj.dir
         comp = newcomp("hometest", user)
 
+        old = current?(:home, user[:name])
         user[:home] = old
 
         trans = assert_events(comp, [], "user")
@@ -70,39 +139,22 @@ class TestUser < TestPuppet
 
         trans = assert_events(comp, [:user_modified], "user")
 
-        obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal("/tmp", obj.dir, "Home was not changed")
+        assert_equal("/tmp", current?(:home, user[:name]), "Home was not changed")
 
         assert_rollback_events(trans, [:user_modified], "user")
 
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal(old, obj.dir, "Home was not reverted")
+        assert_equal(old, current?(:home, user[:name]), "Home was not reverted")
     end
 
     def attrtest_shell(user)
-        obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-        old = obj.shell
+        old = current?(:shell, user[:name])
         comp = newcomp("shelltest", user)
 
         user[:shell] = old
 
         trans = assert_events(comp, [], "user")
 
-        newshell = %w{/bin/sh /bin/bash /sbin/sh /bin/ksh /bin/zsh /bin/csh /bin/tcsh
-            /usr/bin/sh /usr/bin/bash /usr/bin/ksh /usr/bin/zsh /usr/bin/csh
-            /usr/bin/tcsh}.find { |shell|
-                FileTest.exists?(shell) and shell != old
-        }
+        newshell = findshell(old)
 
         unless newshell
             $stderr.puts "Cannot find alternate shell; skipping shell test"
@@ -113,30 +165,23 @@ class TestUser < TestPuppet
 
         trans = assert_events(comp, [:user_modified], "user")
 
-        obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal(newshell, obj.shell, "Shell was not changed")
+        assert_equal(newshell, current?(:shell, user[:name]),
+            "Shell was not changed")
 
         assert_rollback_events(trans, [:user_modified], "user")
 
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal(old, obj.shell, "Shell was not reverted")
+        assert_equal(old, current?(:shell, user[:name]), "Shell was not reverted")
     end
 
     def attrtest_gid(user)
         obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-        old = obj.gid
+        old = current?(:gid,user.name)
         comp = newcomp("gidtest", user)
 
+        user.retrieve
+        Puppet.notice "%s vs %s vs %s" %
+            [user.is(:gid), user.should(:gid), old.inspect]
+
         user[:gid] = old
 
         trans = assert_events(comp, [], "user")
@@ -145,7 +190,6 @@ class TestUser < TestPuppet
                 begin
                     group = Etc.getgrnam(gid)
                 rescue ArgumentError => detail
-                    false
                     next
                 end
                 old != group.gid
@@ -170,32 +214,22 @@ class TestUser < TestPuppet
             user[:gid] = newgid
         }
 
-        assert_events(comp, [], "user")
+        user.retrieve
 
-        obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
+        assert_events(comp, [], "user")
 
-        assert_equal(newgid, obj.gid, "GID was not changed")
+        assert_equal(newgid, current?(:gid,user[:name]), "GID was not changed")
 
         assert_rollback_events(trans, [:user_modified], "user")
 
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal(old, obj.gid, "GID was not reverted")
+        assert_equal(old, current?(:gid,user[:name]), "GID was not reverted")
     end
 
     def attrtest_uid(user)
         obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-        old = obj.uid
         comp = newcomp("uidtest", user)
 
+        old = current?(:uid, user[:name])
         user[:uid] = old
 
         trans = assert_events(comp, [], "user")
@@ -221,20 +255,11 @@ class TestUser < TestPuppet
 
         trans = assert_events(comp, [:user_modified], "user")
 
-        obj = nil
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal(newuid, obj.uid, "UID was not changed")
+        assert_equal(newuid, current?(:uid, user[:name]), "UID was not changed")
 
         assert_rollback_events(trans, [:user_modified], "user")
 
-        assert_nothing_raised {
-            obj = Etc.getpwnam(user[:name])
-        }
-
-        assert_equal(old, obj.uid, "UID was not reverted")
+        assert_equal(old, current?(:uid, user[:name]), "UID was not reverted")
     end
 
     def test_eachmethod
@@ -257,29 +282,26 @@ class TestUser < TestPuppet
             user = nil
             name = "pptest"
 
-            assert_raise(ArgumentError) {
-                Etc.getpwnam(name)
-            }
+            assert(missing?(name), "User %s is present" % name)
 
             assert_nothing_raised {
                 user = Puppet::Type::User.create(
                     :name => name,
-                    :comment => "Puppet Testing User"
+                    :comment => "Puppet Testing User",
+                    :gid => Process.gid,
+                    :shell => findshell(),
+                    :home => "/home/%s" % name
                 )
             }
 
+            @@tmpusers << name
+
             comp = newcomp("usercomp", user)
 
             trans = assert_events(comp, [:user_created], "user")
 
-            @@tmpusers << name
-
-            obj = nil
-            assert_nothing_raised {
-                obj = Etc.getpwnam(name)
-            }
-
-            assert_equal("Puppet Testing User", obj.gecos, "Comment was not set")
+            assert_equal("Puppet Testing User", current?(:comment, user[:name]),
+                "Comment was not set")
 
             tests = Puppet::Type::User.validstates.collect { |name, state|
                 state.name
@@ -296,9 +318,7 @@ class TestUser < TestPuppet
 
             assert_rollback_events(trans, [:user_deleted], "user")
 
-            assert_raise(ArgumentError) {
-                Etc.getpwnam(user[:name])
-            }
+            assert(missing?(user[:name]))
         end
     else
         $stderr.puts "Not root; skipping user creation/modification tests"