[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, master, updated. debian/0.24.6-1-356-g5718585

[James Turnbull]

The following commit has been merged in the master branch:
commit 63ad84587892e9cab851cf516f7a381c5ea51f90
Author: Luke Kanies <luke at madstop.com>
Date:   Tue Sep 30 23:19:38 2008 -0500

    Refactoring and adding tests to the file group property.
    
    Drastically simplified the class, removing a lot of obsolete
    code and adding tests for nearly the whole class.
    
    Signed-off-by: Luke Kanies <luke at madstop.com>

diff --git a/lib/puppet/type/file/group.rb b/lib/puppet/type/file/group.rb
index cc482ff..75beb53 100755
--- a/lib/puppet/type/file/group.rb
+++ b/lib/puppet/type/file/group.rb
@@ -1,6 +1,10 @@
+require 'puppet/util/posix'
+
 # Manage file group ownership.
 module Puppet
     Puppet.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."
@@ -43,31 +47,7 @@ module Puppet
         end
 
         def retrieve
-            if self.should
-                @should = @should.collect do |val|
-                    unless val.is_a?(Integer)
-                        if tmp = validgroup?(val)
-                            val = tmp
-                        else
-                            raise "Could not find group %s" % val
-                        end
-                    else
-                        val
-                    end
-                end
-            end
-            stat = @resource.stat(false)
-
-            unless stat
-                return :absent
-            end
-
-            # Set our method appropriately, depending on links.
-            if stat.ftype == "link" and @resource[:links] != :follow
-                @method = :lchown
-            else
-                @method = :chown
-            end
+            return :absent unless stat = resource.stat(false)
 
             currentvalue = stat.gid
 
@@ -84,12 +64,8 @@ module Puppet
 
         # Determine if the group is valid, and if so, return the GID
         def validgroup?(value)
-            if value =~ /^\d+$/
-                value = value.to_i
-            end
-        
-            if gid = Puppet::Util.gid(value)
-                return gid
+            if number = gid(value)
+                return number
             else
                 return false
             end
@@ -99,32 +75,28 @@ module Puppet
         # 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
-            unless @resource.stat(false)
-                stat = @resource.stat(true)
-                currentvalue = self.retrieve
-
-                unless stat
-                    self.debug "File '%s' does not exist; cannot chgrp" %
-                        @resource[:path]
-                    return nil
-                end
+            # Set our method appropriately, depending on links.
+            if resource[:links] == :manage
+                method = :lchown
+            else
+                method = :chown
             end
 
             gid = nil
-            unless gid = Puppet::Util.gid(self.should)
-                raise Puppet::Error, "Could not find group %s" % self.should
+            @should.each do |group|
+                break if gid = validgroup?(group)
             end
 
+            raise Puppet::Error, "Could not find group(s) %s" % @should.join(",") unless gid
+
             begin
                 # set owner to nil so it's ignored
-                File.send(@method,nil,gid, at resource[:path])
+                File.send(method, nil, gid, resource[:path])
             rescue => detail
-                error = Puppet::Error.new( "failed to chgrp %s to %s: %s" %
-                    [@resource[:path], self.should, detail.message])
+                error = Puppet::Error.new( "failed to chgrp %s to %s: %s" % [resource[:path], gid, detail.message])
                 raise error
             end
             return :file_changed
         end
     end
 end
-
diff --git a/spec/unit/type/file/group.rb b/spec/unit/type/file/group.rb
new file mode 100755
index 0000000..2b47c75
--- /dev/null
+++ b/spec/unit/type/file/group.rb
@@ -0,0 +1,87 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+property = Puppet::Type.type(:file).attrclass(:group)
+
+describe property do
+    before do
+        @resource = mock 'resource'
+        @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
+
+        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
+
+            stat = stub 'stat', :ftype => "foo"
+            @resource.expects(:stat).returns stat
+            stat.expects(:gid).returns 1000
+
+            @group.expects(:warning)
+            @group.retrieve.should == :silly
+        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
+
+        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
+
+            File.expects(:chown).with(nil, 500, "/my/file")
+
+            @group.sync
+        end
+    end
+end

-- 
Puppet packaging for Debian