Merge pull request #4621 from chef/cd/fix-choco-error

Fix the Chocolatey-missing error again

2 files changed, 22 insertions, 28 deletions

diff --git a/lib/chef/provider/package/chocolatey.rb b/lib/chef/provider/package/chocolatey.rb
index 6804e99a13..44fb1de235 100644
--- a/lib/chef/provider/package/chocolatey.rb
+++ b/lib/chef/provider/package/chocolatey.rb
@@ -52,12 +52,7 @@ EOS
         def define_resource_requirements
           super
 
-          requirements.assert(:all_actions) do |a|
-            # GetEnvironmentVariable returns "" on failure.
-            a.assertion { !choco_install_path.to_s.empty? }
-            a.failure_message(Chef::Exceptions::MissingLibrary, CHOCO_MISSING_MSG)
-            a.whyrun("Assuming Chocolatey is installed")
-          end
+          # The check that Chocolatey is installed is in #choco_exe.
 
           # Chocolatey source attribute points to an alternate feed
           # and not a package specific alternate source like other providers
@@ -152,11 +147,13 @@ EOS
         #
         # @return [String] full path of choco.exe
         def choco_exe
-          @choco_exe ||= ::File.join(
-            choco_install_path,
-            "bin",
-            "choco.exe"
-            )
+          @choco_exe ||= begin
+              # if this check is in #define_resource_requirements, it won't get
+              # run before choco.exe gets called from #load_current_resource.
+              exe_path = ::File.join(choco_install_path.to_s, "bin", "choco.exe")
+              raise Chef::Exceptions::MissingLibrary, CHOCO_MISSING_MSG unless ::File.exist?(exe_path)
+              exe_path
+            end
         end
 
         # lets us mock out an incorrect value for testing.
diff --git a/spec/unit/provider/package/chocolatey_spec.rb b/spec/unit/provider/package/chocolatey_spec.rb
index a347aa7ebd..8a69cf3da4 100644
--- a/spec/unit/provider/package/chocolatey_spec.rb
+++ b/spec/unit/provider/package/chocolatey_spec.rb
@@ -462,17 +462,6 @@ munin-node|1.6.1.20130823
       expect(new_resource).to be_updated_by_last_action
     end
   end
-
-  describe "#choco_exe" do
-    it "calls #choco_install_path" do
-      # un-stub #choco_exe from the before{} block.
-      allow(provider).to receive(:choco_exe).and_call_original
-      expect(provider).to receive(:choco_install_path).and_return("spork")
-      provider.instance_variable_set("@choco_exe", nil)
-
-      expect(provider.send(:choco_exe)).to match(%r{spork[/\\]bin[/\\]choco.exe})
-    end
-  end
 end
 
 describe "behavior when Chocolatey is not installed" do
@@ -486,19 +475,27 @@ describe "behavior when Chocolatey is not installed" do
   end
 
   before {
-    allow(provider).to receive(:choco_install_path).and_return("")
+    # the shellout sometimes returns "", but test nil to be safe.
+    allow(provider).to receive(:choco_install_path).and_return(nil)
     provider.instance_variable_set("@choco_install_path", nil)
 
     # we don't care what this returns, but we have to let it be called.
     allow(provider).to receive(:shell_out!).and_return(double(:stdout => ""))
   }
 
-  context "#define_resource_requirements" do
+  let(:error_regex) {
+    /Could not locate.*install.*cookbook.*PowerShell.*GetEnvironmentVariable/m
+  }
+
+  context "#choco_exe" do
     it "triggers a MissingLibrary exception when Chocolatey is not installed" do
-      provider.action = :install
-      provider.load_current_resource
-      provider.define_resource_requirements
-      expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::MissingLibrary, /Could not locate.*install.*cookbook.*PowerShell.*GetEnvironmentVariable/m)
+      expect { provider.send(:choco_exe) }.to raise_error(Chef::Exceptions::MissingLibrary, error_regex)
+    end
+  end
+
+  context "#load_current_resource" do
+    it "triggers a MissingLibrary exception when Chocolatey is not installed" do
+      expect { provider.load_current_resource }.to raise_error(Chef::Exceptions::MissingLibrary, error_regex)
     end
   end
 end