Merge pull request #270 from rico89/fix/holdable

update holdable feature

lib/puppet/provider/package/chocolatey.rb

@@ -107,9 +107,6 @@ def install
     PuppetX::Chocolatey::ChocolateyCommon.set_env_chocolateyinstall
     choco_exe = compiled_choco?
 
-    # always unhold on install
-    unhold if choco_exe
-
     args = []
 
     # also will need to address -sidebyside or -m in the install args to allow
@@ -174,8 +171,8 @@ def uninstall
     PuppetX::Chocolatey::ChocolateyCommon.set_env_chocolateyinstall
     choco_exe = compiled_choco?
 
-    # always unhold on uninstall
-    unhold if choco_exe
+    # unhold on uninstall if package is on hold
+    unhold if on_hold?
 
     args = 'uninstall', @resource[:name][%r{\A\S*}]
 
@@ -207,8 +204,8 @@ def update
     PuppetX::Chocolatey::ChocolateyCommon.set_env_chocolateyinstall
     choco_exe = compiled_choco?
 
-    # always unhold on upgrade
-    unhold if choco_exe
+    # unhold on upgrade if package is on hold
+    unhold if on_hold?
 
     args = []
 
@@ -292,8 +289,13 @@ def self.instances
                    else
                      line.split(' ')
                    end
-          values[1] = :held if pins.include? values[0]
-          packages << new(name: values[0].downcase, ensure: values[1], provider: name)
+          package = {
+            name: values[0].downcase,
+            ensure: values[1],
+            provider: name
+          }
+          package[:mark] = :hold if pins.include? package[:name]
+          packages << new(package)
         end
       end
     rescue Puppet::ExecutionFailure
@@ -351,11 +353,15 @@ def latest
     package_ver
   end
 
-  def hold
+  def deprecated_hold
     raise ArgumentError, 'Only choco v0.9.9+ can use ensure => held' unless compiled_choco?
 
     install
 
+    hold
+  end
+
+  def hold
     args = 'pin', 'add', '-n', @resource[:name][%r{\A\S*}]
 
     chocolatey(*args)
@@ -364,7 +370,14 @@ def hold
   def unhold
     return unless compiled_choco?
 
-    Puppet::Util::Execution.execute([command(:chocolatey), 'pin', 'remove', '-n', @resource[:name][%r{\A\S*}]], failonfail: false)
+    args = 'pin', 'remove', '-n', @resource[:name][%r{\A\S*}]
+
+    chocolatey(*args)
+  end
+
+  def on_hold?
+    return false unless compiled_choco?
+    properties[:mark] == :hold
   end
 
   def package_settings

spec/unit/puppet/provider/package/chocolatey_spec.rb

@@ -397,7 +397,7 @@
         expect(provider).to receive(:chocolatey).with('install', 'chocolatey', '-y', nil)
         expect(provider).to receive(:chocolatey).with('pin', 'add', '-n', 'chocolatey')
 
-        provider.hold
+        provider.deprecated_hold
       end
     end
 
@@ -409,14 +409,15 @@
       it 'throws an argument error with held package' do
         resource[:ensure] = :held
 
-        expect { provider.hold }.to raise_error(ArgumentError, 'Only choco v0.9.9+ can use ensure => held')
+        expect { provider.deprecated_hold }.to raise_error(ArgumentError, 'Only choco v0.9.9+ can use ensure => held')
       end
     end
   end
 
   context 'when uninstalling' do
     context 'with compiled choco client' do
       before :each do
+        allow(provider).to receive(:on_hold?).and_return(false)
         allow(provider).to receive(:use_package_exit_codes_feature_enabled?).and_return(false)
         allow(provider.class).to receive(:compiled_choco?).and_return(true)
         allow(PuppetX::Chocolatey::ChocolateyVersion).to receive(:version).and_return(first_compiled_choco_version)
@@ -466,6 +467,14 @@
 
         provider.uninstall
       end
+
+      it 'calls the unhold method if the current package is on hold' do
+        allow(provider).to receive(:on_hold?).and_return(true)
+        expect(provider).to receive(:chocolatey).with('pin', 'remove', '-n', 'chocolatey')
+        expect(provider).to receive(:chocolatey).with('uninstall', 'chocolatey', '-fy', nil)
+
+        provider.uninstall
+      end
     end
 
     context 'with posh choco client' do
@@ -492,6 +501,7 @@
   context 'when updating' do
     context 'with compiled choco client' do
       before :each do
+        allow(provider).to receive(:on_hold?).and_return(false)
         allow(provider).to receive(:use_package_exit_codes_feature_enabled?).and_return(false)
         allow(provider.class).to receive(:compiled_choco?).and_return(true)
         allow(PuppetX::Chocolatey::ChocolateyVersion).to receive(:version).and_return(first_compiled_choco_version)
@@ -580,6 +590,17 @@
 
         provider.update
       end
+
+      it 'calls the unhold method if the current package is on hold' do
+        expect(provider_class).to receive(:instances).and_return [provider_class.new(ensure: 'latest',
+                                                                                     name: 'chocolatey',
+                                                                                     provider: :chocolatey)]
+        allow(provider).to receive(:on_hold?).and_return(true)
+        expect(provider).to receive(:chocolatey).with('pin', 'remove', '-n', 'chocolatey')
+        expect(provider).to receive(:chocolatey).with('upgrade', 'chocolatey', '-y', nil)
+
+        provider.update
+      end
     end
 
     context 'with posh choco client' do
@@ -740,6 +761,32 @@
                                                name: 'package2')
         end
 
+        it 'returns installed packages with their versions and mark propperty' do
+          expect(provider_class).to receive(:execpipe).and_yield(StringIO.new(%(package1|1.23\n\package2|2.00\n\package3|3.00\n)))
+          expect(Puppet::Util::Execution).to receive(:execute).and_return("Chocolatey v0.10.15 Business\npackage1|1.23|\npackage2|2.0|\n")
+
+          packages = provider_class.instances
+
+          expect(packages.length).to eq(3)
+
+          expect(packages[0].properties).to eq(provider: :chocolatey,
+                                               ensure: '1.23',
+                                               name: 'package1',
+                                               mark: :hold)
+          expect(packages[0].on_hold?).to eq(true)
+
+          expect(packages[1].properties).to eq(provider: :chocolatey,
+                                               ensure: '2.00',
+                                               name: 'package2',
+                                               mark: :hold)
+          expect(packages[1].on_hold?).to eq(true)
+
+          expect(packages[2].properties).to eq(provider: :chocolatey,
+                                               ensure: '3.00',
+                                               name: 'package3')
+          expect(packages[2].on_hold?).to eq(false)
+        end
+
         it 'returns nil on error' do
           expect(provider_class).to receive(:execpipe).and_yield(StringIO.new(%(Unable to search for packages when there are no soures enabled for packages and none were passed as arguments.\n)))