CHEF-2716: Don't compare nil versions, use the parent logic. Add missing

specs for the action_upgrade workaround.
jfcartkeep · Mar 16, 2012 · d8336f6 · d8336f6
1 parent 75981fe
commit d8336f6
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 2 deletions.
diff --git a/chef/lib/chef/provider/package/yum.rb b/chef/lib/chef/provider/package/yum.rb
@@ -1052,6 +1052,7 @@ def install_package(name, version)
                                                "and release? (version-release, e.g. 1.84-10.fc6)"
             end
           end
+
           if flush_cache[:after]
             @yum.reload
           else
@@ -1067,10 +1068,12 @@ def install_package(name, version)
         #
         # Hacky - better overall solution? Custom compare in Package provider?
         def action_upgrade
+          # Could be uninstalled or have no candidate
+          if @current_resource.version.nil? || candidate_version.nil? 
+            super
           # Ensure the candidate is newer
-          if RPMVersion.parse(candidate_version) > RPMVersion.parse(@current_resource.version)
+          elsif RPMVersion.parse(candidate_version) > RPMVersion.parse(@current_resource.version)
             super
-          # Candidate is older
           else
             Chef::Log.debug("#{@new_resource} is at the latest version - nothing to do")
           end
@@ -1086,6 +1089,7 @@ def remove_package(name, version)
           else
             yum_command("yum -d0 -e0 -y#{expand_options(@new_resource.options)} remove #{name}#{yum_arch}")
           end
+
           if flush_cache[:after]
             @yum.reload
           else

diff --git a/chef/spec/unit/provider/package/yum_spec.rb b/chef/spec/unit/provider/package/yum_spec.rb
@@ -488,6 +488,47 @@
       @provider.load_current_resource
       lambda { @provider.upgrade_package("cups", "1.2.4-11.15.el5") }.should raise_error(Chef::Exceptions::Package, %r{is newer than candidate package})
     end
+
+    # Test our little workaround, some crossover into Chef::Provider::Package territory
+    it "should call action_upgrade in the parent if the current resource version is nil" do
+      @yum_cache.stub!(:installed_version).and_return(nil)
+      @provider.load_current_resource
+      @current_resource = Chef::Resource::Package.new('cups')
+      @provider.candidate_version = '11'
+      @provider.should_receive(:upgrade_package).with(
+        "cups",
+        "11"
+      )
+      @provider.action_upgrade
+    end
+
+    it "should call action_upgrade in the parent if the candidate version is nil" do
+      @provider.load_current_resource
+      @current_resource = Chef::Resource::Package.new('cups')
+      @provider.candidate_version = nil 
+      @provider.should_not_receive(:upgrade_package)
+      @provider.action_upgrade
+    end
+
+    it "should call action_upgrade in the parent if the candidate is newer" do
+      @provider.load_current_resource
+      @current_resource = Chef::Resource::Package.new('cups')
+      @provider.candidate_version = '11'
+      @provider.should_receive(:upgrade_package).with(
+        "cups",
+        "11"
+      )
+      @provider.action_upgrade
+    end
+
+    it "should not call action_upgrade in the parent if the candidate is older" do
+      @yum_cache.stub!(:installed_version).and_return("12")
+      @provider.load_current_resource
+      @current_resource = Chef::Resource::Package.new('cups')
+      @provider.candidate_version = '11'
+      @provider.should_not_receive(:upgrade_package)
+      @provider.action_upgrade
+    end
   end
 
   describe "when removing a package" do