Merge pull request #5673 from chef/lcg/group-provider-cleanup

cleanup of group provider

18 files changed, 334 insertions, 313 deletions

diff --git a/lib/chef/mixin/shell_out.rb b/lib/chef/mixin/shell_out.rb
index d341e3d1ec..dfdb19821f 100644
--- a/lib/chef/mixin/shell_out.rb
+++ b/lib/chef/mixin/shell_out.rb
@@ -21,6 +21,60 @@ class Chef
   module Mixin
     module ShellOut
 
+      # PREFERRED APIS:
+      #
+      # shell_out_compact and shell_out_compact! flatten their array arguments and remove nils and pass
+      # the resultant array to shell_out.  this actually eliminates spaces-in-args bugs because this:
+      #
+      # shell_out!("command #{arg}")
+      #
+      # becomes two arguments if arg has spaces and requires quotations:
+      #
+      # shell_out!("command '#{arg}'")
+      #
+      # using shell_out_compact! this becomes:
+      #
+      # shell_out_compact!("command", arg)
+      #
+      # and spaces in the arg just works and it does not become two arguments (and the shell quoting around
+      # the argument must actually be removed).
+      #
+      # there's also an implicit join between all the array elements, and nested arrays are flattened which
+      # means that odd where-do-i-put-the-spaces options handling just works, and instead of this:
+      #
+      #    opts = ""                     # needs to be empty string for when foo and bar are both missing
+      #    opts << " -foo" if needs_foo? # needs the leading space on both of these
+      #    opts << " -bar" if needs_bar?
+      #    shell_out!("cmd#{opts}")      # have to think way too hard about why there's no space here
+      #
+      # becomes:
+      #
+      #    opts = []
+      #    opts << "-foo" if needs_foo?
+      #    opts << "-bar" if needs_bar?
+      #    shell_out_compact!("cmd", opts)
+      #
+      # and opts can be an empty array or nil and it'll work out fine.
+      #
+      # generally its best to use shell_out_compact! in code and setup expectations on shell_out! in tests
+      #
+
+      def shell_out_compact(*args, **options)
+        if options.empty?
+          shell_out(*clean_array(*args))
+        else
+          shell_out(*clean_array(*args), **options)
+        end
+      end
+
+      def shell_out_compact!(*args, **options)
+        if options.empty?
+          shell_out!(*clean_array(*args))
+        else
+          shell_out!(*clean_array(*args), **options)
+        end
+      end
+
       # shell_out! runs a command on the system and will raise an error if the command fails, which is what you want
       # for debugging, shell_out and shell_out! both will display command output to the tty when the log level is debug
       # Generally speaking, 'extend Chef::Mixin::ShellOut' in your recipes and include 'Chef::Mixin::ShellOut' in your LWRPs
@@ -85,11 +139,11 @@ class Chef
       # @param args [String] variable number of string arguments
       # @return [String] nicely concatenated string or empty string
       def a_to_s(*args)
-        # FIXME: this should be cleaned up and deprecated
-        clean_array(*args).join(" ")
+        # FIXME: this should be deprecated in favor of shell_out_compact/shell_out_compact!
+        args.flatten.reject { |i| i.nil? || i == "" }.map(&:to_s).join(" ")
       end
 
-      # Helper for sublcasses to reject nil and empty strings out of an array.  It allows
+      # Helper for sublcasses to reject nil out of an array.  It allows
       # using the array form of shell_out (which avoids the need to surround arguments with
       # quote marks to deal with shells).
       #
@@ -106,15 +160,7 @@ class Chef
       # @param args [String] variable number of string arguments
       # @return [Array] array of strings with nil and null string rejection
       def clean_array(*args)
-        args.flatten.reject { |i| i.nil? || i == "" }.map(&:to_s)
-      end
-
-      def shell_out_compact(*args)
-        shell_out(*clean_array(*args))
-      end
-
-      def shell_out_compact!(*args)
-        shell_out!(*clean_array(*args))
+        args.flatten.compact.map(&:to_s)
       end
 
       private
diff --git a/lib/chef/provider/group.rb b/lib/chef/provider/group.rb
index 8936bd2031..6751052ae4 100644
--- a/lib/chef/provider/group.rb
+++ b/lib/chef/provider/group.rb
@@ -39,38 +39,38 @@ class Chef
       end
 
       def load_current_resource
-        @current_resource = Chef::Resource::Group.new(@new_resource.name)
-        @current_resource.group_name(@new_resource.group_name)
+        @current_resource = Chef::Resource::Group.new(new_resource.name)
+        current_resource.group_name(new_resource.group_name)
 
         group_info = nil
         begin
-          group_info = Etc.getgrnam(@new_resource.group_name)
-        rescue ArgumentError => e
+          group_info = Etc.getgrnam(new_resource.group_name)
+        rescue ArgumentError
           @group_exists = false
-          Chef::Log.debug("#{@new_resource} group does not exist")
+          Chef::Log.debug("#{new_resource} group does not exist")
         end
 
         if group_info
-          @new_resource.gid(group_info.gid) unless @new_resource.gid
-          @current_resource.gid(group_info.gid)
-          @current_resource.members(group_info.mem)
+          new_resource.gid(group_info.gid) unless new_resource.gid
+          current_resource.gid(group_info.gid)
+          current_resource.members(group_info.mem)
         end
 
-        @current_resource
+        current_resource
       end
 
       def define_resource_requirements
         requirements.assert(:modify) do |a|
           a.assertion { @group_exists }
-          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.")
+          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
+          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
@@ -86,41 +86,39 @@ class Chef
       # <false>:: If a change is not required
       def compare_group
         @change_desc = [ ]
-        if @new_resource.gid.to_s != @current_resource.gid.to_s
-          @change_desc << "change gid #{@current_resource.gid} to #{@new_resource.gid}"
+        if new_resource.gid.to_s != current_resource.gid.to_s
+          @change_desc << "change gid #{current_resource.gid} to #{new_resource.gid}"
         end
 
-        if @new_resource.append
+        if new_resource.append
           missing_members = []
-          @new_resource.members.each do |member|
+          new_resource.members.each do |member|
             next if has_current_group_member?(member)
             validate_member!(member)
             missing_members << member
           end
-          if missing_members.length > 0
-            @change_desc << "add missing member(s): #{missing_members.join(", ")}"
+          unless missing_members.empty?
+            @change_desc << "add missing member(s): #{missing_members.join(', ')}"
           end
 
           members_to_be_removed = []
-          @new_resource.excluded_members.each do |member|
+          new_resource.excluded_members.each do |member|
             if has_current_group_member?(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"
+          unless members_to_be_removed.empty?
+            @change_desc << "remove existing member(s): #{members_to_be_removed.join(', ')}"
           end
+        elsif new_resource.members != current_resource.members
+          @change_desc << "replace group members with new list of members"
         end
 
         !@change_desc.empty?
       end
 
       def has_current_group_member?(member)
-        @current_resource.members.include?(member)
+        current_resource.members.include?(member)
       end
 
       def validate_member!(member)
@@ -132,44 +130,41 @@ class Chef
       def action_create
         case @group_exists
         when false
-          converge_by("create group #{@new_resource.group_name}") do
+          converge_by("create group #{new_resource.group_name}") do
             create_group
-            Chef::Log.info("#{@new_resource} created")
+            Chef::Log.info("#{new_resource} created")
           end
         else
           if compare_group
-            converge_by(["alter group #{@new_resource.group_name}"] + change_desc) do
+            converge_by(["alter group #{new_resource.group_name}"] + change_desc) do
               manage_group
-              Chef::Log.info("#{@new_resource} altered")
+              Chef::Log.info("#{new_resource} altered")
             end
           end
         end
       end
 
       def action_remove
-        if @group_exists
-          converge_by("remove group #{@new_resource.group_name}") do
-            remove_group
-            Chef::Log.info("#{@new_resource} removed")
-          end
+        return unless @group_exists
+        converge_by("remove group #{new_resource.group_name}") do
+          remove_group
+          Chef::Log.info("#{new_resource} removed")
         end
       end
 
       def action_manage
-        if @group_exists && compare_group
-          converge_by(["manage group #{@new_resource.group_name}"] + change_desc) do
-            manage_group
-            Chef::Log.info("#{@new_resource} managed")
-          end
+        return unless @group_exists && compare_group
+        converge_by(["manage group #{new_resource.group_name}"] + change_desc) do
+          manage_group
+          Chef::Log.info("#{new_resource} managed")
         end
       end
 
       def action_modify
-        if compare_group
-          converge_by(["modify group #{@new_resource.group_name}"] + change_desc) do
-            manage_group
-            Chef::Log.info("#{@new_resource} modified")
-          end
+        return unless compare_group
+        converge_by(["modify group #{new_resource.group_name}"] + change_desc) do
+          manage_group
+          Chef::Log.info("#{new_resource} modified")
         end
       end
 
diff --git a/lib/chef/provider/group/aix.rb b/lib/chef/provider/group/aix.rb
index 4a02d5ef8c..30aea24464 100644
--- a/lib/chef/provider/group/aix.rb
+++ b/lib/chef/provider/group/aix.rb
@@ -17,7 +17,6 @@
 #
 
 require "chef/provider/group/groupadd"
-require "chef/mixin/shell_out"
 
 class Chef
   class Provider
@@ -33,48 +32,39 @@ class Chef
         end
 
         def create_group
-          command = "mkgroup"
-          command << set_options << " #{@new_resource.group_name}"
-          run_command(:command => command)
+          shell_out_compact!("mkgroup", set_options, new_resource.group_name)
           modify_group_members
         end
 
         def manage_group
-          command = "chgroup"
-          options = set_options
-          #Usage: chgroup [-R load_module] "attr=value" ... group
-          if options.size > 0
-            command << options << " #{@new_resource.group_name}"
-            run_command(:command => command)
-          end
+          shell_out_compact!("chgroup", set_options, options, new_resource.group_name)
           modify_group_members
         end
 
         def remove_group
-          run_command(:command => "rmgroup #{@new_resource.group_name}")
+          shell_out_compact!("rmgroup", new_resource.group_name)
         end
 
         def add_member(member)
-          shell_out!("chgrpmem -m + #{member} #{@new_resource.group_name}")
+          shell_out_compact!("chgrpmem", "-m", "+", member, new_resource.group_name)
         end
 
         def set_members(members)
           return if members.empty?
-          shell_out!("chgrpmem -m = #{members.join(',')} #{@new_resource.group_name}")
+          shell_out_compact!("chgrpmem", "-m", "=", members.join(","), new_resource.group_name)
         end
 
         def remove_member(member)
-          shell_out!("chgrpmem -m - #{member} #{@new_resource.group_name}")
+          shell_out_compact!("chgrpmem", "-m", "-", member, new_resource.group_name)
         end
 
         def set_options
-          opts = ""
-          { :gid => "id" }.sort { |a, b| a[0] <=> b[0] }.each do |field, option|
-            if @current_resource.send(field) != @new_resource.send(field)
-              if @new_resource.send(field)
-                Chef::Log.debug("#{@new_resource} setting #{field} to #{@new_resource.send(field)}")
-                opts << " '#{option}=#{@new_resource.send(field)}'"
-              end
+          opts = []
+          { gid: "id" }.sort { |a, b| a[0] <=> b[0] }.each do |field, option|
+            next unless current_resource.send(field) != new_resource.send(field)
+            if new_resource.send(field)
+              Chef::Log.debug("#{new_resource} setting #{field} to #{new_resource.send(field)}")
+              opts << "#{option}=#{new_resource.send(field)}"
             end
           end
           opts
diff --git a/lib/chef/provider/group/dscl.rb b/lib/chef/provider/group/dscl.rb
index 00b4ce2b93..46932adc94 100644
--- a/lib/chef/provider/group/dscl.rb
+++ b/lib/chef/provider/group/dscl.rb
@@ -24,12 +24,15 @@ class Chef
         provides :group, os: "darwin"
 
         def dscl(*args)
-          host = "."
-          stdout_result = ""; stderr_result = ""; cmd = "dscl #{host} -#{args.join(' ')}"
-          status = shell_out(cmd)
+          argdup = args.dup
+          cmd = argdup.shift
+          shellcmd = [ "dscl", ".", "-#{cmd}", argdup ]
+          status = shell_out_compact(shellcmd)
+          stdout_result = ""
+          stderr_result = ""
           status.stdout.each_line { |line| stdout_result << line }
           status.stderr.each_line { |line| stderr_result << line }
-          return [cmd, status, stdout_result, stderr_result]
+          [shellcmd.flatten.compact.join(" "), status, stdout_result, stderr_result]
         end
 
         def safe_dscl(*args)
@@ -37,18 +40,18 @@ class Chef
           return "" if ( args.first =~ /^delete/ ) && ( result[1].exitstatus != 0 )
           raise(Chef::Exceptions::Group, "dscl error: #{result.inspect}") unless result[1].exitstatus == 0
           raise(Chef::Exceptions::Group, "dscl error: #{result.inspect}") if result[2] =~ /No such key: /
-          return result[2]
+          result[2]
         end
 
         def load_current_resource
-          @current_resource = Chef::Resource::Group.new(@new_resource.name)
-          @current_resource.group_name(@new_resource.group_name)
+          @current_resource = Chef::Resource::Group.new(new_resource.name)
+          current_resource.group_name(new_resource.group_name)
           group_info = nil
           begin
-            group_info = safe_dscl("read /Groups/#{@new_resource.group_name}")
+            group_info = safe_dscl("read /Groups/#{new_resource.group_name}")
           rescue Chef::Exceptions::Group
             @group_exists = false
-            Chef::Log.debug("#{@new_resource} group does not exist")
+            Chef::Log.debug("#{new_resource} group does not exist")
           end
 
           if group_info
@@ -57,15 +60,15 @@ class Chef
               val.strip! if val
               case key.downcase
               when "primarygroupid"
-                @new_resource.gid(val) unless @new_resource.gid
-                @current_resource.gid(val)
+                new_resource.gid(val) unless new_resource.gid
+                current_resource.gid(val)
               when "groupmembership"
-                @current_resource.members(val.split(" "))
+                current_resource.members(val.split(" "))
               end
             end
           end
 
-          @current_resource
+          current_resource
         end
 
         # get a free GID greater than 200
@@ -80,51 +83,51 @@ class Chef
               break
             end
           end
-          return gid || raise("gid not found. Exhausted. Searched #{search_limit} times")
+          gid || raise("gid not found. Exhausted. Searched #{search_limit} times")
         end
 
         def gid_used?(gid)
           return false unless gid
           groups_gids = safe_dscl("list /Groups gid")
-          !! ( groups_gids =~ Regexp.new("#{Regexp.escape(gid.to_s)}\n") )
+          !!( groups_gids =~ Regexp.new("#{Regexp.escape(gid.to_s)}\n") )
         end
 
         def set_gid
-          @new_resource.gid(get_free_gid) if [nil, ""].include? @new_resource.gid
-          raise(Chef::Exceptions::Group, "gid is already in use") if gid_used?(@new_resource.gid)
-          safe_dscl("create /Groups/#{@new_resource.group_name} PrimaryGroupID #{@new_resource.gid}")
+          new_resource.gid(get_free_gid) if [nil, ""].include? new_resource.gid
+          raise(Chef::Exceptions::Group, "gid is already in use") if gid_used?(new_resource.gid)
+          safe_dscl("create /Groups/#{new_resource.group_name} PrimaryGroupID #{new_resource.gid}")
         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([ ])
+          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
 
           # Add any members that need to be added
-          if @new_resource.members && !@new_resource.members.empty?
+          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)
+            new_resource.members.each do |member|
+              members_to_be_added << member unless 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(' ')}")
+              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?
+          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)
+            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(' ')}")
+              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
@@ -132,8 +135,8 @@ class Chef
         def define_resource_requirements
           super
           requirements.assert(:all_actions) do |a|
-            a.assertion { ::File.exists?("/usr/bin/dscl") }
-            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/bin/dscl for #{@new_resource.name}"
+            a.assertion { ::File.exist?("/usr/bin/dscl") }
+            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/bin/dscl for #{new_resource.name}"
             # No whyrun alternative: this component should be available in the base install of any given system that uses it
           end
         end
@@ -145,24 +148,24 @@ class Chef
         end
 
         def manage_group
-          if @new_resource.group_name && (@current_resource.group_name != @new_resource.group_name)
+          if new_resource.group_name && (current_resource.group_name != new_resource.group_name)
             dscl_create_group
           end
-          if @new_resource.gid && (@current_resource.gid != @new_resource.gid)
+          if new_resource.gid && (current_resource.gid != new_resource.gid)
             set_gid
           end
-          if @new_resource.members || @new_resource.excluded_members
+          if new_resource.members || new_resource.excluded_members
             set_members
           end
         end
 
         def dscl_create_group
-          safe_dscl("create /Groups/#{@new_resource.group_name}")
-          safe_dscl("create /Groups/#{@new_resource.group_name} Password '*'")
+          safe_dscl("create /Groups/#{new_resource.group_name}")
+          safe_dscl("create /Groups/#{new_resource.group_name} Password '*'")
         end
 
         def remove_group
-          safe_dscl("delete /Groups/#{@new_resource.group_name}")
+          safe_dscl("delete /Groups/#{new_resource.group_name}")
         end
       end
     end
diff --git a/lib/chef/provider/group/gpasswd.rb b/lib/chef/provider/group/gpasswd.rb
index dcf526b211..d8aff10d5b 100644
--- a/lib/chef/provider/group/gpasswd.rb
+++ b/lib/chef/provider/group/gpasswd.rb
@@ -31,26 +31,26 @@ class Chef
         def define_resource_requirements
           super
           requirements.assert(:all_actions) do |a|
-            a.assertion { ::File.exists?("/usr/bin/gpasswd") }
-            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/bin/gpasswd for #{@new_resource}"
+            a.assertion { ::File.exist?("/usr/bin/gpasswd") }
+            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/bin/gpasswd for #{new_resource}"
             # No whyrun alternative: this component should be available in the base install of any given system that uses it
           end
         end
 
         def set_members(members)
-          unless members.empty?
-            shell_out!("gpasswd -M #{members.join(',')} #{@new_resource.group_name}")
+          if members.empty?
+            shell_out_compact!("gpasswd", "-M", "", new_resource.group_name)
           else
-            shell_out!("gpasswd -M \"\" #{@new_resource.group_name}")
+            shell_out_compact!("gpasswd", "-M", members.join(","), new_resource.group_name)
           end
         end
 
         def add_member(member)
-          shell_out!("gpasswd -a #{member} #{@new_resource.group_name}")
+          shell_out_compact!("gpasswd", "-a", member, new_resource.group_name)
         end
 
         def remove_member(member)
-          shell_out!("gpasswd -d #{member} #{@new_resource.group_name}")
+          shell_out_compact!("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 e94ed93cbd..f73c3b3be9 100644
--- a/lib/chef/provider/group/groupadd.rb
+++ b/lib/chef/provider/group/groupadd.rb
@@ -35,7 +35,7 @@ class Chef
           super
           required_binaries.each do |required_binary|
             requirements.assert(:all_actions) do |a|
-              a.assertion { ::File.exists?(required_binary) }
+              a.assertion { ::File.exist?(required_binary) }
               a.failure_message Chef::Exceptions::Group, "Could not find binary #{required_binary} for #{new_resource}"
               # No whyrun alternative: this component should be available in the base install of any given system that uses it
             end
@@ -64,7 +64,7 @@ class Chef
             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)
+                members_to_be_added << member unless 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}")
@@ -108,14 +108,12 @@ class Chef
         # <string>:: A string containing the option and then the quoted value
         def set_options
           opts = []
-          { :gid => "-g" }.sort { |a, b| a[0] <=> b[0] }.each do |field, option|
-            if current_resource.send(field) != new_resource.send(field)
-              if new_resource.send(field)
-                opts << option
-                opts << new_resource.send(field)
-                Chef::Log.debug("#{new_resource} set #{field} to #{new_resource.send(field)}")
-              end
-            end
+          { gid: "-g" }.sort { |a, b| a[0] <=> b[0] }.each do |field, option|
+            next unless current_resource.send(field) != new_resource.send(field)
+            next unless new_resource.send(field)
+            opts << option
+            opts << new_resource.send(field)
+            Chef::Log.debug("#{new_resource} set #{field} to #{new_resource.send(field)}")
           end
           opts << new_resource.group_name
           opts
diff --git a/lib/chef/provider/group/groupmod.rb b/lib/chef/provider/group/groupmod.rb
index 295d3b3425..01581e4863 100644
--- a/lib/chef/provider/group/groupmod.rb
+++ b/lib/chef/provider/group/groupmod.rb
@@ -26,28 +26,26 @@ class Chef
         def load_current_resource
           super
           %w{group user}.each do |binary|
-            raise Chef::Exceptions::Group, "Could not find binary /usr/sbin/#{binary} for #{@new_resource}" unless ::File.exists?("/usr/sbin/#{binary}")
+            raise Chef::Exceptions::Group, "Could not find binary /usr/sbin/#{binary} for #{new_resource}" unless ::File.exist?("/usr/sbin/#{binary}")
           end
         end
 
         # Create the group
         def create_group
-          command = "group add"
-          command << set_options
-          shell_out!(command)
+          shell_out_compact!("group", "add", set_options)
 
-          add_group_members(@new_resource.members)
+          add_group_members(new_resource.members)
         end
 
         # Manage the group when it already exists
         def manage_group
-          if @new_resource.append
+          if new_resource.append
             members_to_be_added = [ ]
-            if @new_resource.excluded_members && !@new_resource.excluded_members.empty?
+            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)
+              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?
@@ -56,39 +54,39 @@ class Chef
 
                 # Capture the members we need to add in
                 # members_to_be_added to be added later on.
-                @current_resource.members.each do |member|
+                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)
+            if new_resource.members && !new_resource.members.empty?
+              new_resource.members.each do |member|
+                members_to_be_added << member unless 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?
+            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
             # 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?
-            add_group_members(@new_resource.members)
+            Chef::Log.debug("#{new_resource} setting group members to: none") if new_resource.members.empty?
+            add_group_members(new_resource.members)
           end
         end
 
         # Remove the group
         def remove_group
-          shell_out!("group del #{@new_resource.group_name}")
+          shell_out_compact!("group", "del", new_resource.group_name)
         end
 
         # Adds a list of usernames to the group using `user mod`
         def add_group_members(members)
-          Chef::Log.debug("#{@new_resource} adding members #{members.join(', ')}") if !members.empty?
+          Chef::Log.debug("#{new_resource} adding members #{members.join(', ')}") unless members.empty?
           members.each do |user|
-            shell_out!("user mod -G #{@new_resource.group_name} #{user}")
+            shell_out_compact!("user", "mod", "-G", new_resource.group_name, user)
           end
         end
 
@@ -96,15 +94,11 @@ class Chef
         # "<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)
+          shell_out_compact!("group", "mod", "-n", "#{new_resource.group_name}_bak", new_resource.group_name)
 
-          create = "group add"
-          create << set_options(:overwrite_gid => true)
-          shell_out!(create)
+          shell_out_compact!("group", "add", set_options(overwrite_gid: true))
 
-          remove = "group del #{@new_resource.group_name}_bak"
-          shell_out!(remove)
+          shell_out_compact!("group", "del", "#{new_resource.group_name}_bak")
         end
 
         # Little bit of magic as per Adam's useradd provider to pull and assign the command line flags
@@ -112,14 +106,15 @@ class Chef
         # ==== Returns
         # <string>:: A string containing the option and then the quoted value
         def set_options(overwrite_gid = false)
-          opts = ""
-          if overwrite_gid || @new_resource.gid && (@current_resource.gid != @new_resource.gid)
-            opts << " -g '#{@new_resource.gid}'"
+          opts = []
+          if overwrite_gid || new_resource.gid && (current_resource.gid != new_resource.gid)
+            opts << "-g"
+            opts << new_resource.gid
           end
           if overwrite_gid
-            opts << " -o"
+            opts << "-o"
           end
-          opts << " #{@new_resource.group_name}"
+          opts << new_resource.group_name
           opts
         end
       end
diff --git a/lib/chef/provider/group/pw.rb b/lib/chef/provider/group/pw.rb
index 4fd78b6b31..115ccf3714 100644
--- a/lib/chef/provider/group/pw.rb
+++ b/lib/chef/provider/group/pw.rb
@@ -30,46 +30,42 @@ class Chef
           super
 
           requirements.assert(:all_actions) do |a|
-            a.assertion { ::File.exists?("/usr/sbin/pw") }
-            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/pw for #{@new_resource}"
+            a.assertion { ::File.exist?("/usr/sbin/pw") }
+            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/pw for #{new_resource}"
             # No whyrun alternative: this component should be available in the base install of any given system that uses it
           end
         end
 
         # Create the group
         def create_group
-          command = "pw groupadd"
-          command << set_options
-
-          unless @new_resource.members.empty?
+          command = [ "pw", "groupadd", set_options ]
+          unless new_resource.members.empty?
             # pw group[add|mod] -M is used to set the full membership list on a
             # new or existing group. Because pw groupadd does not support the -m
             # and -d options used by manage_group, we treat group creation as a
             # special case and use -M.
-            Chef::Log.debug("#{@new_resource} setting group members: #{@new_resource.members.join(',')}")
-            command += " -M #{@new_resource.members.join(',')}"
+            Chef::Log.debug("#{new_resource} setting group members: #{new_resource.members.join(',')}")
+            command += [ "-M", new_resource.members.join(",") ]
           end
 
-          run_command(:command => command)
+          shell_out_compact!(command)
         end
 
         # Manage the group when it already exists
         def manage_group
-          command = "pw groupmod"
-          command << set_options
           member_options = set_members_options
           if member_options.empty?
-            run_command(:command => command)
+            shell_out_compact!("pw", "groupmod", set_options)
           else
             member_options.each do |option|
-              run_command(:command => command + option)
+              shell_out_compact!("pw", "groupmod", set_options, option)
             end
           end
         end
 
         # Remove the group
         def remove_group
-          run_command(:command => "pw groupdel #{@new_resource.group_name}")
+          shell_out_compact!("pw", "groupdel", new_resource.group_name)
         end
 
         # Little bit of magic as per Adam's useradd provider to pull and assign the command line flags
@@ -77,10 +73,11 @@ class Chef
         # ==== Returns
         # <string>:: A string containing the option and then the quoted value
         def set_options
-          opts = " #{@new_resource.group_name}"
-          if @new_resource.gid && (@current_resource.gid != @new_resource.gid)
-            Chef::Log.debug("#{@new_resource}: current gid (#{@current_resource.gid}) doesnt match target gid (#{@new_resource.gid}), changing it")
-            opts << " -g '#{@new_resource.gid}'"
+          opts = [ new_resource.group_name ]
+          if new_resource.gid && (current_resource.gid != new_resource.gid)
+            Chef::Log.debug("#{new_resource}: current gid (#{current_resource.gid}) doesnt match target gid (#{new_resource.gid}), changing it")
+            opts << "-g"
+            opts << new_resource.gid
           end
           opts
         end
@@ -91,26 +88,26 @@ class Chef
           members_to_be_added = [ ]
           members_to_be_removed = [ ]
 
-          if @new_resource.append
+          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)
+            if new_resource.members && !new_resource.members.empty?
+              new_resource.members.each do |member|
+                members_to_be_added << member unless 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)
+            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
             # Append is not set so we're resetting the membership of
             # the group to the given members.
-            members_to_be_added = @new_resource.members.dup
-            @current_resource.members.each do |member|
+            members_to_be_added = new_resource.members.dup
+            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
@@ -122,13 +119,13 @@ class Chef
           end
 
           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(',')}"
+            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(',')}"
+            Chef::Log.debug("#{new_resource} removing group members: #{members_to_be_removed.join(',')}")
+            opts << [ "-d", members_to_be_removed.join(",") ]
           end
 
           opts
diff --git a/lib/chef/provider/group/suse.rb b/lib/chef/provider/group/suse.rb
index 71336f9311..0790d2c2d9 100644
--- a/lib/chef/provider/group/suse.rb
+++ b/lib/chef/provider/group/suse.rb
@@ -33,21 +33,21 @@ class Chef
         def define_resource_requirements
           super
           requirements.assert(:all_actions) do |a|
-            a.assertion { ::File.exists?("/usr/sbin/groupmod") }
-            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/groupmod for #{@new_resource.name}"
+            a.assertion { ::File.exist?("/usr/sbin/groupmod") }
+            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/groupmod for #{new_resource.name}"
             # No whyrun alternative: this component should be available in the base install of any given system that uses it
           end
 
           requirements.assert(:create, :manage, :modify) do |a|
             a.assertion do
               begin
-                to_add(@new_resource.members).all? { |member| Etc.getpwnam(member) }
+                to_add(new_resource.members).all? { |member| Etc.getpwnam(member) }
               rescue
                 false
               end
             end
-            a.failure_message Chef::Exceptions::Group, "Could not add users #{to_add(@new_resource.members).join(", ")} to #{@new_resource.group_name}: one of these users does not exist"
-            a.whyrun "Could not find one of these users: #{to_add(@new_resource.members).join(", ")}. Assuming it will be created by a prior step"
+            a.failure_message Chef::Exceptions::Group, "Could not add users #{to_add(new_resource.members).join(', ')} to #{new_resource.group_name}: one of these users does not exist"
+            a.whyrun "Could not find one of these users: #{to_add(new_resource.members).join(', ')}. Assuming it will be created by a prior step"
           end
         end
 
@@ -62,19 +62,19 @@ class Chef
         end
 
         def to_add(members)
-          members - @current_resource.members
+          members - current_resource.members
         end
 
         def add_member(member)
-          shell_out!("groupmod -A #{member} #{@new_resource.group_name}")
+          shell_out_compact!("groupmod", "-A", member, new_resource.group_name)
         end
 
         def to_remove(members)
-          @current_resource.members - members
+          current_resource.members - members
         end
 
         def remove_member(member)
-          shell_out!("groupmod -R #{member} #{@new_resource.group_name}")
+          shell_out_compact!("groupmod", "-R", member, new_resource.group_name)
         end
 
       end
diff --git a/lib/chef/provider/group/usermod.rb b/lib/chef/provider/group/usermod.rb
index bef4b667a2..3874f7b4de 100644
--- a/lib/chef/provider/group/usermod.rb
+++ b/lib/chef/provider/group/usermod.rb
@@ -34,19 +34,19 @@ class Chef
           super
 
           requirements.assert(:all_actions) do |a|
-            a.assertion { ::File.exists?("/usr/sbin/usermod") }
-            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/usermod for #{@new_resource}"
+            a.assertion { ::File.exist?("/usr/sbin/usermod") }
+            a.failure_message Chef::Exceptions::Group, "Could not find binary /usr/sbin/usermod for #{new_resource}"
             # No whyrun alternative: this component should be available in the base install of any given system that uses it
           end
 
           requirements.assert(:modify, :manage) do |a|
-            a.assertion { @new_resource.members.empty? || @new_resource.append }
+            a.assertion { new_resource.members.empty? || new_resource.append }
             a.failure_message Chef::Exceptions::Group, "setting group members directly is not supported by #{self}, must set append true in group"
             # No whyrun alternative - this action is simply not supported.
           end
 
           requirements.assert(:all_actions) do |a|
-            a.assertion { @new_resource.excluded_members.empty? }
+            a.assertion { new_resource.excluded_members.empty? }
             a.failure_message Chef::Exceptions::Group, "excluded_members is not supported by #{self}"
             # No whyrun alternative - this action is simply not supported.
           end
@@ -57,17 +57,16 @@ class Chef
           # 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.include?(:create)
-            members.each do |member|
-              add_member(member)
-            end
-          else
+          unless new_resource.action.include?(:create)
             raise Chef::Exceptions::UnsupportedAction, "Setting members directly is not supported by #{self}"
           end
+          members.each do |member|
+            add_member(member)
+          end
         end
 
         def add_member(member)
-          shell_out!("usermod #{append_flags} #{@new_resource.group_name} #{member}")
+          shell_out_compact!("usermod", append_flags, new_resource.group_name, member)
         end
 
         def remove_member(member)
@@ -81,7 +80,7 @@ class Chef
           when "openbsd", "netbsd", "aix", "solaris2", "smartos", "omnios"
             "-G"
           when "solaris", "suse", "opensuse"
-            "-a -G"
+            [ "-a", "-G" ]
           end
         end
 
diff --git a/lib/chef/provider/group/windows.rb b/lib/chef/provider/group/windows.rb
index 5873e42a6b..c0026bf368 100644
--- a/lib/chef/provider/group/windows.rb
+++ b/lib/chef/provider/group/windows.rb
@@ -30,26 +30,26 @@ class Chef
 
         def initialize(new_resource, run_context)
           super
-          @net_group = Chef::Util::Windows::NetGroup.new(@new_resource.group_name)
+          @net_group = Chef::Util::Windows::NetGroup.new(new_resource.group_name)
         end
 
         def load_current_resource
-          @current_resource = Chef::Resource::Group.new(@new_resource.name)
-          @current_resource.group_name(@new_resource.group_name)
+          @current_resource = Chef::Resource::Group.new(new_resource.name)
+          current_resource.group_name(new_resource.group_name)
 
           members = nil
           begin
             members = @net_group.local_get_members
-          rescue => e
+          rescue
             @group_exists = false
-            Chef::Log.debug("#{@new_resource} group does not exist")
+            Chef::Log.debug("#{new_resource} group does not exist")
           end
 
           if members
-            @current_resource.members(members)
+            current_resource.members(members)
           end
 
-          @current_resource
+          current_resource
         end
 
         def create_group
@@ -58,10 +58,10 @@ class Chef
         end
 
         def manage_group
-          if @new_resource.append
+          if new_resource.append
             members_to_be_added = [ ]
-            @new_resource.members.each do |member|
-              members_to_be_added << member if ! has_current_group_member?(member) && validate_member!(member)
+            new_resource.members.each do |member|
+              members_to_be_added << member if !has_current_group_member?(member) && validate_member!(member)
             end
 
             # local_add_members will raise ERROR_MEMBER_IN_ALIAS if a
@@ -69,19 +69,19 @@ class Chef
             @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|
-              member_sid = lookup_account_name(member)
+            new_resource.excluded_members.each do |member|
+              lookup_account_name(member)
               members_to_be_removed << member if has_current_group_member?(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)
+            @net_group.local_set_members(new_resource.members)
           end
         end
 
         def has_current_group_member?(member)
           member_sid = lookup_account_name(member)
-          @current_resource.members.include?(member_sid)
+          current_resource.members.include?(member_sid)
         end
 
         def remove_group
@@ -97,12 +97,10 @@ class Chef
         end
 
         def lookup_account_name(account_name)
-          begin
-            Chef::ReservedNames::Win32::Security.lookup_account_name(locally_qualified_name(account_name))[1].to_s
-          rescue Chef::Exceptions::Win32APIError
-            Chef::Log.warn("SID for '#{locally_qualified_name(account_name)}' could not be found")
-            ""
-          end
+          Chef::ReservedNames::Win32::Security.lookup_account_name(locally_qualified_name(account_name))[1].to_s
+        rescue Chef::Exceptions::Win32APIError
+          Chef::Log.warn("SID for '#{locally_qualified_name(account_name)}' could not be found")
+          ""
         end
 
       end
diff --git a/spec/unit/provider/group/dscl_spec.rb b/spec/unit/provider/group/dscl_spec.rb
index bc69d7580e..9dc969c352 100644
--- a/spec/unit/provider/group/dscl_spec.rb
+++ b/spec/unit/provider/group/dscl_spec.rb
@@ -28,12 +28,12 @@ describe Chef::Provider::Group::Dscl do
     @provider = Chef::Provider::Group::Dscl.new(@new_resource, @run_context)
     @provider.current_resource = @current_resource
 
-    @status = double(:stdout => "\n", :stderr => "", :exitstatus => 0)
+    @status = double(stdout: "\n", stderr: "", exitstatus: 0)
     allow(@provider).to receive(:shell_out).and_return(@status)
   end
 
   it "should run shell_out with the supplied array of arguments appended to the dscl command" do
-    expect(@provider).to receive(:shell_out).with("dscl . -cmd /Path arg1 arg2")
+    expect(@provider).to receive(:shell_out).with("dscl", ".", "-cmd", "/Path", "arg1", "arg2")
     @provider.dscl("cmd", "/Path", "arg1", "arg2")
   end
 
@@ -57,7 +57,7 @@ describe Chef::Provider::Group::Dscl do
 
     describe "with the dscl command returning a non zero exit status for a delete" do
       before do
-        @status = double("Process::Status", :exitstatus => 1)
+        @status = double("Process::Status", exitstatus: 1)
         allow(@provider).to receive(:dscl).and_return(["cmd", @status, "stdout", "stderr"])
       end
 
@@ -232,20 +232,20 @@ describe Chef::Provider::Group::Dscl do
   end
 
   describe "when loading the current system state" do
-    before (:each) do
+    before(:each) do
       @provider.action = :create
       @provider.load_current_resource
       @provider.define_resource_requirements
     end
 
     it "raises an error if the required binary /usr/bin/dscl doesn't exist" do
-      expect(File).to receive(:exists?).with("/usr/bin/dscl").and_return(false)
+      expect(File).to receive(:exist?).with("/usr/bin/dscl").and_return(false)
 
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
     it "doesn't raise an error if /usr/bin/dscl exists" do
-      allow(File).to receive(:exists?).and_return(true)
+      allow(File).to receive(:exist?).and_return(true)
       expect { @provider.process_resource_requirements }.not_to raise_error
     end
   end
@@ -321,11 +321,11 @@ EOF
   end
 
   it "should parse gid properly" do
-    allow(File).to receive(:exists?).and_return(true)
+    allow(File).to receive(:exist?).and_return(true)
     expect(@current_resource.gid).to eq("999")
   end
   it "should parse members properly" do
-    allow(File).to receive(:exists?).and_return(true)
+    allow(File).to receive(:exist?).and_return(true)
     expect(@current_resource.members).to eq(%w{waka bar})
   end
 end
diff --git a/spec/unit/provider/group/gpasswd_spec.rb b/spec/unit/provider/group/gpasswd_spec.rb
index 16e6ad15ea..add87bf008 100644
--- a/spec/unit/provider/group/gpasswd_spec.rb
+++ b/spec/unit/provider/group/gpasswd_spec.rb
@@ -27,11 +27,11 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
     @new_resource.members %w{lobster rage fist}
     @new_resource.append false
     @provider = Chef::Provider::Group::Gpasswd.new(@new_resource, @run_context)
-    #@provider.stub(:run_command).and_return(true)
+    # @provider.stub(:run_command).and_return(true)
   end
 
   describe "when determining the current group state" do
-    before (:each) do
+    before(:each) do
       @provider.action = :create
       @provider.load_current_resource
       @provider.define_resource_requirements
@@ -41,13 +41,13 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
     # for Chef::Provider::Group - no need to repeat it here.  We'll
     # include only what's specific to this provider.
     it "should raise an error if the required binary /usr/bin/gpasswd doesn't exist" do
-      allow(File).to receive(:exists?).and_return(true)
-      expect(File).to receive(:exists?).with("/usr/bin/gpasswd").and_return(false)
+      allow(File).to receive(:exist?).and_return(true)
+      expect(File).to receive(:exist?).with("/usr/bin/gpasswd").and_return(false)
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
     it "shouldn't raise an error if the required binaries exist" do
-      allow(File).to receive(:exists?).and_return(true)
+      allow(File).to receive(:exist?).and_return(true)
       expect { @provider.process_resource_requirements }.not_to raise_error
     end
   end
@@ -66,7 +66,7 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
 
       it "logs a message and sets group's members to 'none'" do
         expect(Chef::Log).to receive(:debug).with("group[wheel] setting group members to: none")
-        expect(@provider).to receive(:shell_out!).with("gpasswd -M \"\" wheel")
+        expect(@provider).to receive(:shell_out!).with("gpasswd", "-M", "", "wheel")
         @provider.modify_group_members
       end
     end
@@ -91,7 +91,7 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
       end
 
       it "should run gpasswd with the members joined by ',' followed by the target group" do
-        expect(@provider).to receive(:shell_out!).with("gpasswd -M lobster,rage,fist wheel")
+        expect(@provider).to receive(:shell_out!).with("gpasswd", "-M", "lobster,rage,fist", "wheel")
         @provider.modify_group_members
       end
 
@@ -104,9 +104,9 @@ describe Chef::Provider::Group::Gpasswd, "modify_group_members" do
 
         it "should run gpasswd individually for each user when the append option is set" do
           @new_resource.append(true)
-          expect(@provider).to receive(:shell_out!).with("gpasswd -a lobster wheel")
-          expect(@provider).to receive(:shell_out!).with("gpasswd -a rage wheel")
-          expect(@provider).to receive(:shell_out!).with("gpasswd -a fist wheel")
+          expect(@provider).to receive(:shell_out!).with("gpasswd", "-a", "lobster", "wheel")
+          expect(@provider).to receive(:shell_out!).with("gpasswd", "-a", "rage", "wheel")
+          expect(@provider).to receive(:shell_out!).with("gpasswd", "-a", "fist", "wheel")
           @provider.modify_group_members
         end
       end
diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb
index 70a98ff2f5..929dd00450 100644
--- a/spec/unit/provider/group/groupadd_spec.rb
+++ b/spec/unit/provider/group/groupadd_spec.rb
@@ -46,7 +46,7 @@ describe Chef::Provider::Group::Groupadd do
 
   describe "#set_options" do
     field_list = {
-      :gid => "-g",
+      gid: "-g",
     }
 
     field_list.each do |attribute, option|
@@ -168,25 +168,25 @@ describe Chef::Provider::Group::Groupadd do
     end
 
     before do
-      allow(File).to receive(:exists?).and_return(false)
+      allow(File).to receive(:exist?).and_return(false)
       provider.define_resource_requirements
     end
 
     it "should raise an error if the required binary /usr/sbin/groupadd doesn't exist" do
-      expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(false)
+      expect(File).to receive(:exist?).with("/usr/sbin/groupadd").and_return(false)
       expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
     it "should raise an error if the required binary /usr/sbin/groupmod doesn't exist" do
-      expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(true)
-      expect(File).to receive(:exists?).with("/usr/sbin/groupmod").and_return(false)
+      expect(File).to receive(:exist?).with("/usr/sbin/groupadd").and_return(true)
+      expect(File).to receive(:exist?).with("/usr/sbin/groupmod").and_return(false)
       expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
     it "should raise an error if the required binary /usr/sbin/groupdel doesn't exist" do
-      expect(File).to receive(:exists?).with("/usr/sbin/groupadd").and_return(true)
-      expect(File).to receive(:exists?).with("/usr/sbin/groupmod").and_return(true)
-      expect(File).to receive(:exists?).with("/usr/sbin/groupdel").and_return(false)
+      expect(File).to receive(:exist?).with("/usr/sbin/groupadd").and_return(true)
+      expect(File).to receive(:exist?).with("/usr/sbin/groupmod").and_return(true)
+      expect(File).to receive(:exist?).with("/usr/sbin/groupdel").and_return(false)
       expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
diff --git a/spec/unit/provider/group/groupmod_spec.rb b/spec/unit/provider/group/groupmod_spec.rb
index 21d026409e..6629b718d0 100644
--- a/spec/unit/provider/group/groupmod_spec.rb
+++ b/spec/unit/provider/group/groupmod_spec.rb
@@ -33,17 +33,17 @@ describe Chef::Provider::Group::Groupmod do
   describe "manage_group" do
     describe "when determining the current group state" do
       it "should raise an error if the required binary /usr/sbin/group doesn't exist" do
-        expect(File).to receive(:exists?).with("/usr/sbin/group").and_return(false)
+        expect(File).to receive(:exist?).with("/usr/sbin/group").and_return(false)
         expect { @provider.load_current_resource }.to raise_error(Chef::Exceptions::Group)
       end
       it "should raise an error if the required binary /usr/sbin/user doesn't exist" do
-        expect(File).to receive(:exists?).with("/usr/sbin/group").and_return(true)
-        expect(File).to receive(:exists?).with("/usr/sbin/user").and_return(false)
+        expect(File).to receive(:exist?).with("/usr/sbin/group").and_return(true)
+        expect(File).to receive(:exist?).with("/usr/sbin/user").and_return(false)
         expect { @provider.load_current_resource }.to raise_error(Chef::Exceptions::Group)
       end
 
       it "shouldn't raise an error if the required binaries exist" do
-        allow(File).to receive(:exists?).and_return(true)
+        allow(File).to receive(:exist?).and_return(true)
         expect { @provider.load_current_resource }.not_to raise_error
       end
     end
@@ -62,9 +62,9 @@ describe Chef::Provider::Group::Groupmod do
 
         it "logs a message and sets group's members to 'none', then removes existing group members" do
           expect(Chef::Log).to receive(:debug).with("group[wheel] setting group members to: none")
-          expect(@provider).to receive(:shell_out!).with("group mod -n wheel_bak wheel")
-          expect(@provider).to receive(:shell_out!).with("group add -g '123' -o wheel")
-          expect(@provider).to receive(:shell_out!).with("group del wheel_bak")
+          expect(@provider).to receive(:shell_out!).with("group", "mod", "-n", "wheel_bak", "wheel")
+          expect(@provider).to receive(:shell_out!).with("group", "add", "-g", "123", "-o", "wheel")
+          expect(@provider).to receive(:shell_out!).with("group", "del", "wheel_bak")
           @provider.manage_group
         end
       end
@@ -90,10 +90,10 @@ describe Chef::Provider::Group::Groupmod do
 
         it "updates group membership correctly" do
           allow(Chef::Log).to receive(:debug)
-          expect(@provider).to receive(:shell_out!).with("group mod -n wheel_bak wheel")
-          expect(@provider).to receive(:shell_out!).with("user mod -G wheel lobster")
-          expect(@provider).to receive(:shell_out!).with("group add -g '123' -o wheel")
-          expect(@provider).to receive(:shell_out!).with("group del wheel_bak")
+          expect(@provider).to receive(:shell_out!).with("group", "mod", "-n", "wheel_bak", "wheel")
+          expect(@provider).to receive(:shell_out!).with("user", "mod", "-G", "wheel", "lobster")
+          expect(@provider).to receive(:shell_out!).with("group", "add", "-g", "123", "-o", "wheel")
+          expect(@provider).to receive(:shell_out!).with("group", "del", "wheel_bak")
           @provider.manage_group
         end
       end
@@ -108,10 +108,10 @@ describe Chef::Provider::Group::Groupmod do
       end
 
       it "should run a group add command and some user mod commands" do
-        expect(@provider).to receive(:shell_out!).with("group add -g '123' wheel")
-        expect(@provider).to receive(:shell_out!).with("user mod -G wheel lobster")
-        expect(@provider).to receive(:shell_out!).with("user mod -G wheel rage")
-        expect(@provider).to receive(:shell_out!).with("user mod -G wheel fist")
+        expect(@provider).to receive(:shell_out!).with("group", "add", "-g", "123", "wheel")
+        expect(@provider).to receive(:shell_out!).with("user", "mod", "-G", "wheel", "lobster")
+        expect(@provider).to receive(:shell_out!).with("user", "mod", "-G", "wheel", "rage")
+        expect(@provider).to receive(:shell_out!).with("user", "mod", "-G", "wheel", "fist")
         @provider.create_group
       end
     end
@@ -125,7 +125,7 @@ describe Chef::Provider::Group::Groupmod do
       end
 
       it "should run a group del command" do
-        expect(@provider).to receive(:shell_out!).with("group del wheel")
+        expect(@provider).to receive(:shell_out!).with("group", "del", "wheel")
         @provider.remove_group
       end
     end
diff --git a/spec/unit/provider/group/pw_spec.rb b/spec/unit/provider/group/pw_spec.rb
index 97ffcb3c31..f109ba6ba1 100644
--- a/spec/unit/provider/group/pw_spec.rb
+++ b/spec/unit/provider/group/pw_spec.rb
@@ -37,19 +37,19 @@ describe Chef::Provider::Group::Pw do
 
   describe "when setting options for the pw command" do
     it "does not set the gid option if gids match or are unmanaged" do
-      expect(@provider.set_options).to eq(" wheel")
+      expect(@provider.set_options).to eq(["wheel"])
     end
 
     it "sets the option for gid if it is not nil" do
       @new_resource.gid(42)
-      expect(@provider.set_options).to eql(" wheel -g '42'")
+      expect(@provider.set_options).to eql(["wheel", "-g", 42])
     end
   end
 
   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)
-      expect(@provider).to receive(:run_command).with({ :command => "pw groupadd wheel -g '23' -M root,aj" }).and_return(true)
+      expect(@provider).to receive(:shell_out!).with("pw", "groupadd", "wheel", "-g", "23", "-M", "root,aj").and_return(true)
       @provider.create_group
     end
   end
@@ -59,8 +59,8 @@ describe Chef::Provider::Group::Pw do
     it "should run pw groupmod with the return of set_options" do
       @new_resource.gid(42)
       @new_resource.members(["someone"])
-      expect(@provider).to receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -m someone" }).and_return(true)
-      expect(@provider).to receive(:run_command).with({ :command => "pw groupmod wheel -g '42' -d root,aj" }).and_return(true)
+      expect(@provider).to receive(:shell_out!).with("pw", "groupmod", "wheel", "-g", "42", "-m", "someone").and_return(true)
+      expect(@provider).to receive(:shell_out!).with("pw", "groupmod", "wheel", "-g", "42", "-d", "root,aj").and_return(true)
       @provider.manage_group
     end
 
@@ -68,7 +68,7 @@ describe Chef::Provider::Group::Pw do
 
   describe "when removing the group" do
     it "should run pw groupdel with the new resources group name" do
-      expect(@provider).to receive(:run_command).with({ :command => "pw groupdel wheel" }).and_return(true)
+      expect(@provider).to receive(:shell_out!).with("pw", "groupdel", "wheel").and_return(true)
       @provider.remove_group
     end
   end
@@ -98,7 +98,7 @@ describe Chef::Provider::Group::Pw do
       end
 
       it "should set the -d option with the members joined by ','" do
-        expect(@provider.set_members_options).to eql([ " -d all,your,base" ])
+        expect(@provider.set_members_options).to eql([ ["-d", "all,your,base"] ])
       end
     end
 
@@ -114,24 +114,24 @@ describe Chef::Provider::Group::Pw do
       end
 
       it "should set the -m option with the members joined by ','" do
-        expect(@provider.set_members_options).to eql([ " -m all,your,base" ])
+        expect(@provider.set_members_options).to eql([[ "-m", "all,your,base" ]])
       end
     end
   end
 
   describe "load_current_resource" do
-    before (:each) do
+    before(:each) do
       @provider.action = :create
       @provider.load_current_resource
       @provider.define_resource_requirements
     end
     it "should raise an error if the required binary /usr/sbin/pw doesn't exist" do
-      expect(File).to receive(:exists?).with("/usr/sbin/pw").and_return(false)
+      expect(File).to receive(:exist?).with("/usr/sbin/pw").and_return(false)
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
     it "shouldn't raise an error if /usr/sbin/pw exists" do
-      allow(File).to receive(:exists?).and_return(true)
+      allow(File).to receive(:exist?).and_return(true)
       expect { @provider.process_resource_requirements }.not_to raise_error
     end
   end
diff --git a/spec/unit/provider/group/suse_spec.rb b/spec/unit/provider/group/suse_spec.rb
index da4e8e9155..e61d865b6d 100644
--- a/spec/unit/provider/group/suse_spec.rb
+++ b/spec/unit/provider/group/suse_spec.rb
@@ -7,7 +7,7 @@
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
 #
-#     http://www.apache.org/licenses/LICENSE-2.0
+#      http://www.apache.org/licenses/LICENSE-2.0
 #
 # Unless required by applicable law or agreed to in writing, software
 # distributed under the License is distributed on an "AS IS" BASIS,
@@ -47,7 +47,7 @@ describe Chef::Provider::Group::Suse do
 
   describe "when determining the current group state" do
     before(:each) do
-      allow(File).to receive(:exists?).and_return(true)
+      allow(File).to receive(:exist?).and_return(true)
       provider.action = :create
       provider.define_resource_requirements
     end
@@ -56,7 +56,7 @@ describe Chef::Provider::Group::Suse do
     # for Chef::Provider::Group - no need to repeat it here.  We'll
     # include only what's specific to this provider.
     it "should raise an error if the required binary /usr/sbin/groupmod doesn't exist" do
-      expect(File).to receive(:exists?).with("/usr/sbin/groupmod").and_return(false)
+      expect(File).to receive(:exist?).with("/usr/sbin/groupmod").and_return(false)
       expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
@@ -76,14 +76,14 @@ describe Chef::Provider::Group::Suse do
 
   describe "#add_member" do
     it "should call out to groupmod to add user" do
-      expect(provider).to receive(:shell_out!).with("groupmod -A new_user new_group")
+      expect(provider).to receive(:shell_out!).with("groupmod", "-A", "new_user", "new_group")
       provider.add_member("new_user")
     end
   end
 
   describe "#remove_member" do
     it "should call out to groupmod to remove user" do
-      expect(provider).to receive(:shell_out!).with("groupmod -R new_user new_group")
+      expect(provider).to receive(:shell_out!).with("groupmod", "-R", "new_user", "new_group")
       provider.remove_member("new_user")
     end
   end
diff --git a/spec/unit/provider/group/usermod_spec.rb b/spec/unit/provider/group/usermod_spec.rb
index 7bd45c4f1b..c6422b12c9 100644
--- a/spec/unit/provider/group/usermod_spec.rb
+++ b/spec/unit/provider/group/usermod_spec.rb
@@ -46,18 +46,18 @@ describe Chef::Provider::Group::Usermod do
 
     describe "with supplied members" do
       platforms = {
-        "openbsd" => "-G",
-        "netbsd" => "-G",
-        "solaris" => "-a -G",
-        "suse" => "-a -G",
-        "opensuse" => "-a -G",
-        "smartos" => "-G",
-        "omnios" => "-G",
+        "openbsd" => [ "-G" ],
+        "netbsd" => [ "-G" ],
+        "solaris" => [ "-a", "-G" ],
+        "suse" => [ "-a", "-G" ],
+        "opensuse" => [ "-a", "-G" ],
+        "smartos" => [ "-G" ],
+        "omnios" => [ "-G" ],
       }
 
       before do
         allow(@new_resource).to receive(:members).and_return(%w{all your base})
-        allow(File).to receive(:exists?).and_return(true)
+        allow(File).to receive(:exist?).and_return(true)
       end
 
       it "should raise an error when setting the entire group directly" do
@@ -85,9 +85,9 @@ describe Chef::Provider::Group::Usermod do
           @provider.current_resource = current_resource
           @node.automatic_attrs[:platform] = platform
           allow(@new_resource).to receive(:append).and_return(true)
-          expect(@provider).to receive(:shell_out!).with("usermod #{flags} wheel all")
-          expect(@provider).to receive(:shell_out!).with("usermod #{flags} wheel your")
-          expect(@provider).to receive(:shell_out!).with("usermod #{flags} wheel base")
+          expect(@provider).to receive(:shell_out!).with("usermod", *flags, "wheel", "all")
+          expect(@provider).to receive(:shell_out!).with("usermod", *flags, "wheel", "your")
+          expect(@provider).to receive(:shell_out!).with("usermod", *flags, "wheel", "base")
           @provider.modify_group_members
         end
       end
@@ -96,19 +96,19 @@ describe Chef::Provider::Group::Usermod do
 
   describe "when loading the current resource" do
     before(:each) do
-      allow(File).to receive(:exists?).and_return(false)
+      allow(File).to receive(:exist?).and_return(false)
       @provider.action = :create
       @provider.define_resource_requirements
     end
 
     it "should raise an error if the required binary /usr/sbin/usermod doesn't exist" do
-      allow(File).to receive(:exists?).and_return(true)
-      expect(File).to receive(:exists?).with("/usr/sbin/usermod").and_return(false)
+      allow(File).to receive(:exist?).and_return(true)
+      expect(File).to receive(:exist?).with("/usr/sbin/usermod").and_return(false)
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Group)
     end
 
     it "shouldn't raise an error if the required binaries exist" do
-      allow(File).to receive(:exists?).and_return(true)
+      allow(File).to receive(:exist?).and_return(true)
       expect { @provider.process_resource_requirements }.not_to raise_error
     end
   end