diff --git a/lib/puppet/provider/file/posix.rb b/lib/puppet/provider/file/posix.rb
index 23aabed3b..a3c75eef2 100644
--- a/lib/puppet/provider/file/posix.rb
+++ b/lib/puppet/provider/file/posix.rb
@@ -1,83 +1,135 @@
 Puppet::Type.type(:file).provide :posix do
   desc "Uses POSIX functionality to manage file's users and rights."
 
   confine :feature => :posix
 
   include Puppet::Util::POSIX
   include Puppet::Util::Warnings
 
   require 'etc'
 
   def uid2name(id)
     return id.to_s if id.is_a?(Symbol) or id.is_a?(String)
     return nil if id > Puppet[:maximum_uid].to_i
 
     begin
       user = Etc.getpwuid(id)
     rescue TypeError, ArgumentError
       return nil
     end
 
     if user.uid == ""
       return nil
     else
       return user.name
     end
   end
 
   # Determine if the user is valid, and if so, return the UID
   def name2uid(value)
     Integer(value) rescue uid(value) || false
   end
 
+  def gid2name(id)
+    return id.to_s if id.is_a?(Symbol) or id.is_a?(String)
+    return nil if id > Puppet[:maximum_uid].to_i
+
+    begin
+      group = Etc.getgrgid(id)
+    rescue TypeError, ArgumentError
+      return nil
+    end
+
+    if group.gid == ""
+      return nil
+    else
+      return group.name
+    end
+  end
+
+  def name2gid(value)
+    Integer(value) rescue gid(value) || false
+  end
+
   def owner
     unless stat = resource.stat
       return :absent
     end
 
     currentvalue = stat.uid
 
     # On OS X, files that are owned by -2 get returned as really
     # large UIDs instead of negative ones.  This isn't a Ruby bug,
     # it's an OS X bug, since it shows up in perl, too.
     if currentvalue > Puppet[:maximum_uid].to_i
       self.warning "Apparently using negative UID (#{currentvalue}) on a platform that does not consistently handle them"
       currentvalue = :silly
     end
 
     currentvalue
   end
 
   def owner=(should)
     # Set our method appropriately, depending on links.
     if resource[:links] == :manage
       method = :lchown
     else
       method = :chown
     end
 
     begin
       File.send(method, should, nil, resource[:path])
     rescue => detail
       raise Puppet::Error, "Failed to set owner to '#{should}': #{detail}"
     end
   end
 
+  def group
+    return :absent unless stat = resource.stat
+
+    currentvalue = stat.gid
+
+    # On OS X, files that are owned by -2 get returned as really
+    # large GIDs instead of negative ones.  This isn't a Ruby bug,
+    # it's an OS X bug, since it shows up in perl, too.
+    if currentvalue > Puppet[:maximum_uid].to_i
+      self.warning "Apparently using negative GID (#{currentvalue}) on a platform that does not consistently handle them"
+      currentvalue = :silly
+    end
+
+    currentvalue
+  end
+
+  def group=(should)
+    # Set our method appropriately, depending on links.
+    if resource[:links] == :manage
+      method = :lchgrp
+    else
+      method = :chgrp
+    end
+
+    begin
+      File.send(method, nil, should, resource[:path])
+    rescue => detail
+      raise Puppet::Error, "Failed to set group to '#{should}': #{detail}"
+    end
+  end
+
   def mode
     if stat = resource.stat
       return (stat.mode & 007777).to_s(8)
     else
       return :absent
     end
   end
 
   def mode=(value)
     begin
       File.chmod(value.to_i(8), resource[:path])
     rescue => detail
       error = Puppet::Error.new("failed to set mode #{mode} on #{resource[:path]}: #{detail.message}")
       error.set_backtrace detail.backtrace
       raise error
     end
   end
 end
diff --git a/lib/puppet/provider/file/windows.rb b/lib/puppet/provider/file/windows.rb
index d3ea9b0b7..d71e7d43c 100644
--- a/lib/puppet/provider/file/windows.rb
+++ b/lib/puppet/provider/file/windows.rb
@@ -1,79 +1,100 @@
 Puppet::Type.type(:file).provide :windows do
   desc "Uses Microsoft Windows functionality to manage file's users and rights."
 
   confine :feature => :microsoft_windows
 
   include Puppet::Util::Warnings
 
   if Puppet.features.microsoft_windows?
     require 'puppet/util/windows'
     require 'puppet/util/adsi'
     include Puppet::Util::Windows::Security
   end
 
   ERROR_INVALID_SID_STRUCTURE = 1337
 
-  def uid2name(id)
+  def id2name(id)
     # If it's a valid sid, get the name. Otherwise, it's already a name, so
     # just return it.
     begin
       if string_to_sid_ptr(id)
         name = nil
         Puppet::Util::ADSI.execquery(
           "SELECT Name FROM Win32_Account WHERE SID = '#{id}'
            AND LocalAccount = true"
-        ).each { |u| name ||= u.name }
+        ).each { |a| name ||= a.name }
         return name
       end
     rescue Puppet::Util::Windows::Error => e
       raise unless e.code == ERROR_INVALID_SID_STRUCTURE
     end
 
     id
   end
 
-  # Determine if the user is valid, and if so, return the UID
-  def name2uid(value)
-    # If it's a valid sid, then return it. Else, it's name we need to convert
+  # Determine if the account is valid, and if so, return the UID
+  def name2id(value)
+    # If it's a valid sid, then return it. Else, it's a name we need to convert
     # to sid.
     begin
       return value if string_to_sid_ptr(value)
     rescue Puppet::Util::Windows::Error => e
       raise unless e.code == ERROR_INVALID_SID_STRUCTURE
     end
 
     Puppet::Util::ADSI.sid_for_account(value) rescue nil
   end
 
+  # We use users and groups interchangeably, so use the same methods for both
+  # (the type expects different methods, so we have to oblige).
+  alias :uid2name :id2name
+  alias :gid2name :id2name
+
+  alias :name2gid :name2id
+  alias :name2uid :name2id
+
   def owner
     return :absent unless resource.exist?
     get_owner(resource[:path])
   end
 
   def owner=(should)
     begin
       set_owner(should, resource[:path])
     rescue => detail
       raise Puppet::Error, "Failed to set owner to '#{should}': #{detail}"
     end
   end
 
+  def group
+    return :absent unless resource.exist?
+    get_group(resource[:path])
+  end
+
+  def group=(should)
+    begin
+      set_group(should, resource[:path])
+    rescue => detail
+      raise Puppet::Error, "Failed to set group to '#{should}': #{detail}"
+    end
+  end
+
   def mode
     if resource.exist?
       get_mode(resource[:path]).to_s(8)
     else
       :absent
     end
   end
 
   def mode=(value)
     begin
       set_mode(value.to_i(8), resource[:path])
     rescue => detail
       error = Puppet::Error.new("failed to set mode #{mode} on #{resource[:path]}: #{detail.message}")
       error.set_backtrace detail.backtrace
       raise error
     end
     :file_changed
   end
 end
diff --git a/lib/puppet/type/file/group.rb b/lib/puppet/type/file/group.rb
index 29655313a..4310a106d 100755
--- a/lib/puppet/type/file/group.rb
+++ b/lib/puppet/type/file/group.rb
@@ -1,116 +1,33 @@
 require 'puppet/util/posix'
 
 # Manage file group ownership.
 module Puppet
   Puppet::Type.type(:file).newproperty(:group) do
-    include Puppet::Util::POSIX
-
-    require 'etc'
     desc "Which group should own the file.  Argument can be either group
       name or group ID."
-    @event = :file_changed
 
     validate do |group|
       raise(Puppet::Error, "Invalid group name '#{group.inspect}'") unless group and group != ""
     end
 
-    def id2name(id)
-      return id.to_s if id.is_a?(Symbol)
-      return nil if id > Puppet[:maximum_uid].to_i
-      begin
-        group = Etc.getgrgid(id)
-      rescue ArgumentError
-        return nil
-      end
-      if group.gid == ""
-        return nil
-      else
-        return group.name
-      end
-    end
-
-    # We want to print names, not numbers
-    def is_to_s(currentvalue)
-      if currentvalue.is_a? Integer
-        id2name(currentvalue) || currentvalue
-      else
-        return currentvalue.to_s
-      end
-    end
-
-    def should_to_s(newvalue = @should)
-      if newvalue.is_a? Integer
-        id2name(newvalue) || newvalue
-      else
-        return newvalue.to_s
-      end
-    end
-
     def insync?(current)
-      return true if Puppet.features.microsoft_windows?
-
-      @should.each do |value|
-        if value =~ /^\d+$/
-          gid = Integer(value)
-        elsif value.is_a?(String)
-          fail "Could not find group #{value}" unless gid = gid(value)
-        else
-          gid = value
-        end
-
-        return true if gid == current
+      # We don't want to validate/munge groups until we actually start to
+      # evaluate this property, because they might be added during the catalog
+      # apply.
+      @should.map! do |val|
+        provider.name2gid(val) or raise "Could not find group #{val}"
       end
-      false
-    end
-
-    def retrieve
-      return 0 if Puppet.features.microsoft_windows?
-      return :absent unless stat = resource.stat
-
-      currentvalue = stat.gid
 
-      # On OS X, files that are owned by -2 get returned as really
-      # large GIDs instead of negative ones.  This isn't a Ruby bug,
-      # it's an OS X bug, since it shows up in perl, too.
-      if currentvalue > Puppet[:maximum_uid].to_i
-        self.warning "Apparently using negative GID (#{currentvalue}) on a platform that does not consistently handle them"
-        currentvalue = :silly
-      end
-
-      currentvalue
+      @should.include?(current)
     end
 
-    # Determine if the group is valid, and if so, return the GID
-    def validgroup?(value)
-      Integer(value) rescue gid(value) || false
+    # We want to print names, not numbers
+    def is_to_s(currentvalue)
+      provider.gid2name(currentvalue) || currentvalue
     end
 
-    # Normal users will only be able to manage certain groups.  Right now,
-    # we'll just let it fail, but we should probably set things up so
-    # that users get warned if they try to change to an unacceptable group.
-    def sync
-      # Set our method appropriately, depending on links.
-      if resource[:links] == :manage
-        method = :lchown
-      else
-        method = :chown
-      end
-
-      gid = nil
-      @should.each do |group|
-        break if gid = validgroup?(group)
-      end
-
-      raise Puppet::Error, "Could not find group(s) #{@should.join(",")}" unless gid
-
-      begin
-        # set owner to nil so it's ignored
-        File.send(method, nil, gid, resource[:path])
-      rescue => detail
-        error = Puppet::Error.new( "failed to chgrp #{resource[:path]} to #{gid}: #{detail.message}")
-        raise error
-      end
-      :file_changed
+    def should_to_s(newvalue)
+      provider.gid2name(newvalue) || newvalue
     end
   end
 end
diff --git a/spec/unit/provider/file/posix_spec.rb b/spec/unit/provider/file/posix_spec.rb
index b8106ca9f..546aab2b5 100644
--- a/spec/unit/provider/file/posix_spec.rb
+++ b/spec/unit/provider/file/posix_spec.rb
@@ -1,133 +1,226 @@
 #!/usr/bin/env rspec
 
 require 'spec_helper'
 
 describe Puppet::Type.type(:file).provider(:posix), :if => Puppet.features.posix? do
   include PuppetSpec::Files
 
   let(:path) { tmpfile('posix_file_spec') }
   let(:resource) { Puppet::Type.type(:file).new :path => path, :mode => 0777, :provider => described_class.name }
   let(:provider) { resource.provider }
 
   describe "#mode" do
     it "should return a string with the higher-order bits stripped away" do
       FileUtils.touch(path)
       File.chmod(0644, path)
 
       provider.mode.should == '644'
     end
 
     it "should return absent if the file doesn't exist" do
       provider.mode.should == :absent
     end
   end
 
   describe "#mode=" do
     it "should chmod the file to the specified value" do
       FileUtils.touch(path)
       File.chmod(0644, path)
 
       provider.mode = '0755'
 
       provider.mode.should == '755'
     end
 
     it "should pass along any errors encountered" do
       expect do
         provider.mode = '644'
       end.to raise_error(Puppet::Error, /failed to set mode/)
     end
   end
 
   describe "#uid2name" do
     it "should return the name of the user identified by the id" do
       Etc.stubs(:getpwuid).with(501).returns(Struct::Passwd.new('jilluser', nil, 501))
 
       provider.uid2name(501).should == 'jilluser'
     end
 
     it "should return the argument if it's already a name" do
       provider.uid2name('jilluser').should == 'jilluser'
     end
 
     it "should return nil if the argument is above the maximum uid" do
       provider.uid2name(Puppet[:maximum_uid] + 1).should == nil
     end
 
     it "should return nil if the user doesn't exist" do
       Etc.expects(:getpwuid).raises(ArgumentError, "can't find user for 999")
 
       provider.uid2name(999).should == nil
     end
   end
 
   describe "#name2uid" do
     it "should return the id of the user if it exists" do
       passwd = Struct::Passwd.new('bobbo', nil, 502)
 
       Etc.stubs(:getpwnam).with('bobbo').returns(passwd)
       Etc.stubs(:getpwuid).with(502).returns(passwd)
 
       provider.name2uid('bobbo').should == 502
     end
 
     it "should return the argument if it's already an id" do
       provider.name2uid('503').should == 503
     end
 
     it "should return false if the user doesn't exist" do
       Etc.stubs(:getpwnam).with('chuck').raises(ArgumentError, "can't find user for chuck")
 
       provider.name2uid('chuck').should == false
     end
   end
 
   describe "#owner" do
     it "should return the uid of the file owner" do
       FileUtils.touch(path)
       owner = File.stat(path).uid
 
       provider.owner.should == owner
     end
 
     it "should return absent if the file can't be statted" do
       provider.owner.should == :absent
     end
 
     it "should warn and return :silly if the value is beyond the maximum uid" do
       stat = stub('stat', :uid => Puppet[:maximum_uid] + 1)
       resource.stubs(:stat).returns(stat)
 
       provider.owner.should == :silly
       @logs.should be_any {|log| log.level == :warning and log.message =~ /Apparently using negative UID/}
     end
   end
 
   describe "#owner=" do
     it "should set the owner but not the group of the file" do
       File.expects(:lchown).with(15, nil, resource[:path])
 
       provider.owner = 15
     end
 
     it "should chown a link if managing links" do
       resource[:links] = :manage
       File.expects(:lchown).with(20, nil, resource[:path])
 
       provider.owner = 20
     end
 
     it "should chown a link target if following links" do
       resource[:links] = :follow
       File.expects(:chown).with(20, nil, resource[:path])
 
       provider.owner = 20
     end
 
     it "should pass along any error encountered setting the owner" do
       File.expects(:lchown).raises(ArgumentError)
 
       expect { provider.owner = 25 }.to raise_error(Puppet::Error, /Failed to set owner to '25'/)
     end
   end
+
+  describe "#gid2name" do
+    it "should return the name of the group identified by the id" do
+      Etc.stubs(:getgrgid).with(501).returns(Struct::Passwd.new('unicorns', nil, nil, 501))
+
+      provider.gid2name(501).should == 'unicorns'
+    end
+
+    it "should return the argument if it's already a name" do
+      provider.gid2name('leprechauns').should == 'leprechauns'
+    end
+
+    it "should return nil if the argument is above the maximum gid" do
+      provider.gid2name(Puppet[:maximum_uid] + 1).should == nil
+    end
+
+    it "should return nil if the group doesn't exist" do
+      Etc.expects(:getgrgid).raises(ArgumentError, "can't find group for 999")
+
+      provider.gid2name(999).should == nil
+    end
+  end
+
+  describe "#name2gid" do
+    it "should return the id of the group if it exists" do
+      passwd = Struct::Passwd.new('penguins', nil, nil, 502)
+
+      Etc.stubs(:getgrnam).with('penguins').returns(passwd)
+      Etc.stubs(:getgrgid).with(502).returns(passwd)
+
+      provider.name2gid('penguins').should == 502
+    end
+
+    it "should return the argument if it's already an id" do
+      provider.name2gid('503').should == 503
+    end
+
+    it "should return false if the group doesn't exist" do
+      Etc.stubs(:getgrnam).with('wombats').raises(ArgumentError, "can't find group for wombats")
+
+      provider.name2gid('wombats').should == false
+    end
+
+  end
+
+  describe "#group" do
+    it "should return the gid of the file group" do
+      FileUtils.touch(path)
+      group = File.stat(path).gid
+
+      provider.group.should == group
+    end
+
+    it "should return absent if the file can't be statted" do
+      provider.group.should == :absent
+    end
+
+    it "should warn and return :silly if the value is beyond the maximum gid" do
+      stat = stub('stat', :gid => Puppet[:maximum_uid] + 1)
+      resource.stubs(:stat).returns(stat)
+
+      provider.group.should == :silly
+      @logs.should be_any {|log| log.level == :warning and log.message =~ /Apparently using negative GID/}
+    end
+  end
+
+  describe "#group=" do
+    it "should set the group but not the owner of the file" do
+      File.expects(:lchgrp).with(nil, 15, resource[:path])
+
+      provider.group = 15
+    end
+
+    it "should chgrp a link if managing links" do
+      resource[:links] = :manage
+      File.expects(:lchgrp).with(nil, 20, resource[:path])
+
+      provider.group = 20
+    end
+
+    it "should chgrp a link target if following links" do
+      resource[:links] = :follow
+      File.expects(:chgrp).with(nil, 20, resource[:path])
+
+      provider.group = 20
+    end
+
+    it "should pass along any error encountered setting the group" do
+      File.expects(:lchgrp).raises(ArgumentError)
+
+      expect { provider.group = 25 }.to raise_error(Puppet::Error, /Failed to set group to '25'/)
+    end
+  end
 end
diff --git a/spec/unit/provider/file/windows_spec.rb b/spec/unit/provider/file/windows_spec.rb
index 416a54687..6cedeebf0 100644
--- a/spec/unit/provider/file/windows_spec.rb
+++ b/spec/unit/provider/file/windows_spec.rb
@@ -1,110 +1,136 @@
 #!/usr/bin/env rspec
 
 require 'spec_helper'
 if Puppet.features.microsoft_windows?
   require 'puppet/util/windows'
   class WindowsSecurity
     extend Puppet::Util::Windows::Security
   end
 end
 
 describe Puppet::Type.type(:file).provider(:windows), :if => Puppet.features.microsoft_windows? do
   include PuppetSpec::Files
 
   let(:path) { tmpfile('windows_file_spec') }
   let(:resource) { Puppet::Type.type(:file).new :path => path, :mode => 0777, :provider => described_class.name }
   let(:provider) { resource.provider }
 
   describe "#mode" do
     it "should return a string with the higher-order bits stripped away" do
       FileUtils.touch(path)
       WindowsSecurity.set_mode(0644, path)
 
       provider.mode.should == '644'
     end
 
     it "should return absent if the file doesn't exist" do
       provider.mode.should == :absent
     end
   end
 
   describe "#mode=" do
     it "should chmod the file to the specified value" do
       FileUtils.touch(path)
       WindowsSecurity.set_mode(0644, path)
 
       provider.mode = '0755'
 
       provider.mode.should == '755'
     end
 
     it "should pass along any errors encountered" do
       expect do
         provider.mode = '644'
       end.to raise_error(Puppet::Error, /failed to set mode/)
     end
   end
 
-  describe "#uid2name" do
+  describe "#id2name" do
     it "should return the name of the user identified by the sid" do
       result = [stub('user', :name => 'quinn')]
       Puppet::Util::ADSI.stubs(:execquery).returns(result)
 
-      provider.uid2name('S-1-1-50').should == 'quinn'
+      provider.id2name('S-1-1-50').should == 'quinn'
     end
 
     it "should return the argument if it's already a name" do
-      provider.uid2name('flannigan').should == 'flannigan'
+      provider.id2name('flannigan').should == 'flannigan'
     end
 
     it "should return nil if the user doesn't exist" do
       Puppet::Util::ADSI.stubs(:execquery).returns []
 
-      provider.uid2name('S-1-1-50').should == nil
+      provider.id2name('S-1-1-50').should == nil
     end
   end
 
-  describe "#name2uid" do
+  describe "#name2id" do
     it "should return the sid of the user" do
       Puppet::Util::ADSI.stubs(:execquery).returns [stub('account', :Sid => 'S-1-1-50')]
 
-      provider.name2uid('anybody').should == 'S-1-1-50'
+      provider.name2id('anybody').should == 'S-1-1-50'
     end
 
     it "should return the argument if it's already a sid" do
-      provider.name2uid('S-1-1-50').should == 'S-1-1-50'
+      provider.name2id('S-1-1-50').should == 'S-1-1-50'
     end
 
     it "should return nil if the user doesn't exist" do
       Puppet::Util::ADSI.stubs(:execquery).returns []
 
-      provider.name2uid('someone').should == nil
+      provider.name2id('someone').should == nil
     end
   end
 
   describe "#owner" do
     it "should return the sid of the owner if the file does exist" do
       FileUtils.touch(resource[:path])
       provider.stubs(:get_owner).with(resource[:path]).returns('S-1-1-50')
 
       provider.owner.should == 'S-1-1-50'
     end
 
     it "should return absent if the file doesn't exist" do
       provider.owner.should == :absent
     end
   end
 
   describe "#owner=" do
     it "should set the owner to the specified value" do
       provider.expects(:set_owner).with('S-1-1-50', resource[:path])
       provider.owner = 'S-1-1-50'
     end
 
     it "should propagate any errors encountered when setting the owner" do
       provider.stubs(:set_owner).raises(ArgumentError)
 
       expect { provider.owner = 'S-1-1-50' }.to raise_error(Puppet::Error, /Failed to set owner/)
     end
   end
+
+  describe "#group" do
+    it "should return the sid of the group if the file does exist" do
+      FileUtils.touch(resource[:path])
+      provider.stubs(:get_group).with(resource[:path]).returns('S-1-1-50')
+
+      provider.group.should == 'S-1-1-50'
+    end
+
+    it "should return absent if the file doesn't exist" do
+      provider.group.should == :absent
+    end
+  end
+
+  describe "#group=" do
+    it "should set the group to the specified value" do
+      provider.expects(:set_group).with('S-1-1-50', resource[:path])
+      provider.group = 'S-1-1-50'
+    end
+
+    it "should propagate any errors encountered when setting the group" do
+      provider.stubs(:set_group).raises(ArgumentError)
+
+      expect { provider.group = 'S-1-1-50' }.to raise_error(Puppet::Error, /Failed to set group/)
+    end
+  end
 end
diff --git a/spec/unit/type/file/group_spec.rb b/spec/unit/type/file/group_spec.rb
index 707a37cd6..3817eb665 100755
--- a/spec/unit/type/file/group_spec.rb
+++ b/spec/unit/type/file/group_spec.rb
@@ -1,122 +1,60 @@
 #!/usr/bin/env rspec
-require 'spec_helper'
-
-property = Puppet::Type.type(:file).attrclass(:group)
-
-describe property do
-  before do
-    @resource = stub 'resource', :line => "foo", :file => "bar"
-    @resource.stubs(:[]).returns "foo"
-    @resource.stubs(:[]).with(:path).returns "/my/file"
-    @group = property.new :resource => @resource
-  end
-
-  it "should have a method for testing whether a group is valid" do
-    @group.must respond_to(:validgroup?)
-  end
-
-  it "should return the found gid if a group is valid" do
-    @group.expects(:gid).with("foo").returns 500
-    @group.validgroup?("foo").should == 500
-  end
-
-  it "should return false if a group is not valid" do
-    @group.expects(:gid).with("foo").returns nil
-    @group.validgroup?("foo").should be_false
-  end
-
-  describe "when retrieving the current value" do
-    it "should return :absent if the file cannot stat" do
-      @resource.expects(:stat).returns nil
-
-      @group.retrieve.should == :absent
-    end
 
-    it "should get the gid from the stat instance from the file" do
-      stat = stub 'stat', :ftype => "foo"
-      @resource.expects(:stat).returns stat
-      stat.expects(:gid).returns 500
-
-      @group.retrieve.should == 500
-    end
+require 'spec_helper'
 
-    it "should warn and return :silly if the found value is higher than the maximum uid value" do
-      Puppet.settings.expects(:value).with(:maximum_uid).returns 500
+describe Puppet::Type.type(:file).attrclass(:group) do
+  include PuppetSpec::Files
 
-      stat = stub 'stat', :ftype => "foo"
-      @resource.expects(:stat).returns stat
-      stat.expects(:gid).returns 1000
+  let(:path) { tmpfile('mode_spec') }
+  let(:resource) { Puppet::Type.type(:file).new :path => path, :group => 'users' }
+  let(:group) { resource.property(:group) }
 
-      @group.expects(:warning)
-      @group.retrieve.should == :silly
-    end
+  before :each do
+    # If the provider was already loaded without root, it won't have the
+    # feature, so we have to add it here to test.
+    Puppet::Type.type(:file).defaultprovider.has_feature :manages_ownership
   end
 
-  describe "when determining if the file is in sync" do
-    it "should directly compare the group values if the desired group is an integer" do
-      @group.should = [10]
-      @group.must be_safe_insync(10)
-    end
+  describe "#insync?" do
+    before :each do
+      resource[:group] = ['foos', 'bars']
 
-    it "should treat numeric strings as integers" do
-      @group.should = ["10"]
-      @group.must be_safe_insync(10)
+      resource.provider.stubs(:name2gid).with('foos').returns 1001
+      resource.provider.stubs(:name2gid).with('bars').returns 1002
     end
 
-    it "should convert the group name to an integer if the desired group is a string" do
-      @group.expects(:gid).with("foo").returns 10
-      @group.should = %w{foo}
+    it "should fail if an group's id can't be found by name" do
+      resource.provider.stubs(:name2gid).returns nil
 
-      @group.must be_safe_insync(10)
+      expect { group.insync?(5) }.to raise_error(/Could not find group foos/)
     end
 
-    it "should not validate that groups exist when a group is specified as an integer" do
-      @group.expects(:gid).never
-      @group.validgroup?(10)
+    it "should use the id for comparisons, not the name" do
+      group.insync?('foos').should be_false
     end
 
-    it "should fail if it cannot convert a group name to an integer" do
-      @group.expects(:gid).with("foo").returns nil
-      @group.should = %w{foo}
-
-      lambda { @group.safe_insync?(10) }.should raise_error(Puppet::Error)
+    it "should return true if the current group is one of the desired group" do
+      group.insync?(1001).should be_true
     end
 
-    it "should return false if the groups are not equal" do
-      @group.should = [10]
-      @group.should_not be_safe_insync(20)
+    it "should return false if the current group is not one of the desired group" do
+      group.insync?(1003).should be_false
     end
   end
 
-  describe "when changing the group" do
-    before do
-      @group.should = %w{one}
-      @group.stubs(:gid).returns 500
-    end
-
-    it "should chown the file if :links is set to :follow" do
-      @resource.expects(:[]).with(:links).returns :follow
-      File.expects(:chown)
-
-      @group.sync
-    end
-
-    it "should lchown the file if :links is set to :manage" do
-      @resource.expects(:[]).with(:links).returns :manage
-      File.expects(:lchown)
-
-      @group.sync
-    end
+  %w[is_to_s should_to_s].each do |prop_to_s|
+    describe "##{prop_to_s}" do
+      it "should use the name of the user if it can find it" do
+        resource.provider.stubs(:gid2name).with(1001).returns 'foos'
 
-    it "should use the first valid group in its 'should' list" do
-      @group.should = %w{one two three}
-      @group.expects(:validgroup?).with("one").returns nil
-      @group.expects(:validgroup?).with("two").returns 500
-      @group.expects(:validgroup?).with("three").never
+        group.send(prop_to_s, 1001).should == 'foos'
+      end
 
-      File.expects(:chown).with(nil, 500, "/my/file")
+      it "should use the id of the user if it can't" do
+        resource.provider.stubs(:gid2name).with(1001).returns nil
 
-      @group.sync
+        group.send(prop_to_s, 1001).should == 1001
+      end
     end
   end
 end