Refactor the group tests and handle OSX

1 files changed, 59 insertions, 48 deletions

diff --git a/spec/functional/resource/group_spec.rb b/spec/functional/resource/group_spec.rb
index 4909040ba5..679d169727 100644
--- a/spec/functional/resource/group_spec.rb
+++ b/spec/functional/resource/group_spec.rb
@@ -95,7 +95,7 @@ describe Chef::Resource::Group, :requires_root_or_running_windows, :not_supporte
 
   def create_user(username)
     user(username).run_action(:create) if ! windows_domain_user?(username)
-    # TODO: User shouldn't exist
+    # TODO: User should exist
   end
 
   def remove_user(username)
@@ -135,91 +135,102 @@ describe Chef::Resource::Group, :requires_root_or_running_windows, :not_supporte
       group_should_not_exist(group_name)
     end
 
-    describe "when append is not set" do
-      let(:included_members) { [spec_members[1]] }
-
-      before do
-        create_user(spec_members[1])
-        create_user(spec_members[0])
-        add_members_to_group([spec_members[0]])
-      end
+    # dscl doesn't perform any error checking and will let you add users that don't exist.
+    describe "when no users exist", :not_supported_on_mac_osx do
+      describe "when append is not set" do
+        # excluded_members can only be used when append is set.  It is ignored otherwise.
+        let(:excluded_members) { [] }
 
-      after do
-        remove_user(spec_members[1])
-        remove_user(spec_members[0])
+        it "should raise an error" do
+          expect { group_resource.run_action(tested_action) }.to raise_error(Chef::Exceptions::Win32APIError)
+        end
       end
 
-      it "should remove the existing users and add the new users to the group" do
-        group_resource.run_action(tested_action)
+      describe "when append is set" do
+        before do
+          group_resource.append(true)
+        end
 
-        expect(user_exist_in_group?(spec_members[1])).to eq(true)
-        expect(user_exist_in_group?(spec_members[0])).to eq(false)
+        it "should raise an error" do
+          expect { group_resource.run_action(tested_action) }.to raise_error(Chef::Exceptions::Win32APIError)
+        end
       end
     end
 
-    describe "when append is set" do
-      before(:each) do
-        group_resource.append(true)
-      end
-
-      describe "when the users exist" do
-        before do
-          (included_members + excluded_members).each do |member|
-            create_user(member)
-          end
+    describe "when the users exist" do
+      before do
+        (spec_members).each do |member|
+          create_user(member)
         end
+      end
 
-        after do
-          (included_members + excluded_members).each do |member|
-            remove_user(member)
-          end
+      after do
+        (spec_members).each do |member|
+          remove_user(member)
         end
+      end
 
-        it "should add included members to the group" do
+      describe "when append is not set" do
+        it "should set the group to to contain given members" do
           group_resource.run_action(tested_action)
 
           included_members.each do |member|
             expect(user_exist_in_group?(member)).to eq(true)
           end
-          excluded_members.each do |member|
+          (spec_members - included_members).each do |member|
             expect(user_exist_in_group?(member)).to eq(false)
           end
         end
 
-        describe "when group contains some users" do
-          before(:each) do
-            add_members_to_group([ spec_members[0], spec_members[2] ])
+        describe "when group already contains some users" do
+          before do
+            add_members_to_group([included_members[0]])
+            add_members_to_group(spec_members - included_members)
           end
 
-          it "should add the included users and remove excluded users" do
+          it "should remove all existing users and only add the new users to the group" do
             group_resource.run_action(tested_action)
 
             included_members.each do |member|
               expect(user_exist_in_group?(member)).to eq(true)
             end
-            excluded_members.each do |member|
+            (spec_members - included_members).each do |member|
               expect(user_exist_in_group?(member)).to eq(false)
             end
           end
         end
       end
 
-      describe "when the users doesn't exist" do
-        let (:append_expected_exception) do
-          ohai[:platform_family] == "windows" ? Chef::Exceptions::Win32APIError : Mixlib::ShellOut::ShellCommandFailed
+      describe "when append is set" do
+        before(:each) do
+          group_resource.append(true)
         end
-        describe "when append is not set" do
-          it "should raise an error" do
-            expect { group_resource.run_action(tested_action) }.to raise_error(append_expected_exception)
+
+        it "should add included members to the group" do
+          group_resource.run_action(tested_action)
+
+          included_members.each do |member|
+            expect(user_exist_in_group?(member)).to eq(true)
+          end
+          excluded_members.each do |member|
+            expect(user_exist_in_group?(member)).to eq(false)
           end
         end
 
-        describe "when append is set" do
-          before do
-            group_resource.append(true)
+        describe "when group already contains some users" do
+          before(:each) do
+            add_members_to_group([included_members[0], excluded_members[0]])
           end
-          it "should raise an error" do
-            expect { group_resource.run_action(tested_action) }.to raise_error(append_expected_exception)
+
+          it "should add the included users and remove excluded users" do
+            group_resource.run_action(tested_action)
+
+            included_members.each do |member|
+              expect(user_exist_in_group?(member)).to eq(true)
+            end
+            excluded_members.each do |member|
+              expect(user_exist_in_group?(member)).to eq(false)
+            end
           end
         end
       end