Did some more refactorign added more functional specs for missing scenarios

Signed-off-by: Vasu1105 <vasundhara.jagdale@msystechnologies.com>

2 files changed, 60 insertions, 20 deletions

diff --git a/lib/chef/resource/windows_user_privilege.rb b/lib/chef/resource/windows_user_privilege.rb
index 338fa5f680..85f1557d09 100644
--- a/lib/chef/resource/windows_user_privilege.rb
+++ b/lib/chef/resource/windows_user_privilege.rb
@@ -82,9 +82,10 @@ class Chef
       property :privilege, [Array, String],
         description: "Privilege to set for users.",
         required: true,
+        coerce: proc { |v| v.is_a?(String) ? Array[v] : v },
         callbacks: {
            "Option privilege must include any of the: #{privilege_opts}" => lambda {
-             |v| v.is_a?(Array) ? (privilege_opts & v).size == v.size : privilege_opts.include?(v)
+             |v| (privilege_opts & v).size == v.size
            },
          }
 
@@ -137,14 +138,13 @@ class Chef
 
       action :remove do
         curr_res_privilege = current_resource.privilege
-        new_res_privilege = new_resource.privilege.is_a?(String) ? Array(new_resource.privilege) : new_resource.privilege
-        missing_res_privileges = (new_res_privilege - curr_res_privilege)
+        missing_res_privileges = (new_resource.privilege - curr_res_privilege)
 
         if missing_res_privileges
           Chef::Log.info("User \'#{new_resource.principal}\' for Privilege: #{missing_res_privileges.join(", ")} not found. Nothing to remove.")
         end
 
-        (new_res_privilege - missing_res_privileges).each do |user_right|
+        (new_resource.privilege - missing_res_privileges).each do |user_right|
           converge_by("removing user privilege #{user_right}") do
             Chef::ReservedNames::Win32::Security.remove_account_right(new_resource.principal, user_right)
           end
diff --git a/spec/functional/resource/windows_user_privilege_spec.rb b/spec/functional/resource/windows_user_privilege_spec.rb
index fa134b4fe7..9bfb1d731e 100644
--- a/spec/functional/resource/windows_user_privilege_spec.rb
+++ b/spec/functional/resource/windows_user_privilege_spec.rb
@@ -19,8 +19,6 @@ require_relative "../../spec_helper"
 require_relative "../../functional/resource/base"
 
 describe Chef::Resource::WindowsUserPrivilege, :windows_only do
-  include Chef::Mixin::PowershellExec
-
   let(:principal) { nil }
   let(:privilege) { nil }
   let(:users) { nil }
@@ -48,18 +46,36 @@ describe Chef::Resource::WindowsUserPrivilege, :windows_only do
   describe "#add privilege" do
     after { subject.run_action(:remove) }
 
-    let(:principal) { "Administrator" }
-    let(:privilege) { "SeCreateSymbolicLinkPrivilege" }
+    context "when privilege is passed as string" do
+      let(:principal) { "Administrator" }
+      let(:privilege) { "SeCreateSymbolicLinkPrivilege" }
 
-    it "adds user to privilege" do
-      subject.run_action(:add)
-      expect(subject).to be_updated_by_last_action
+      it "adds user to privilege" do
+        subject.run_action(:add)
+        expect(subject).to be_updated_by_last_action
+      end
+
+      it "is idempotent" do
+        subject.run_action(:add)
+        subject.run_action(:add)
+        expect(subject).not_to be_updated_by_last_action
+      end
     end
 
-    it "is idempotent" do
-      subject.run_action(:add)
-      subject.run_action(:add)
-      expect(subject).not_to be_updated_by_last_action
+    context "when privilege is passed as array" do
+      let(:principal) { "Administrator" }
+      let(:privilege) { ["SeCreateSymbolicLinkPrivilege", "SeCreatePagefilePrivilege"] }
+
+      it "adds user to privilege" do
+        subject.run_action(:add)
+        expect(subject).to be_updated_by_last_action
+      end
+
+      it "is idempotent" do
+        subject.run_action(:add)
+        subject.run_action(:add)
+        expect(subject).not_to be_updated_by_last_action
+      end
     end
   end
 
@@ -92,12 +108,36 @@ describe Chef::Resource::WindowsUserPrivilege, :windows_only do
 
   describe "#remove privilege" do
     let(:principal) { "Administrator" }
-    let(:privilege) { "SeCreateSymbolicLinkPrivilege" }
+    context "when privilege is passed as array" do
+      let(:privilege) { "SeCreateSymbolicLinkPrivilege" }
+      it "remove user from privilege" do
+        subject.run_action(:add)
+        subject.run_action(:remove)
+        expect(subject).to be_updated_by_last_action
+      end
+
+      it "is idempotent" do
+        subject.run_action(:add)
+        subject.run_action(:remove)
+        subject.run_action(:remove)
+        expect(subject).not_to be_updated_by_last_action
+      end
+    end
 
-    it "remove user from privilege" do
-      subject.run_action(:add)
-      subject.run_action(:remove)
-      expect(subject).to be_updated_by_last_action
+    context "when privilege is passed as array" do
+      let(:privilege) { ["SeCreateSymbolicLinkPrivilege", "SeCreatePagefilePrivilege"] }
+      it "remove user from privilege" do
+        subject.run_action(:add)
+        subject.run_action(:remove)
+        expect(subject).to be_updated_by_last_action
+      end
+
+      it "is idempotent" do
+        subject.run_action(:add)
+        subject.run_action(:remove)
+        subject.run_action(:remove)
+        expect(subject).not_to be_updated_by_last_action
+      end
     end
   end