Merge pull request #1134 from opscode/CHEF-3297

CHEF-3297: Add excluded_members property to the group resources

23 files changed, 659 insertions, 289 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index 99054eb254..1c952318f1 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -77,6 +77,7 @@ class Chef
     class DsclCommandFailed < RuntimeError; end
     class UserIDNotFound < ArgumentError; end
     class GroupIDNotFound < ArgumentError; end
+    class ConflictingMembersInGroup < ArgumentError; end
     class InvalidResourceReference < RuntimeError; end
     class ResourceNotFound < RuntimeError; end
     class InvalidResourceSpecification < ArgumentError; end
diff --git a/lib/chef/provider/group.rb b/lib/chef/provider/group.rb
index eacb033492..09c2a0052d 100644
--- a/lib/chef/provider/group.rb
+++ b/lib/chef/provider/group.rb
@@ -63,6 +63,17 @@ class Chef
           a.failure_message(Chef::Exceptions::Group, "Cannot modify #{@new_resource} - group does not exist!")
           a.whyrun("Group #{@new_resource} does not exist. Unless it would have been created earlier in this run, this attempt to modify it would fail.")
         end
+
+        requirements.assert(:all_actions) do |a|
+          # Make sure that the resource doesn't contain any common
+          # user names in the members and exclude_members properties.
+          if !@new_resource.members.nil? && !@new_resource.excluded_members.nil?
+            common_members = @new_resource.members & @new_resource.excluded_members
+            a.assertion { common_members.empty? }
+            a.failure_message(Chef::Exceptions::ConflictingMembersInGroup, "Attempting to both add and remove users from a group: '#{common_members.join(', ')}'")
+            # No why-run alternative
+          end
+        end
       end
 
       # Check to see if a group needs any changes. Populate
@@ -72,10 +83,9 @@ class Chef
       # <true>:: If a change is required
       # <false>:: If a change is not required
       def compare_group
-        @change_desc = nil
+        @change_desc = [ ]
         if @new_resource.gid != @current_resource.gid
-          @change_desc = "change gid #{@current_resource.gid} to #{@new_resource.gid}"
-          return true
+          @change_desc << "change gid #{@current_resource.gid} to #{@new_resource.gid}"
         end
 
         if(@new_resource.append)
@@ -85,16 +95,25 @@ class Chef
             missing_members << member
           end
           if missing_members.length > 0
-            @change_desc = "add missing member(s): #{missing_members.join(", ")}"
-            return true
+            @change_desc << "add missing member(s): #{missing_members.join(", ")}"
+          end
+
+          members_to_be_removed = []
+          @new_resource.excluded_members.each do |member|
+            if @current_resource.members.include?(member)
+              members_to_be_removed << member
+            end
+          end
+          if members_to_be_removed.length > 0
+            @change_desc << "remove existing member(s): #{members_to_be_removed.join(", ")}"
           end
         else
           if @new_resource.members != @current_resource.members
-            @change_desc = "replace group members with new list of members"
-            return true
+            @change_desc << "replace group members with new list of members"
           end
         end
-        return false
+
+        !@change_desc.empty?
       end
 
       def action_create
@@ -106,7 +125,7 @@ class Chef
           end
         else
           if compare_group
-            converge_by(["alter group #{@new_resource}", @change_desc ]) do
+            converge_by(["alter group #{@new_resource}"] + change_desc) do
               manage_group
               Chef::Log.info("#{@new_resource} altered")
             end
@@ -125,7 +144,7 @@ class Chef
 
       def action_manage
         if @group_exists && compare_group
-          converge_by(["manage group #{@new_resource}", @change_desc]) do
+          converge_by(["manage group #{@new_resource}"] + change_desc) do
             manage_group
             Chef::Log.info("#{@new_resource} managed")
           end
@@ -134,7 +153,7 @@ class Chef
 
       def action_modify
         if compare_group
-          converge_by(["modify group #{@new_resource}", @change_desc]) do
+          converge_by(["modify group #{@new_resource}"] + change_desc) do
             manage_group
             Chef::Log.info("#{@new_resource} modified")
           end
diff --git a/lib/chef/provider/group/dscl.rb b/lib/chef/provider/group/dscl.rb
index d0b2a4d499..c204c09321 100644
--- a/lib/chef/provider/group/dscl.rb
+++ b/lib/chef/provider/group/dscl.rb
@@ -73,14 +73,36 @@ class Chef
         end
 
         def set_members
+          # First reset the memberships if the append is not set
           unless @new_resource.append
             Chef::Log.debug("#{@new_resource} removing group members #{@current_resource.members.join(' ')}") unless @current_resource.members.empty?
             safe_dscl("create /Groups/#{@new_resource.group_name} GroupMembers ''") # clear guid list
             safe_dscl("create /Groups/#{@new_resource.group_name} GroupMembership ''") # clear user list
+            @current_resource.members([ ])
           end
-          unless @new_resource.members.empty?
-            Chef::Log.debug("#{@new_resource} setting group members #{@new_resource.members.join(', ')}")
-            safe_dscl("append /Groups/#{@new_resource.group_name} GroupMembership #{@new_resource.members.join(' ')}")
+
+          # Add any members that need to be added
+          if @new_resource.members && !@new_resource.members.empty?
+            members_to_be_added = [ ]
+            @new_resource.members.each do |member|
+              members_to_be_added << member if !@current_resource.members.include?(member)
+            end
+            unless members_to_be_added.empty?
+              Chef::Log.debug("#{@new_resource} setting group members #{members_to_be_added.join(', ')}")
+              safe_dscl("append /Groups/#{@new_resource.group_name} GroupMembership #{members_to_be_added.join(' ')}")
+            end
+          end
+
+          # Remove any members that need to be removed
+          if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
+            members_to_be_removed = [ ]
+            @new_resource.excluded_members.each do |member|
+              members_to_be_removed << member if @current_resource.members.include?(member)
+            end
+            unless members_to_be_removed.empty?
+              Chef::Log.debug("#{@new_resource} removing group members #{members_to_be_removed.join(', ')}")
+              safe_dscl("delete /Groups/#{@new_resource.group_name} GroupMembership #{members_to_be_removed.join(' ')}")
+            end
           end
         end
 
@@ -110,7 +132,7 @@ class Chef
           if @new_resource.gid && (@current_resource.gid != @new_resource.gid)
             set_gid
           end
-          if @new_resource.members && (@current_resource.members != @new_resource.members)
+          if @new_resource.members || @new_resource.excluded_members
             set_members
           end
         end
diff --git a/lib/chef/provider/group/gpasswd.rb b/lib/chef/provider/group/gpasswd.rb
index 2638b82383..a65a7ffd53 100644
--- a/lib/chef/provider/group/gpasswd.rb
+++ b/lib/chef/provider/group/gpasswd.rb
@@ -39,25 +39,20 @@ class Chef
           end
         end
 
-        def modify_group_members
-            if(@new_resource.append)
-              unless @new_resource.members.empty?
-                @new_resource.members.each do |member|
-                  Chef::Log.debug("#{@new_resource} appending member #{member} to group #{@new_resource.group_name}")
-                  shell_out!("gpasswd -a #{member} #{@new_resource.group_name}")
-                end
-              else
-                Chef::Log.debug("#{@new_resource} not changing group members, the group has no members to add")
-              end
-            else
-              unless @new_resource.members.empty?
-                Chef::Log.debug("#{@new_resource} setting group members to #{@new_resource.members.join(', ')}")
-                shell_out!("gpasswd -M #{@new_resource.members.join(',')} #{@new_resource.group_name}")
-              else
-                Chef::Log.debug("#{@new_resource} setting group members to: none")
-                shell_out!("gpasswd -M \"\" #{@new_resource.group_name}")
-              end
-            end
+        def set_members(members)
+          unless members.empty?
+            shell_out!("gpasswd -M #{members.join(',')} #{@new_resource.group_name}")
+          else
+            shell_out!("gpasswd -M \"\" #{@new_resource.group_name}")
+          end
+        end
+
+        def add_member(member)
+          shell_out!("gpasswd -a #{member} #{@new_resource.group_name}")
+        end
+
+        def remove_member(member)
+          shell_out!("gpasswd -d #{member} #{@new_resource.group_name}")
         end
       end
     end
diff --git a/lib/chef/provider/group/groupadd.rb b/lib/chef/provider/group/groupadd.rb
index 45ae308612..cb480aab54 100644
--- a/lib/chef/provider/group/groupadd.rb
+++ b/lib/chef/provider/group/groupadd.rb
@@ -65,8 +65,48 @@ class Chef
         end
 
         def modify_group_members
-          raise Chef::Exceptions::Group, "you must override modify_group_members in #{self.to_s}"
+          if @new_resource.append
+            if @new_resource.members && !@new_resource.members.empty?
+              members_to_be_added = [ ]
+              @new_resource.members.each do |member|
+                members_to_be_added << member if !@current_resource.members.include?(member)
+              end
+              members_to_be_added.each do |member|
+                Chef::Log.debug("#{@new_resource} appending member #{member} to group #{@new_resource.group_name}")
+                add_member(member)
+              end
+            end
+
+            if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
+              members_to_be_removed = [ ]
+              @new_resource.excluded_members.each do |member|
+                members_to_be_removed << member if @current_resource.members.include?(member)
+              end
+
+              members_to_be_removed.each do |member|
+                Chef::Log.debug("#{@new_resource} removing member #{member} from group #{@new_resource.group_name}")
+                remove_member(member)
+              end
+            end
+          else
+            members_description = @new_resource.members.empty? ? "none" : @new_resource.members.join(", ")
+            Chef::Log.debug("#{@new_resource} setting group members to: #{members_description}")
+            set_members(@new_resource.members)
+          end
+        end
+
+        def add_member(member)
+          raise Chef::Exceptions::Group, "you must override add_member in #{self.to_s}"
+        end
+
+        def remove_member(member)
+          raise Chef::Exceptions::Group, "you must override remove_member in #{self.to_s}"
+        end
+
+        def set_members(members)
+          raise Chef::Exceptions::Group, "you must override set_members in #{self.to_s}"
         end
+
         # Little bit of magic as per Adam's useradd provider to pull the assign the command line flags
         #
         # ==== Returns
diff --git a/lib/chef/provider/group/groupmod.rb b/lib/chef/provider/group/groupmod.rb
index 10fc680d78..c8b6458db0 100644
--- a/lib/chef/provider/group/groupmod.rb
+++ b/lib/chef/provider/group/groupmod.rb
@@ -44,45 +44,40 @@ class Chef
         # Manage the group when it already exists
         def manage_group
           if @new_resource.append
-            to_add = @new_resource.members.dup
-            to_add.reject! { |user| @current_resource.members.include?(user) }
-
-            to_delete = Array.new
-
-            Chef::Log.debug("#{@new_resource} not changing group members, the group has no members to add") if to_add.empty?
+            members_to_be_added = [ ]
+            if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
+              # First find out if any member needs to be removed
+              members_to_be_removed = [ ]
+              @new_resource.excluded_members.each do |member|
+                members_to_be_removed << member if @current_resource.members.include?(member)
+              end
+
+              unless members_to_be_removed.empty?
+                # We are using a magic trick to remove the groups.
+                reset_group_membership
+
+                # Capture the members we need to add in
+                # members_to_be_added to be added later on.
+                @current_resource.members.each do |member|
+                  members_to_be_added << member unless members_to_be_removed.include?(member)
+                end
+              end
+            end
+
+            if @new_resource.members && !@new_resource.members.empty?
+              @new_resource.members.each do |member|
+                members_to_be_added << member if !@current_resource.members.include?(member)
+              end
+            end
+
+            Chef::Log.debug("#{@new_resource} not changing group members, the group has no members to add") if members_to_be_added.empty?
+
+            add_group_members(members_to_be_added)
           else
-            to_add = @new_resource.members.dup
-            to_add.reject! { |user| @current_resource.members.include?(user) }
-
-            to_delete = @current_resource.members.dup
-            to_delete.reject! { |user| @new_resource.members.include?(user) }
-
+            # We are resetting the members of a group so use the same trick
+            reset_group_membership
             Chef::Log.debug("#{@new_resource} setting group members to: none") if @new_resource.members.empty?
-          end
-
-          if to_delete.empty?
-            # If we are only adding new members to this group, then
-            # call add_group_members with only those users
-            add_group_members(to_add)
-          else
-            Chef::Log.debug("#{@new_resource} removing members #{to_delete.join(', ')}")
-
-            # This is tricky, but works: rename the existing group to
-            # "<name>_bak", create a new group with the same GID and
-            # "<name>", then set correct members on that group
-            rename = "group mod -n #{@new_resource.group_name}_bak #{@new_resource.group_name}"
-            shell_out!(rename)
-
-            create = "group add"
-            create << set_options(:overwrite_gid => true)
-            shell_out!(create)
-
-            # Ignore to_add here, since we're replacing the group we
-            # have to add all members who should be in the group.
             add_group_members(@new_resource.members)
-
-            remove = "group del #{@new_resource.group_name}_bak"
-            shell_out!(remove)
           end
         end
 
@@ -99,6 +94,21 @@ class Chef
           end
         end
 
+        # This is tricky, but works: rename the existing group to
+        # "<name>_bak", create a new group with the same GID and
+        # "<name>", then set correct members on that group
+        def reset_group_membership
+          rename = "group mod -n #{@new_resource.group_name}_bak #{@new_resource.group_name}"
+          shell_out!(rename)
+
+          create = "group add"
+          create << set_options(:overwrite_gid => true)
+          shell_out!(create)
+
+          remove = "group del #{@new_resource.group_name}_bak"
+          shell_out!(remove)
+        end
+
         # Little bit of magic as per Adam's useradd provider to pull and assign the command line flags
         #
         # ==== Returns
diff --git a/lib/chef/provider/group/pw.rb b/lib/chef/provider/group/pw.rb
index 66da8281be..3ec6f6f668 100644
--- a/lib/chef/provider/group/pw.rb
+++ b/lib/chef/provider/group/pw.rb
@@ -39,16 +39,28 @@ class Chef
         def create_group
           command = "pw groupadd"
           command << set_options
-          command << set_members_option
-          run_command(:command => command)
+          member_options = set_members_options
+          if member_options.empty?
+            run_command(:command => command)
+          else
+            member_options.each do |option|
+              run_command(:command => command + option)
+            end
+          end
         end
 
         # Manage the group when it already exists
         def manage_group
           command = "pw groupmod"
           command << set_options
-          command << set_members_option
-          run_command(:command => command)
+          member_options = set_members_options
+          if member_options.empty?
+            run_command(:command => command)
+          else
+            member_options.each do |option|
+              run_command(:command => command + option)
+            end
+          end
         end
 
         # Remove the group
@@ -70,21 +82,52 @@ class Chef
         end
 
         # Set the membership option depending on the current resource states
-        def set_members_option
-          opt = ""
-          unless @new_resource.members.empty?
-            opt << " -M #{@new_resource.members.join(',')}"
-            Chef::Log.debug("#{@new_resource} setting group members to #{@new_resource.members.join(', ')}")
+        def set_members_options
+          opts = [ ]
+          members_to_be_added = [ ]
+          members_to_be_removed = [ ]
+
+          if @new_resource.append
+            # Append is set so we will only add members given in the
+            # members list and remove members given in the
+            # excluded_members list.
+            if @new_resource.members && !@new_resource.members.empty?
+              @new_resource.members.each do |member|
+                members_to_be_added << member if !@current_resource.members.include?(member)
+              end
+            end
+
+            if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
+              @new_resource.excluded_members.each do |member|
+                members_to_be_removed << member if @current_resource.members.include?(member)
+              end
+            end
           else
-            # New member list is empty so we should delete any old group members
-            unless @current_resource.members.empty?
-              opt << " -d #{@current_resource.members.join(',')}"
-              Chef::Log.debug("#{@new_resource} removing group members #{@current_resource.members.join(', ')}")
-            else
-              Chef::Log.debug("#{@new_resource} not changing group members, the group has no members")
+            # Append is not set so we're resetting the membership of
+            # the group to the given members.
+            members_to_be_added = @new_resource.members
+            @current_resource.members.each do |member|
+              # No need to re-add a member if it's present in the new
+              # list of members
+              if members_to_be_added.include? member
+                members_to_be_added.delete member
+              else
+                members_to_be_removed << member
+              end
             end
           end
-          opt
+
+          unless members_to_be_added.empty?
+            Chef::Log.debug("#{@new_resource} adding group members: #{members_to_be_added.join(',')}")
+            opts << " -m #{members_to_be_added.join(',')}"
+          end
+
+          unless members_to_be_removed.empty?
+            Chef::Log.debug("#{@new_resource} removing group members: #{members_to_be_removed.join(',')}")
+            opts << " -d #{members_to_be_removed.join(',')}"
+          end
+
+          opts
         end
 
       end
diff --git a/lib/chef/provider/group/suse.rb b/lib/chef/provider/group/suse.rb
index 4c343bddf9..14380f1705 100644
--- a/lib/chef/provider/group/suse.rb
+++ b/lib/chef/provider/group/suse.rb
@@ -39,21 +39,24 @@ class Chef
           end
         end
 
-        def modify_group_members
-          unless @new_resource.members.empty?
-            if(@new_resource.append)
-              @new_resource.members.each do |member|
-                Chef::Log.debug("#{@new_resource} appending member #{member} to group #{@new_resource.group_name}")
-                shell_out!("groupmod -A #{member} #{@new_resource.group_name}")
-              end
-            else
-              Chef::Log.debug("#{@new_resource} setting group members to #{@new_resource.members.join(', ')}")
-              shell_out!("groupmod -A #{@new_resource.members.join(',')} #{@new_resource.group_name}")
-            end
-          else
-            Chef::Log.debug("#{@new_resource} not changing group members, the group has no members")
+        def set_members(members)
+          unless @current_resource.members.empty?
+            shell_out!("groupmod -R #{@current_resource.members.join(',')} #{@new_resource.group_name}")
           end
+
+          unless members.empty?
+            shell_out!("groupmod -A #{members.join(',')} #{@new_resource.group_name}")
+          end
+        end
+
+        def add_member(member)
+          shell_out!("groupmod -A #{member} #{@new_resource.group_name}")
         end
+
+        def remove_member(member)
+          shell_out!("groupmod -R #{member} #{@new_resource.group_name}")
+        end
+
       end
     end
   end
diff --git a/lib/chef/provider/group/usermod.rb b/lib/chef/provider/group/usermod.rb
index 5788ac8fad..8323c3e9ba 100644
--- a/lib/chef/provider/group/usermod.rb
+++ b/lib/chef/provider/group/usermod.rb
@@ -17,12 +17,15 @@
 #
 
 require 'chef/provider/group/groupadd'
+require 'chef/mixin/shell_out'
 
 class Chef
   class Provider
     class Group
       class Usermod < Chef::Provider::Group::Groupadd
 
+        include Chef::Mixin::ShellOut
+
         def load_current_resource
           super
         end
@@ -36,32 +39,52 @@ class Chef
             # No whyrun alternative: this component should be available in the base install of any given system that uses it
           end
 
-          requirements.assert(:modify, :create) do |a|
+          requirements.assert(:modify, :manage) do |a|
             a.assertion { @new_resource.members.empty? || @new_resource.append }
             a.failure_message Chef::Exceptions::Group, "setting group members directly is not supported by #{self.to_s}, must set append true in group"
             # No whyrun alternative - this action is simply not supported.
           end
-        end
 
-        def modify_group_members
-          case node[:platform]
-          when "openbsd", "netbsd", "aix", "solaris2", "smartos"
-            append_flags = "-G"
-          when "solaris", "suse", "opensuse"
-            append_flags = "-a -G"
+          requirements.assert(:all_actions) do |a|
+            a.assertion { @new_resource.excluded_members.empty? }
+            a.failure_message Chef::Exceptions::Group, "excluded_members is not supported by #{self.to_s}"
+            # No whyrun alternative - this action is simply not supported.
           end
+        end
 
-          unless @new_resource.members.empty?
-            if(@new_resource.append)
-              @new_resource.members.each do |member|
-                Chef::Log.debug("#{@new_resource} appending member #{member} to group #{@new_resource.group_name}")
-                run_command(:command => "usermod #{append_flags} #{@new_resource.group_name} #{member}" )
-              end
+        def set_members(members)
+          return if members.empty?
+          # This provider only supports adding members with
+          # append. Only if the action is create we will go
+          # ahead and add members.
+          if @new_resource.action == :create
+            members.each do |member|
+              add_member(member)
             end
           else
-            Chef::Log.debug("#{@new_resource} not changing group members, the group has no members")
+            raise Chef::Exceptions::UnsupportedAction, "Setting members directly is not supported by #{self.to_s}"
           end
         end
+
+        def add_member(member)
+          shell_out!("usermod #{append_flags} #{@new_resource.group_name} #{member}")
+        end
+
+        def remove_member(member)
+          # This provider only supports adding members with
+          # append. This function should never be called.
+          raise Chef::Exceptions::UnsupportedAction, "Removing members members is not supported by #{self.to_s}"
+        end
+
+        def append_flags
+          case node[:platform]
+          when "openbsd", "netbsd", "aix", "solaris2", "smartos"
+            "-G"
+          when "solaris", "suse", "opensuse"
+            "-a -G"
+          end
+        end
+
       end
     end
   end
diff --git a/lib/chef/provider/group/windows.rb b/lib/chef/provider/group/windows.rb
index da12366329..cb0ab76927 100644
--- a/lib/chef/provider/group/windows.rb
+++ b/lib/chef/provider/group/windows.rb
@@ -57,13 +57,20 @@ class Chef
 
         def manage_group
           if @new_resource.append
-            begin
-              #ERROR_MEMBER_IN_ALIAS if a member already exists in the group
-              @net_group.local_add_members(@new_resource.members)
-            rescue
-              members = @new_resource.members + @current_resource.members
-              @net_group.local_set_members(members.uniq)
+            members_to_be_added = [ ]
+            @new_resource.members.each do |member|
+              members_to_be_added << member if !@current_resource.members.include?(member)
             end
+
+            # local_add_members will raise ERROR_MEMBER_IN_ALIAS if a
+            # member already exists in the group.
+            @net_group.local_add_members(members_to_be_added) unless members_to_be_added.empty?
+
+            members_to_be_removed = [ ]
+            @new_resource.excluded_members.each do |member|
+              members_to_be_removed << member if @current_resource.members.include?(member)
+            end
+            @net_group.local_delete_members(members_to_be_removed) unless members_to_be_removed.empty?
           else
             @net_group.local_set_members(@new_resource.members)
           end
diff --git a/lib/chef/resource/group.rb b/lib/chef/resource/group.rb
index 17f14c8387..daf851fac6 100644
--- a/lib/chef/resource/group.rb
+++ b/lib/chef/resource/group.rb
@@ -31,6 +31,7 @@ class Chef
         @group_name = name
         @gid = nil
         @members = []
+        @excluded_members = []
         @action = :create
         @append = false
         @non_unique = false
@@ -64,6 +65,16 @@ class Chef
 
       alias_method :users, :members
 
+      def excluded_members(arg=nil)
+        converted_members = arg.is_a?(String) ? [].push(arg) : arg
+        set_or_return(
+          :excluded_members,
+          converted_members,
+          :kind_of => [ Array ]
+        )
+      end
+
+
       def append(arg=nil)
         set_or_return(
           :append,
diff --git a/lib/chef/util/windows/net_group.rb b/lib/chef/util/windows/net_group.rb
index 817e47efa8..e5d78614db 100644
--- a/lib/chef/util/windows/net_group.rb
+++ b/lib/chef/util/windows/net_group.rb
@@ -63,7 +63,7 @@ class Chef::Util::Windows::NetGroup < Chef::Util::Windows
         members = 0.chr * (nread * (PTR_SIZE * 3)) #nread * sizeof(LOCALGROUP_MEMBERS_INFO_1)
         memcpy(members, ptr, members.size)
 
-        #3 pointer fields in LOCALGROUP_MEMBERS_INFO_1, offset 2*PTR_SIZE is lgrmi1_name
+        # 3 pointer fields in LOCALGROUP_MEMBERS_INFO_1, offset 2*PTR_SIZE is lgrmi1_name
         nread.times do |i|
           offset = (i * 3) + 2
           member = lpwstr_to_s(members, offset)
@@ -92,6 +92,10 @@ class Chef::Util::Windows::NetGroup < Chef::Util::Windows
     modify_members(members, NetLocalGroupAddMembers)
   end
 
+  def local_delete_members(members)
+    modify_members(members, NetLocalGroupDelMembers)
+  end
+
   def local_delete
     rc = NetLocalGroupDel.call(nil, @name)
     if rc != NERR_Success
diff --git a/spec/functional/resource/group_spec.rb b/spec/functional/resource/group_spec.rb
index 2c9a568979..b8369d087b 100644
--- a/spec/functional/resource/group_spec.rb
+++ b/spec/functional/resource/group_spec.rb
@@ -20,33 +20,36 @@
 require 'spec_helper'
 require 'functional/resource/base'
 
-describe Chef::Resource::Group, :requires_root_or_running_windows do
-
-  def group_should_exist(resource)
+# Chef::Resource::Group are turned off on Mac OS X 10.6 due to caching
+# issues around Etc.getgrnam() not picking up the group membership
+# changes that are done on the system. Etc.endgrent is not functioning
+# correctly on certain 10.6 boxes.
+describe Chef::Resource::Group, :requires_root_or_running_windows, :not_supported_on_mac_osx_106 do
+  def group_should_exist(group)
     case ohai[:platform_family]
     when "debian", "fedora", "rhel", "suse", "gentoo", "slackware", "arch"
-      expect { Etc::getgrnam(resource.name) }.to_not raise_error(ArgumentError, "can't find group for #{resource.name}")
-      expect(resource.name).to eq(Etc::getgrnam(resource.name).name)
+      expect { Etc::getgrnam(group) }.to_not raise_error(ArgumentError, "can't find group for #{group}")
+      expect(group).to eq(Etc::getgrnam(group).name)
     when "windows"
-      expect { Chef::Util::Windows::NetGroup.new(resource.group_name).local_get_members }.to_not raise_error(ArgumentError, "The group name could not be found.")
+      expect { Chef::Util::Windows::NetGroup.new(group).local_get_members }.to_not raise_error(ArgumentError, "The group name could not be found.")
     end
   end
 
-  def user_exist_in_group?(resource, user)
+  def user_exist_in_group?(user)
     case ohai[:platform_family]
-    when "debian", "fedora", "rhel", "suse", "gentoo", "slackware", "arch"
-      Etc::getgrnam(resource.name).mem.include?(user)
     when "windows"
-      Chef::Util::Windows::NetGroup.new(resource.group_name).local_get_members.include?(user)
+      Chef::Util::Windows::NetGroup.new(group_name).local_get_members.include?(user)
+    else
+      Etc::getgrnam(group_name).mem.include?(user)
     end
   end
 
-  def group_should_not_exist(resource)
+  def group_should_not_exist(group)
     case ohai[:platform_family]
     when "debian", "fedora", "rhel", "suse", "gentoo", "slackware", "arch"
-      expect { Etc::getgrnam(resource.name) }.to raise_error(ArgumentError, "can't find group for #{resource.name}")
+      expect { Etc::getgrnam(group) }.to raise_error(ArgumentError, "can't find group for #{group}")
     when "windows"
-      expect { Chef::Util::Windows::NetGroup.new(resource.group_name).local_get_members }.to raise_error(ArgumentError, "The group name could not be found.")
+      expect { Chef::Util::Windows::NetGroup.new(group).local_get_members }.to raise_error(ArgumentError, "The group name could not be found.")
     end
   end
 
@@ -54,151 +57,287 @@ describe Chef::Resource::Group, :requires_root_or_running_windows do
     return resource.gid == Etc::getgrnam(resource.name).gid if unix?
   end
 
-  def get_user_resource(username)
+  def user(username)
     usr = Chef::Resource::User.new("#{username}", run_context)
-    usr.password("Jetsream123!")
+    if ohai[:platform_family] == "windows"
+      usr.password("ComplexPass11!")
+    end
     usr
   end
 
   def create_user(username)
-    get_user_resource(username).run_action(:create)
+    user(username).run_action(:create)
+    # TODO: User shouldn't exist
   end
 
   def remove_user(username)
-    get_user_resource(username).run_action(:remove)
+    user(username).run_action(:remove)
+    # TODO: User shouldn't exist
   end
 
-  before do
-    @grp_resource = Chef::Resource::Group.new("test-group-#{SecureRandom.random_number(9999)}", run_context)
-  end
+  shared_examples_for "correct group management" do
+    def add_members_to_group(members)
+      temp_resource = group_resource.dup
+      temp_resource.members(members)
+      temp_resource.excluded_members([ ])
+      temp_resource.append(true)
+      temp_resource.run_action(:modify)
+      members.each do |member|
+        user_exist_in_group?(member).should == true
+      end
+    end
 
-  context "group create action" do
-    after(:each) do
-      @grp_resource.run_action(:remove)
+    def create_group
+      temp_resource = group_resource.dup
+      temp_resource.members([ ])
+      temp_resource.excluded_members([ ])
+      temp_resource.run_action(:create)
+      group_should_exist(group_name)
+      included_members.each do |member|
+        user_exist_in_group?(member).should == false
+      end
     end
 
-    it "create a group" do
-      @grp_resource.run_action(:create)
-      group_should_exist(@grp_resource)
+    before(:each) do
+      create_group
     end
 
-    context "group name with 256 characters", :windows_only do
-      before(:each) do
-        grp_name = "theoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestree"
-        @new_grp = Chef::Resource::Group.new(grp_name, run_context)
+    after(:each) do
+      group_resource.run_action(:remove)
+      group_should_not_exist(group_name)
+    end
+
+    describe "when append is not set" do
+      let(:included_members) { ["spec-Eric"] }
+
+      before do
+        create_user("spec-Eric")
+        create_user("spec-Gordon")
+        add_members_to_group(["spec-Gordon"])
       end
+
       after do
-        @new_grp.run_action(:remove)
+        remove_user("spec-Eric")
+        remove_user("spec-Gordon")
       end
-      it " create a group" do
-        @new_grp.run_action(:create)
-        group_should_exist(@new_grp)
+
+      it "should remove the existing users and add the new users to the group" do
+        group_resource.run_action(tested_action)
+
+        user_exist_in_group?("spec-Eric").should == true
+        user_exist_in_group?("spec-Gordon").should == false
       end
     end
-    context "group name with more than 256 characters", :windows_only do
+
+    describe "when append is set" do
       before(:each) do
-        grp_name = "theoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreeQQQQQQQQQQQQQQQQQ"
-        @new_grp = Chef::Resource::Group.new(grp_name, run_context)
+        group_resource.append(true)
       end
-      it " not create a group" do
-        expect { @new_grp.run_action(:create) }.to raise_error
-        group_should_not_exist(@new_grp)
+
+      describe "when the users exist" do
+        before do
+          (included_members + excluded_members).each do |member|
+            create_user(member)
+          end
+        end
+
+        after do
+          (included_members + excluded_members).each do |member|
+            remove_user(member)
+          end
+        end
+
+        it "should add included members to the group" do
+          group_resource.run_action(tested_action)
+
+          included_members.each do |member|
+            user_exist_in_group?(member).should == true
+          end
+          excluded_members.each do |member|
+            user_exist_in_group?(member).should == false
+          end
+        end
+
+        describe "when group contains some users" do
+          before(:each) do
+            add_members_to_group([ "spec-Gordon", "spec-Anthony" ])
+          end
+
+          it "should add the included users and remove excluded users" do
+            group_resource.run_action(tested_action)
+
+            included_members.each do |member|
+              user_exist_in_group?(member).should == true
+            end
+            excluded_members.each do |member|
+              user_exist_in_group?(member).should == false
+            end
+          end
+        end
+      end
+
+      describe "when the users doesn't exist" do
+        describe "when append is not set" do
+          it "should raise an error" do
+            lambda { @grp_resource.run_action(tested_action) }.should raise_error
+          end
+        end
+
+        describe "when append is set" do
+          it "should raise an error" do
+            lambda { @grp_resource.run_action(tested_action) }.should raise_error
+          end
+        end
       end
     end
   end
 
-  context "group remove action" do
-    before(:each) do
-      @grp_resource.run_action(:create)
+  let(:group_name) { "cheftest-#{SecureRandom.random_number(9999)}" }
+  let(:included_members) { nil }
+  let(:excluded_members) { nil }
+  let(:group_resource) {
+    group = Chef::Resource::Group.new(group_name, run_context)
+    group.members(included_members)
+    group.excluded_members(excluded_members)
+    group
+  }
+
+  it "append should be false by default" do
+    group_resource.append.should == false
+  end
+
+  describe "group create action" do
+    after(:each) do
+      group_resource.run_action(:remove)
+      group_should_not_exist(group_name)
+    end
+
+    it "should create a group" do
+      group_resource.run_action(:create)
+      group_should_exist(group_name)
+    end
+
+    describe "when group name is length 256", :windows_only do
+      let!(:group_name) { "theoldmanwalkingdownthestreetalwayshadagood\
+smileonhisfacetheoldmanwalkingdownthestreetalwayshadagoodsmileonhisface\
+theoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalking\
+downthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestree" }
+
+      it "should create a group" do
+        group_resource.run_action(:create)
+        group_should_exist(group_name)
+      end
     end
 
-    it "remove a group" do
-      @grp_resource.run_action(:remove)
-      group_should_not_exist(@grp_resource)
+    describe "when group name length is more than 256", :windows_only do
+      let!(:group_name) { "theoldmanwalkingdownthestreetalwayshadagood\
+smileonhisfacetheoldmanwalkingdownthestreetalwayshadagoodsmileonhisface\
+theoldmanwalkingdownthestreetalwayshadagoodsmileonhisfacetheoldmanwalking\
+downthestreetalwayshadagoodsmileonhisfacetheoldmanwalkingdownthestreeQQQQQQ" }
+
+      it "should not create a group" do
+        lambda { group_resource.run_action(:create) }.should raise_error
+        group_should_not_exist(group_name)
+      end
+    end
+
+    # not_supported_on_solaris because of the use of excluded_members
+    describe "should raise an error when same member is included in the members and excluded_members", :not_supported_on_solaris do
+      it "should raise an error" do
+        invalid_resource = group_resource.dup
+        invalid_resource.members(["Jack"])
+        invalid_resource.excluded_members(["Jack"])
+        lambda { invalid_resource.run_action(:create)}.should raise_error(Chef::Exceptions::ConflictingMembersInGroup)
+      end
     end
   end
 
-  context "group modify action", :unsupported_group_provider_platform do
-    before(:each) do
-      @grp_resource.run_action(:create)
+  describe "group remove action" do
+    describe "when there is a group" do
+      before do
+        group_resource.run_action(:create)
+        group_should_exist(group_name)
+      end
+
+      it "should remove a group" do
+        group_resource.run_action(:remove)
+        group_should_not_exist(group_name)
+      end
     end
 
-    after(:each) do
-      @grp_resource.run_action(:remove)
-    end
-
-    it "add user to group" do
-      user1 = "user1-#{SecureRandom.random_number(9999)}"
-      user2 = "user2-#{SecureRandom.random_number(9999)}"
-
-      create_user(user1)
-      @grp_resource.members(user1)
-      expect(user_exist_in_group?(@grp_resource, user1)).to be_false
-      @grp_resource.run_action(:modify)
-      group_should_exist(@grp_resource)
-      expect(user_exist_in_group?(@grp_resource, user1)).to be_true
-
-      create_user(user2)
-      expect(user_exist_in_group?(@grp_resource, user2)).to be_false
-      @grp_resource.members(user2)
-      @grp_resource.run_action(:modify)
-      group_should_exist(@grp_resource)
-
-      #default append is false, so modify action remove old member user1 from group and add new member user2
-      expect(user_exist_in_group?(@grp_resource, user1)).to be_false
-      expect(user_exist_in_group?(@grp_resource, user2)).to be_true
-      remove_user(user1)
-      remove_user(user2)
-    end
-
-
-    it "append user to a group" do
-      user1 = "user1-#{SecureRandom.random_number(9999)}"
-      user2 = "user2-#{SecureRandom.random_number(9999)}"
-      create_user(user1)
-      @grp_resource.members(user1)
-      expect(user_exist_in_group?(@grp_resource, user1)).to be_false
-      #default append attribute is false
-      @grp_resource.run_action(:modify)
-      group_should_exist(@grp_resource)
-      expect(user_exist_in_group?(@grp_resource, user1)).to be_true
-      #set append attribute to true
-      @grp_resource.append(true)
-      create_user(user2)
-      expect(user_exist_in_group?(@grp_resource, user2)).to be_false
-      @grp_resource.members(user2)
-      @grp_resource.run_action(:modify)
-      group_should_exist(@grp_resource)
-      expect(user_exist_in_group?(@grp_resource, user1)).to be_true
-      expect(user_exist_in_group?(@grp_resource, user2)).to be_true
-      remove_user(user1)
-      remove_user(user2)
-    end
-
-    it "raise error on add non-existent user to group" do
-      user1 = "user1-#{SecureRandom.random_number(9999)}"
-      @grp_resource.members(user1)
-      @grp_resource.append(true)
-      expect(user_exist_in_group?(@grp_resource, user1)).to be_false
-      expect { @grp_resource.run_action(:modify) }.to raise_error
+    describe "when there is no group" do
+      it "should be no-op" do
+        group_resource.run_action(:remove)
+        group_should_not_exist(group_name)
+      end
     end
   end
 
-  context "group manage action", :unix_only, :unsupported_group_provider_platform do
-    before(:each) do
-      @grp_resource.run_action(:create)
+  describe "group modify action", :not_supported_on_solaris do
+    let(:included_members) { ["spec-Gordon", "spec-Eric"] }
+    let(:excluded_members) { ["spec-Anthony"] }
+    let(:tested_action) { :modify }
+
+    describe "when there is no group" do
+      it "should raise an error" do
+        lambda { group_resource.run_action(:modify) }.should raise_error
+      end
     end
 
-    after(:each) do
-      @grp_resource.run_action(:remove)
+    describe "when there is a group" do
+      it_behaves_like "correct group management"
+    end
+  end
+
+  describe "group manage action", :not_supported_on_solaris do
+    let(:included_members) { ["spec-Gordon", "spec-Eric"] }
+    let(:excluded_members) { ["spec-Anthony"] }
+    let(:tested_action) { :manage }
+
+    describe "when there is no group" do
+      it "should raise an error" do
+        lambda { group_resource.run_action(:manage) }.should_not raise_error
+        group_should_not_exist(group_name)
+      end
     end
 
-    it "change gid of the group" do
-      grp_id = 1234567890
-      @grp_resource.gid(grp_id)
-      @grp_resource.run_action(:manage)
-      group_should_exist(@grp_resource)
-      expect(compare_gid(@grp_resource, grp_id)).to be_true
+    describe "when there is a group" do
+      it_behaves_like "correct group management"
+    end
+  end
+
+  describe "group resource with Usermod provider", :solaris_only do
+    describe "when excluded_members is set" do
+      let(:excluded_members) { ["spec-Anthony"] }
+
+      it ":manage should raise an error" do
+        lambda {group_resource.run_action(:manage) }.should raise_error
+      end
+
+      it ":modify should raise an error" do
+        lambda {group_resource.run_action(:modify) }.should raise_error
+      end
+
+      it ":create should raise an error" do
+        lambda {group_resource.run_action(:create) }.should raise_error
+      end
+    end
+
+    describe "when append is not set" do
+      let(:included_members) { ["spec-Gordon", "spec-Eric"] }
+
+      before(:each) do
+        group_resource.append(false)
+      end
+
+      it ":manage should raise an error" do
+        lambda {group_resource.run_action(:manage) }.should raise_error
+      end
+
+      it ":modify should raise an error" do
+        lambda {group_resource.run_action(:modify) }.should raise_error
+      end
     end
   end
 end
+
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 618f782c7c..e60bdfd434 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -102,15 +102,15 @@ RSpec.configure do |config|
 
   # Add jruby filters here
   config.filter_run_excluding :windows_only => true unless windows?
+  config.filter_run_excluding :not_supported_on_mac_osx_106 => true if mac_osx_106?
   config.filter_run_excluding :not_supported_on_win2k3 => true if windows_win2k3?
   config.filter_run_excluding :not_supported_on_solaris => true if solaris?
   config.filter_run_excluding :win2k3_only => true unless windows_win2k3?
   config.filter_run_excluding :windows64_only => true unless windows64?
   config.filter_run_excluding :windows32_only => true unless windows32?
+  config.filter_run_excluding :solaris_only => true unless solaris?
   config.filter_run_excluding :system_windows_service_gem_only => true unless system_windows_service_gem?
   config.filter_run_excluding :unix_only => true unless unix?
-  # Remove this filter once these issues are fixed: OC-9764, OC-9765, OC-9766, OC-9767
-  config.filter_run_excluding :unsupported_group_provider_platform => true if (os_x? or solaris? or freebsd? or suse?)
   config.filter_run_excluding :supports_cloexec => true unless supports_cloexec?
   config.filter_run_excluding :selinux_only => true unless selinux_enabled?
   config.filter_run_excluding :ruby_18_only => true unless ruby_18?
@@ -118,9 +118,9 @@ RSpec.configure do |config|
   config.filter_run_excluding :ruby_gte_19_only => true unless ruby_gte_19?
   config.filter_run_excluding :ruby_20_only => true unless ruby_20?
   config.filter_run_excluding :ruby_gte_20_only => true unless ruby_gte_20?
-  config.filter_run_excluding :requires_root => true unless ENV['USER'] == 'root' || ENV['LOGIN'] == 'root'
-  config.filter_run_excluding :requires_root_or_running_windows => true unless (ENV['USER'] == 'root' or windows?)
-  config.filter_run_excluding :requires_unprivileged_user => true if ENV['USER'] == 'root'
+  config.filter_run_excluding :requires_root => true unless root?
+  config.filter_run_excluding :requires_root_or_running_windows => true unless (root? || windows?)
+  config.filter_run_excluding :requires_unprivileged_user => true if root?
   config.filter_run_excluding :uses_diff => true unless has_diff?
 
   running_platform_arch = `uname -m`.strip
diff --git a/spec/support/platform_helpers.rb b/spec/support/platform_helpers.rb
index 1501a162e0..66f1fd4b25 100644
--- a/spec/support/platform_helpers.rb
+++ b/spec/support/platform_helpers.rb
@@ -1,4 +1,7 @@
 require 'fcntl'
+require 'chef/mixin/shell_out'
+
+include Chef::Mixin::ShellOut
 
 def ruby_gte_20?
   RUBY_VERSION.to_f >= 2.0
@@ -32,6 +35,19 @@ def windows_win2k3?
   (host.version && host.version.start_with?("5.2"))
 end
 
+def mac_osx_106?
+  if File.exists? "/usr/bin/sw_vers"
+    result = shell_out("/usr/bin/sw_vers")
+    result.stdout.each_line do |line|
+      if line =~ /^ProductVersion:\s10.6.*$/
+        return true
+      end
+    end
+  end
+
+  false
+end
+
 # detects if the hardware is 64-bit (evaluates to true in "WOW64" mode in a 32-bit app on a 64-bit system)
 def windows64?
   windows? && ( ENV['PROCESSOR_ARCHITECTURE'] == 'AMD64' || ENV['PROCESSOR_ARCHITEW6432'] == 'AMD64' )
@@ -95,3 +111,8 @@ end
 def suse?
   File.exists?("/etc/SuSE-release")
 end
+
+def root?
+  return false if windows?
+  Process.euid == 0
+end
diff --git a/spec/unit/provider/group/dscl_spec.rb b/spec/unit/provider/group/dscl_spec.rb
index 28d78cbdbd..360a799b5f 100644
--- a/spec/unit/provider/group/dscl_spec.rb
+++ b/spec/unit/provider/group/dscl_spec.rb
@@ -265,6 +265,7 @@ describe Chef::Provider::Group::Dscl do
     it "should manage the group_name if it changed and the new resources group_name is not null" do
       @current_resource.group_name("oldval")
       @new_resource.group_name("newname")
+      @provider.should_receive(:set_members).and_return(true)
       @provider.should_receive(:safe_dscl).with("create /Groups/newname")
       @provider.should_receive(:safe_dscl).with("create /Groups/newname Password '*'")
       @provider.manage_group
diff --git a/spec/unit/provider/group/gpasswd_spec.rb b/spec/unit/provider/group/gpasswd_spec.rb
index 8889ba322e..f765667300 100644
--- a/spec/unit/provider/group/gpasswd_spec.rb
+++ b/spec/unit/provider/group/gpasswd_spec.rb
@@ -76,8 +76,7 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
         @new_resource.members([])
       end
 
-      it "logs a message and does not modify group membership" do
-        Chef::Log.should_receive(:debug).with("group[wheel] not changing group members, the group has no members to add")
+      it "does not modify group membership" do
         @provider.should_not_receive(:shell_out!)
         @provider.modify_group_members
       end
@@ -85,7 +84,7 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
 
     describe "when the resource specifies group members" do
       it "should log an appropriate debug message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] setting group members to lobster, rage, fist")
+        Chef::Log.should_receive(:debug).with("group[wheel] setting group members to: lobster, rage, fist")
         @provider.stub!(:shell_out!)
         @provider.modify_group_members
       end
@@ -95,12 +94,20 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
         @provider.modify_group_members
       end
 
-      it "should run gpasswd individually for each user when the append option is set" do
-        @new_resource.append(true)
-        @provider.should_receive(:shell_out!).with("gpasswd -a lobster wheel")
-        @provider.should_receive(:shell_out!).with("gpasswd -a rage wheel")
-        @provider.should_receive(:shell_out!).with("gpasswd -a fist wheel")
-        @provider.modify_group_members
+      describe "when no user exists in the system" do
+        before do
+          current_resource = @new_resource.dup
+          current_resource.members([ ])
+          @provider.current_resource = current_resource
+        end
+
+        it "should run gpasswd individually for each user when the append option is set" do
+          @new_resource.append(true)
+          @provider.should_receive(:shell_out!).with("gpasswd -a lobster wheel")
+          @provider.should_receive(:shell_out!).with("gpasswd -a rage wheel")
+          @provider.should_receive(:shell_out!).with("gpasswd -a fist wheel")
+          @provider.modify_group_members
+        end
       end
 
     end
diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb
index 0cc1167142..91073727c8 100644
--- a/spec/unit/provider/group/groupadd_spec.rb
+++ b/spec/unit/provider/group/groupadd_spec.rb
@@ -143,10 +143,9 @@ describe Chef::Provider::Group::Groupadd do
     end
   end
 
-  describe "modify_group_members" do
-
-    it "should raise an error when calling modify_group_members" do
-      lambda { @provider.modify_group_members }.should raise_error(Chef::Exceptions::Group, "you must override modify_group_members in #{@provider.to_s}")
+  [:add_member, :remove_member, :set_members].each do |m|
+    it "should raise an error when calling #{m}" do
+      lambda { @provider.send(m, [ ]) }.should raise_error(Chef::Exceptions::Group, "you must override #{m} in #{@provider.to_s}")
     end
   end
 
diff --git a/spec/unit/provider/group/groupmod_spec.rb b/spec/unit/provider/group/groupmod_spec.rb
index 69b96c6edb..bfacfa5c26 100644
--- a/spec/unit/provider/group/groupmod_spec.rb
+++ b/spec/unit/provider/group/groupmod_spec.rb
@@ -62,7 +62,6 @@ describe Chef::Provider::Group::Groupmod do
 
         it "logs a message and sets group's members to 'none', then removes existing group members" do
           Chef::Log.should_receive(:debug).with("group[wheel] setting group members to: none")
-          Chef::Log.should_receive(:debug).with("group[wheel] removing members lobster, rage, fist")
           @provider.should_receive(:shell_out!).with("group mod -n wheel_bak wheel")
           @provider.should_receive(:shell_out!).with("group add -g '123' -o wheel")
           @provider.should_receive(:shell_out!).with("group del wheel_bak")
diff --git a/spec/unit/provider/group/pw_spec.rb b/spec/unit/provider/group/pw_spec.rb
index e7c38f9555..a545f8ee70 100644
--- a/spec/unit/provider/group/pw_spec.rb
+++ b/spec/unit/provider/group/pw_spec.rb
@@ -49,7 +49,7 @@ describe Chef::Provider::Group::Pw do
   describe "when creating a group" do
     it "should run pw groupadd with the return of set_options and set_members_option" do
       @new_resource.gid(23)
-      @provider.should_receive(:run_command).with({ :command => "pw groupadd wheel -g '23' -M root,aj" }).and_return(true)
+      @provider.should_receive(:run_command).with({ :command => "pw groupadd wheel -g '23'" }).and_return(true)
       @provider.create_group
     end
   end
@@ -58,7 +58,9 @@ describe Chef::Provider::Group::Pw do
 
     it "should run pw groupmod with the return of set_options" do
       @new_resource.gid(42)
-      @provider.should_receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -M root,aj" }).and_return(true)
+      @new_resource.members(["someone"])
+      @provider.should_receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -m someone" }).and_return(true)
+      @provider.should_receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -d root,aj" }).and_return(true)
       @provider.manage_group
     end
 
@@ -79,13 +81,8 @@ describe Chef::Provider::Group::Pw do
         @current_resource.stub!(:members).and_return([])
       end
 
-      it "should log an appropriate message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] not changing group members, the group has no members")
-        @provider.set_members_option
-      end
-
       it "should set no options" do
-        @provider.set_members_option.should eql("")
+        @provider.set_members_options.should eql([ ])
       end
     end
 
@@ -96,12 +93,12 @@ describe Chef::Provider::Group::Pw do
       end
 
       it "should log an appropriate message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] removing group members all, your, base")
-        @provider.set_members_option
+        Chef::Log.should_receive(:debug).with("group[wheel] removing group members: all,your,base")
+        @provider.set_members_options
       end
 
       it "should set the -d option with the members joined by ','" do
-        @provider.set_members_option.should eql(" -d all,your,base")
+        @provider.set_members_options.should eql([ " -d all,your,base" ])
       end
     end
 
@@ -112,12 +109,12 @@ describe Chef::Provider::Group::Pw do
       end
 
       it "should log an appropriate debug message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] setting group members to all, your, base")
-        @provider.set_members_option
+        Chef::Log.should_receive(:debug).with("group[wheel] adding group members: all,your,base")
+        @provider.set_members_options
       end
 
       it "should set the -M option with the members joined by ','" do
-        @provider.set_members_option.should eql(" -M all,your,base")
+        @provider.set_members_options.should eql([ " -m all,your,base" ])
       end
     end
   end
diff --git a/spec/unit/provider/group/usermod_spec.rb b/spec/unit/provider/group/usermod_spec.rb
index 7f2931f2e2..1687cc0b65 100644
--- a/spec/unit/provider/group/usermod_spec.rb
+++ b/spec/unit/provider/group/usermod_spec.rb
@@ -25,6 +25,7 @@ describe Chef::Provider::Group::Usermod do
     @run_context = Chef::RunContext.new(@node, {}, @events)
     @new_resource = Chef::Resource::Group.new("wheel")
     @new_resource.members [ "all", "your", "base" ]
+    @new_resource.excluded_members [ ]
     @provider = Chef::Provider::Group::Usermod.new(@new_resource, @run_context)
     @provider.stub!(:run_command)
   end
@@ -33,11 +34,12 @@ describe Chef::Provider::Group::Usermod do
 
     describe "with an empty members array" do
       before do
+        @new_resource.stub!(:append).and_return(true)
         @new_resource.stub!(:members).and_return([])
       end
 
       it "should log an appropriate message" do
-        Chef::Log.should_receive(:debug).with("group[wheel] not changing group members, the group has no members")
+        @provider.should_not_receive(:shell_out!)
         @provider.modify_group_members
       end
     end
@@ -65,13 +67,26 @@ describe Chef::Provider::Group::Usermod do
         lambda { @provider.run_action(@provider.process_resource_requirements) }.should raise_error(Chef::Exceptions::Group, "setting group members directly is not supported by #{@provider.to_s}, must set append true in group")
       end
 
+      it "should raise an error when excluded_members are set" do
+        @provider.define_resource_requirements
+        @provider.load_current_resource
+        @provider.instance_variable_set("@group_exists", true)
+        @provider.action = :modify
+        @new_resource.stub!(:append).and_return(true)
+        @new_resource.stub!(:excluded_members).and_return(["someone"])
+        lambda { @provider.run_action(@provider.process_resource_requirements) }.should raise_error(Chef::Exceptions::Group, "excluded_members is not supported by #{@provider.to_s}")
+      end
+
       platforms.each do |platform, flags|
         it "should usermod each user when the append option is set on #{platform}" do
+          current_resource = @new_resource.dup
+          current_resource.members([ ])
+          @provider.current_resource = current_resource
           @node.automatic_attrs[:platform] = platform
           @new_resource.stub!(:append).and_return(true)
-          @provider.should_receive(:run_command).with({:command => "usermod #{flags} wheel all"})
-          @provider.should_receive(:run_command).with({:command => "usermod #{flags} wheel your"})
-          @provider.should_receive(:run_command).with({:command => "usermod #{flags} wheel base"})
+          @provider.should_receive(:shell_out!).with("usermod #{flags} wheel all")
+          @provider.should_receive(:shell_out!).with("usermod #{flags} wheel your")
+          @provider.should_receive(:shell_out!).with("usermod #{flags} wheel base")
           @provider.modify_group_members
         end
       end
diff --git a/spec/unit/provider/group/windows_spec.rb b/spec/unit/provider/group/windows_spec.rb
index a65cfc241d..14a4f4bb40 100644
--- a/spec/unit/provider/group/windows_spec.rb
+++ b/spec/unit/provider/group/windows_spec.rb
@@ -70,14 +70,6 @@ describe Chef::Provider::Group::Windows do
       @provider.manage_group
     end
 
-    it "should call @net_group.local_set_members if append fails" do
-      @new_resource.stub!(:append).and_return(true)
-      @net_group.stub!(:local_add_members).and_raise(ArgumentError)
-      @net_group.should_receive(:local_add_members).with(@new_resource.members)
-      @net_group.should_receive(:local_set_members).with(@new_resource.members + @current_resource.members)
-      @provider.manage_group
-    end
-
   end
 
   describe "remove_group" do
diff --git a/spec/unit/provider/group_spec.rb b/spec/unit/provider/group_spec.rb
index caad9878b6..a076593511 100644
--- a/spec/unit/provider/group_spec.rb
+++ b/spec/unit/provider/group_spec.rb
@@ -108,6 +108,19 @@ describe Chef::Provider::User do
       @provider.compare_group.should be_true
     end
 
+    it "should return false if append is true and excluded_members include a non existing member" do
+      @new_resource.excluded_members << "extra_user"
+      @new_resource.stub!(:append).and_return(true)
+      @provider.compare_group.should be_false
+    end
+
+    it "should return true if the append is true and excluded_members include an existing user" do
+      @new_resource.members.each {|m| @new_resource.excluded_members << m }
+      @new_resource.members.clear
+      @new_resource.stub!(:append).and_return(true)
+      @provider.compare_group.should be_true
+    end
+
   end
 
   describe "when creating a group" do
@@ -127,6 +140,7 @@ describe Chef::Provider::User do
     it "should check to see if the group has mismatched attributes if the group exists" do
       @provider.group_exists = true
       @provider.stub!(:compare_group).and_return(false)
+      @provider.stub!(:change_desc).and_return([ ])
       @provider.run_action(:create)
       @provider.new_resource.should_not be_updated
     end
@@ -134,6 +148,7 @@ describe Chef::Provider::User do
     it "should call manage_group if the group exists and has mismatched attributes" do
       @provider.group_exists = true
       @provider.stub!(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return([ ])
       @provider.should_receive(:manage_group).and_return(true)
       @provider.run_action(:create)
     end
@@ -141,6 +156,7 @@ describe Chef::Provider::User do
     it "should set the new_resources updated flag when it creates the group if we call manage_group" do
       @provider.group_exists = true
       @provider.stub!(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.stub!(:manage_group).and_return(true)
       @provider.run_action(:create)
       @new_resource.should be_updated
@@ -172,12 +188,14 @@ describe Chef::Provider::User do
 
     it "should run manage_group if the group exists and has mismatched attributes" do
       @provider.should_receive(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.should_receive(:manage_group).and_return(true)
       @provider.run_action(:manage)
     end
 
     it "should set the new resources updated flag to true if manage_group is called" do
       @provider.stub!(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.stub!(:manage_group).and_return(true)
       @provider.run_action(:manage)
       @new_resource.should be_updated
@@ -191,6 +209,7 @@ describe Chef::Provider::User do
 
     it "should not run manage_group if the group exists but has no differing attributes" do
       @provider.should_receive(:compare_group).and_return(false)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.should_not_receive(:manage_group)
       @provider.run_action(:manage)
     end
@@ -204,12 +223,14 @@ describe Chef::Provider::User do
 
     it "should run manage_group if the group exists and has mismatched attributes" do
       @provider.should_receive(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.should_receive(:manage_group).and_return(true)
       @provider.run_action(:modify)
     end
 
     it "should set the new resources updated flag to true if manage_group is called" do
       @provider.stub!(:compare_group).and_return(true)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.stub!(:manage_group).and_return(true)
       @provider.run_action(:modify)
       @new_resource.should be_updated
@@ -217,6 +238,7 @@ describe Chef::Provider::User do
 
     it "should not run manage_group if the group exists but has no differing attributes" do
       @provider.should_receive(:compare_group).and_return(false)
+      @provider.stub!(:change_desc).and_return(["Some changes are going to be done."])
       @provider.should_not_receive(:manage_group)
       @provider.run_action(:modify)
     end
@@ -233,26 +255,26 @@ describe Chef::Provider::User do
        @new_resource.members << "user2"
        @new_resource.stub!(:append).and_return true
        @provider.compare_group.should be_true
-       @provider.change_desc.should == "add missing member(s): user1, user2"
+       @provider.change_desc.should == [ "add missing member(s): user1, user2" ]
     end
 
     it "should report that the group members will be overwritten if not appending" do
        @new_resource.members << "user1"
        @new_resource.stub!(:append).and_return false
        @provider.compare_group.should be_true
-       @provider.change_desc.should == "replace group members with new list of members"
+       @provider.change_desc.should == [ "replace group members with new list of members" ]
     end
 
     it "should report the gid will be changed when it does not match" do
       @current_resource.stub!(:gid).and_return("BADF00D")
       @provider.compare_group.should be_true
-      @provider.change_desc.should == "change gid #{@current_resource.gid} to #{@new_resource.gid}"
+      @provider.change_desc.should == [ "change gid #{@current_resource.gid} to #{@new_resource.gid}" ]
 
     end
 
     it "should report no change reason when no change is required" do
       @provider.compare_group.should be_false
-      @provider.change_desc.should == nil
+      @provider.change_desc.should == [ ]
     end
   end