Updated the spec and library to be more robust / correct.

2 files changed, 69 insertions, 21 deletions

diff --git a/chef/lib/chef/provider/package/macports.rb b/chef/lib/chef/provider/package/macports.rb
index c9caef8840..071d52939d 100644
--- a/chef/lib/chef/provider/package/macports.rb
+++ b/chef/lib/chef/provider/package/macports.rb
@@ -10,26 +10,31 @@ class Chef
           Chef::Log.debug("Current version is #{@current_resource.version}") if @current_resource.version
 
           @candidate_version = macports_candidate_version
+
+          if !@new_resource.version and !@candidate_version
+            raise Chef::Exceptions::Package, "Could not get a candidate version for this package -- #{@new_resource.name} does not seem to be a valid package!"
+          end
+
           Chef::Log.debug("MacPorts candidate version is #{@candidate_version}") if @candidate_version
 
           @current_resource
         end
 
         def current_installed_version
-          command = "port installed #{@new_resource.package_name} | grep \"(active)\""
-          output = get_line_from_command(command)
-
-          if output.empty?
-            nil
-          else
-            match = output.match(/^.+ @([^\s]+) \(active\)$/)
-            match[1]
+          command = "port installed #{@new_resource.package_name}"
+          output = get_response_from_command(command)
+
+          response = nil
+          output.each_line do |line|
+            match = line.match(/^.+ @([^\s]+) \(active\)$/)
+            response = match[1] if match
           end
+          response
         end
 
         def macports_candidate_version
           command = "port info --version #{@new_resource.package_name}"
-          output = get_line_from_command(command)
+          output = get_response_from_command(command)
 
           match = output.match(/^version: (.+)$/)
 
@@ -38,26 +43,41 @@ class Chef
 
         def install_package(name, version)
           unless @current_resource.version == version
+            command = "port install #{name}"
+            command << " @#{version}" if version and !version.empty? 
             run_command(
-              :command => "port install #{name} @#{version}"
+              :command => command
             )
           end
         end
 
         def purge_package(name, version)
+          command = "port uninstall #{name}"
+          command << " @#{version}" if version and !version.empty?
           run_command(
-            :command => "port uninstall #{name} @#{version}"
+            :command => command
           )
         end
 
         def remove_package(name, version)
+          command = "port deactivate #{name}"
+          command << " @#{version}" if version and !version.empty?
+
           run_command(
-            :command => "port deactivate #{name} @#{version}"
+            :command => command
           )
         end
 
         def upgrade_package(name, version)
-          unless @current_resource.version == version
+          # Saving this to a variable -- weird rSpec behavior
+          # happens otherwise...
+          current_version = @current_resource.version
+
+          if current_version.nil? or current_version.empty?
+            # Macports doesn't like when you upgrade a package
+            # that hasn't been installed.
+            install_package(name, version)
+          elsif current_version != version
             run_command(
               :command => "port upgrade #{name} @#{version}"
             )
@@ -65,10 +85,14 @@ class Chef
         end
 
         private
-        def get_line_from_command(command)
+        def get_response_from_command(command)
           output = nil
           status = popen4(command) do |pid, stdin, stdout, stderr|
-            output = stdout.readline.strip
+            begin
+              output = stdout.read
+            rescue Exception
+              raise Chef::Exceptions::Package, "Could not read from STDOUT on command: #{command}"
+            end
           end
           unless status.exitstatus == 0 || status.exitstatus == 1
             raise Chef::Exceptions::Package, "#{command} failed - #{status.insect}!"
diff --git a/chef/spec/unit/provider/package/macports_spec.rb b/chef/spec/unit/provider/package/macports_spec.rb
index d49c208bbe..ca004116ac 100644
--- a/chef/spec/unit/provider/package/macports_spec.rb
+++ b/chef/spec/unit/provider/package/macports_spec.rb
@@ -78,13 +78,18 @@ describe Chef::Provider::Package::Macports do
 
   describe "current_installed_version" do
     it "should return the current version if the package is installed" do
-      @stdout.should_receive(:readline).and_return("  openssl @0.9.8k_0 (active)\n")
+      @stdout.should_receive(:read).and_return(<<EOF
+The following ports are currently installed:
+  openssl @0.9.8k_0 (active)
+EOF
+      )
+
       @provider.should_receive(:popen4).and_yield(@pid, @stdin, @stdout, @stderr).and_return(@status)
       @provider.current_installed_version.should == "0.9.8k_0"
     end
 
     it "should return nil if a package is not currently installed" do
-      @stdout.should_receive(:readline).and_return("       \n")
+      @stdout.should_receive(:read).and_return("       \n")
       @provider.should_receive(:popen4).and_yield(@pid, @stdin, @stdout, @stderr).and_return(@status)
       @provider.current_installed_version.should be_nil
     end
@@ -92,13 +97,13 @@ describe Chef::Provider::Package::Macports do
 
   describe "macports_candidate_version" do
     it "should return the latest available version of a given package" do
-      @stdout.should_receive(:readline).and_return("version: 4.2.7\n")
+      @stdout.should_receive(:read).and_return("version: 4.2.7\n")
       @provider.should_receive(:popen4).and_yield(@pid, @stdin, @stdout, @stderr).and_return(@status)
       @provider.macports_candidate_version.should == "4.2.7"
     end
 
     it "should return nil if there is no version for a given package" do
-      @stdout.should_receive(:readline).and_return("Error: port fadsfadsfads not found\n")
+      @stdout.should_receive(:read).and_return("Error: port fadsfadsfads not found\n")
       @provider.should_receive(:popen4).and_yield(@pid, @stdin, @stdout, @stderr).and_return(@status)
       @provider.macports_candidate_version.should be_nil
     end
@@ -127,6 +132,11 @@ describe Chef::Provider::Package::Macports do
       @provider.should_receive(:run_command).with(:command => "port uninstall zsh @4.2.7")
       @provider.purge_package("zsh", "4.2.7")
     end
+
+    it "should purge the currently active version if no explicit version is passed in" do
+      @provider.should_receive(:run_command).with(:command => "port uninstall zsh")
+      @provider.purge_package("zsh", nil)
+    end
   end
 
   describe "remove_package" do
@@ -134,23 +144,37 @@ describe Chef::Provider::Package::Macports do
       @provider.should_receive(:run_command).with(:command => "port deactivate zsh @4.2.7")
       @provider.remove_package("zsh", "4.2.7")
     end
+
+    it "should remove the currently active version if no explicit version is passed in" do
+      @provider.should_receive(:run_command).with(:command => "port deactivate zsh")
+      @provider.remove_package("zsh", nil)
+    end
   end
 
   describe "upgrade_package" do
     it "should run the port upgrade command with the correct version" do
-      @current_resource.should_receive(:version).and_return("4.1.6")
+      @current_resource.should_receive(:version).at_least(:once).and_return("4.1.6")
       @provider.current_resource = @current_resource
+
       @provider.should_receive(:run_command).with(:command => "port upgrade zsh @4.2.7")
 
       @provider.upgrade_package("zsh", "4.2.7")
     end
 
     it "should not run the port upgrade command if the version is already installed" do
-      @current_resource.should_receive(:version).and_return("4.2.7")
+      @current_resource.should_receive(:version).at_least(:once).and_return("4.2.7")
       @provider.current_resource = @current_resource
       @provider.should_not_receive(:run_command)
 
       @provider.upgrade_package("zsh", "4.2.7")
     end
+
+    it "should call install_package if the package isn't currently installed" do
+      @current_resource.should_receive(:version).at_least(:once).and_return(nil)
+      @provider.current_resource = @current_resource
+      @provider.should_receive(:install_package).and_return(true)
+
+      @provider.upgrade_package("zsh", "4.2.7")
+    end
   end
 end