Migrating the apt and dpkg tests to rspec.

I left the aptitude and aptrpm tests as an exercise
for the reader.

Signed-off-by: Luke Kanies <luke@madstop.com>

6 files changed, 346 insertions, 303 deletions

diff --git a/lib/puppet/provider/package/apt.rb b/lib/puppet/provider/package/apt.rb
index d0e520f0a..80465129d 100755
--- a/lib/puppet/provider/package/apt.rb
+++ b/lib/puppet/provider/package/apt.rb
@@ -45,9 +45,19 @@ Puppet::Type.type(:package).provide :apt, :parent => :dpkg, :source => :dpkg do
         if @resource[:responsefile]
             self.run_preseed
         end
-        should = @resource.should(:ensure)
+        should = @resource[:ensure]
 
         checkforcdrom()
+        cmd = %w{-q -y}
+
+        keep = ""
+        if config = @resource[:configfiles]
+            if config == :keep
+                cmd << "-o" << 'DPkg::Options::=--force-confold'
+            else
+                cmd << "-o" << 'DPkg::Options::=--force-confnew'
+            end
+        end
 
         str = @resource[:name]
         case should
@@ -57,19 +67,6 @@ Puppet::Type.type(:package).provide :apt, :parent => :dpkg, :source => :dpkg do
             # Add the package version
             str += "=%s" % should
         end
-        cmd = %w{-q -y}
-
-        keep = ""
-        if config = @resource[:configfiles]
-            case config
-            when :keep
-                cmd << "-o" << 'DPkg::Options::=--force-confold'
-            when :replace
-                cmd << "-o" << 'DPkg::Options::=--force-confnew'
-            else
-                raise Puppet::Error, "Invalid 'configfiles' value %s" % config
-            end
-        end
 
         cmd << :install << str
         
@@ -92,7 +89,7 @@ Puppet::Type.type(:package).provide :apt, :parent => :dpkg, :source => :dpkg do
 	# preseeds answers to dpkg-set-selection from the "responsefile"
 	#
     def run_preseed
-        if response = @resource[:responsefile] and FileTest.exists?(response)
+        if response = @resource[:responsefile] and FileTest.exist?(response)
             self.info("Preseeding %s to debconf-set-selections" % response)
 
             preseed response
@@ -101,16 +98,12 @@ Puppet::Type.type(:package).provide :apt, :parent => :dpkg, :source => :dpkg do
         end
     end
 
-    def update
-        self.install
-    end
-
     def uninstall
         aptget "-y", "-q", :remove, @resource[:name]
     end
 
     def purge
-        aptget '-y', '-q', 'remove', '--purge', @resource[:name]
+        aptget '-y', '-q', :remove, '--purge', @resource[:name]
         # workaround a "bug" in apt, that already removed packages are not purged
         super
     end
diff --git a/lib/puppet/provider/package/dpkg.rb b/lib/puppet/provider/package/dpkg.rb
index ae79f714c..67d31a592 100755
--- a/lib/puppet/provider/package/dpkg.rb
+++ b/lib/puppet/provider/package/dpkg.rb
@@ -23,30 +23,39 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
 
             # now turn each returned line into a package object
             process.each { |line|
-                if match = regex.match(line)
-                    hash = {}
+                if hash = parse_line(line)
+                    packages << new(hash)
+                end
+            }
+        end
 
-                    fields.zip(match.captures) { |field,value|
-                        hash[field] = value
-                    }
+        return packages
+    end
 
-                    hash[:provider] = self.name
+    REGEX = %r{^(\S+) +(\S+) +(\S+) (\S+) (\S*)$}
+    FIELDS = [:desired, :error, :status, :name, :ensure]
 
-                    if hash[:status] == 'not-installed'
-                        hash[:ensure] = :purged
-                    elsif hash[:status] != "installed"
-                        hash[:ensure] = :absent
-                    end
+    def self.parse_line(line)
+        if match = REGEX.match(line)
+            hash = {}
 
-                    packages << new(hash)
-                else
-                    Puppet.warning "Failed to match dpkg-query line %s" %
-                        line.inspect
-                end
+            FIELDS.zip(match.captures) { |field,value|
+                hash[field] = value
             }
+
+            hash[:provider] = self.name
+
+            if hash[:status] == 'not-installed'
+                hash[:ensure] = :purged
+            elsif hash[:status] != "installed"
+                hash[:ensure] = :absent
+            end
+        else
+            Puppet.warning "Failed to match dpkg-query line %s" % line.inspect
+            return nil
         end
 
-        return packages
+        return hash
     end
 
     def install
@@ -56,15 +65,10 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
         
         args = []
 
-        if config = @resource[:configfiles]
-            case config
-            when :keep
-                args << '--force-confold'
-            when :replace
-                args << '--force-confnew'
-            else
-                raise Puppet::Error, "Invalid 'configfiles' value %s" % config
-            end
+        if @resource[:configfiles] == :keep
+            args << '--force-confold'
+        else
+            args << '--force-confnew'
         end
         args << '-i' << file
 
@@ -80,7 +84,7 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
         output = dpkg_deb "--show", @resource[:source]
         matches = /^(\S+)\t(\S+)$/.match(output).captures
         unless matches[0].match(@resource[:name])
-            Puppet.warning "source doesn't contain named package, but %s" % matches[0]
+            warning "source doesn't contain named package, but %s" % matches[0]
         end
         matches[1]
     end
@@ -103,22 +107,8 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
                 :name => @resource[:name], :error => 'ok'}
 
         end
-        # Our regex for matching dpkg-query output.  We could probably just
-        # use split here, but I'm not positive that dpkg-query will never
-        # return whitespace.
-        regex = %r{^(\S+) (\S+) (\S+) (\S+) (\S*)$}
 
-        line = output.split("\n").shift.chomp
-        
-        if match = regex.match(line)
-            fields.zip(match.captures) { |field,value|
-                hash[field] = value
-            }
-        else
-            notice "Failed to handle dpkg-query line %s" % line.inspect
-            return {:ensure => :absent, :status => 'missing',
-                :name => @resource[:name], :error => 'ok'}
-        end
+        hash = self.class.parse_line(output) || {:ensure => :absent, :status => 'missing', :name => @resource[:name], :error => 'ok'}
 
         if hash[:error] != "ok"
             raise Puppet::Error.new(
@@ -127,13 +117,6 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
             )
         end
 
-        # DPKG can discuss packages that are no longer installed, so allow that.
-        if hash[:status] == "not-installed"
-            hash[:ensure] = :purged
-        elsif hash[:status] != "installed"
-            hash[:ensure] = :absent
-        end
-
         return hash
     end
 
@@ -145,4 +128,3 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
         dpkg "--purge", @resource[:name]
     end
 end
-
diff --git a/spec/unit/provider/package/apt.rb b/spec/unit/provider/package/apt.rb
new file mode 100755
index 000000000..0ec1cd9ed
--- /dev/null
+++ b/spec/unit/provider/package/apt.rb
@@ -0,0 +1,138 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+provider = Puppet::Type.type(:package).provider(:apt)
+
+describe provider do
+    before do
+        @resource = stub 'resource', :[] => "asdf"
+        @provider = provider.new(@resource)
+
+        @fakeresult = "install ok installed asdf 1.0\n"
+    end
+
+    it "should be versionable" do
+        provider.should be_versionable
+    end
+
+    it "should use :install to update" do
+        @provider.expects(:install)
+        @provider.update
+    end
+
+    it "should use 'apt-get remove' to uninstall" do
+        @provider.expects(:aptget).with("-y", "-q", :remove, "asdf")
+
+        @provider.uninstall
+    end
+
+    it "should use 'apt-get purge' and 'dpkg purge' to purge" do
+        @provider.expects(:aptget).with("-y", "-q", :remove, "--purge", "asdf")
+        @provider.expects(:dpkg).with("--purge", "asdf")
+
+        @provider.purge
+    end
+
+    it "should use 'apt-cache policy' to determine the latest version of a package" do
+        @provider.expects(:aptcache).with(:policy, "asdf").returns "asdf:
+Installed: 1:1.0
+Candidate: 1:1.1
+Version table:
+ 1:1.0
+    650 http://ftp.osuosl.org testing/main Packages
+*** 1:1.1
+    100 /var/lib/dpkg/status"
+
+        @provider.latest.should == "1:1.1"
+    end
+
+    it "should print and error and return nil if no policy is found" do
+        @provider.expects(:aptcache).with(:policy, "asdf").returns "asdf:"
+
+        @provider.expects(:err)
+        @provider.latest.should be_nil
+    end
+
+    it "should be able to preseed" do
+        @provider.should respond_to(:run_preseed)
+    end
+
+    it "should preseed with the provided responsefile when preseeding is called for" do
+        @resource.expects(:[]).with(:responsefile).returns "/my/file"
+        FileTest.expects(:exist?).with("/my/file").returns true
+
+        @provider.expects(:info)
+        @provider.expects(:preseed).with("/my/file")
+
+        @provider.run_preseed
+    end
+
+    it "should not preseed if no responsefile is provided" do
+        @resource.expects(:[]).with(:responsefile).returns nil
+
+        @provider.expects(:info)
+        @provider.expects(:preseed).never
+
+        @provider.run_preseed
+    end
+
+    it "should fail if a cdrom is listed in the sources list and :allowcdrom is not specified"
+
+    describe "when installing" do
+        it "should preseed if a responsefile is provided" do
+            @resource.expects(:[]).with(:responsefile).returns "/my/file"
+            @provider.expects(:run_preseed)
+
+            @provider.stubs(:aptget)
+            @provider.install
+        end
+
+        it "should check for a cdrom" do
+            @provider.expects(:checkforcdrom)
+
+            @provider.stubs(:aptget)
+            @provider.install
+        end
+
+        it "should use 'apt-get install' with the package name if no version is asked for" do
+            @resource.expects(:[]).with(:ensure).returns :installed
+            @provider.expects(:aptget).with { |*command| command[-1] == "asdf" and command[-2] == :install }
+            
+            @provider.install
+        end
+
+        it "should specify the package version if one is asked for" do
+            @resource.expects(:[]).with(:ensure).returns "1.0"
+            @provider.expects(:aptget).with { |*command| command[-1] == "asdf=1.0" }
+            
+            @provider.install
+        end
+
+        it "should do a quiet install" do
+            @provider.expects(:aptget).with { |*command| command.include?("-q") }
+            
+            @provider.install
+        end
+
+        it "should default to 'yes' for all questions" do
+            @provider.expects(:aptget).with { |*command| command.include?("-y") }
+            
+            @provider.install
+        end
+
+        it "should keep config files if asked" do
+            @resource.expects(:[]).with(:configfiles).returns :keep
+            @provider.expects(:aptget).with { |*command| command.include?("DPkg::Options::=--force-confold") }
+            
+            @provider.install
+        end
+
+        it "should replace config files if asked" do
+            @resource.expects(:[]).with(:configfiles).returns :replace
+            @provider.expects(:aptget).with { |*command| command.include?("DPkg::Options::=--force-confnew") }
+            
+            @provider.install
+        end
+    end
+end
diff --git a/spec/unit/provider/package/dpkg.rb b/spec/unit/provider/package/dpkg.rb
new file mode 100755
index 000000000..08aaca875
--- /dev/null
+++ b/spec/unit/provider/package/dpkg.rb
@@ -0,0 +1,163 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+provider = Puppet::Type.type(:package).provider(:dpkg)
+
+describe provider do
+    before do
+        @resource = stub 'resource', :[] => "asdf"
+        @provider = provider.new(@resource)
+
+        @fakeresult = "install ok installed asdf 1.0\n"
+    end
+
+    it "should have documentation" do
+        provider.doc.should be_instance_of(String)
+    end
+
+    describe "when listing all instances" do
+        before do
+            provider.stubs(:command).with(:dpkgquery).returns "myquery"
+        end
+
+        it "should use dpkg-query" do
+            provider.expects(:command).with(:dpkgquery).returns "myquery"
+            provider.expects(:execpipe).with("myquery -W --showformat '${Status} ${Package} ${Version}\\n'").returns @fakeresult
+
+            provider.instances
+        end
+
+        it "should create and return an instance with each parsed line from dpkg-query" do
+            pipe = mock 'pipe'
+            pipe.expects(:each).yields @fakeresult
+            provider.expects(:execpipe).yields pipe
+
+            asdf = mock 'pkg1'
+            provider.expects(:new).with(:ensure => "1.0", :error => "ok", :desired => "install", :name => "asdf", :status => "installed", :provider => :dpkg).returns asdf
+
+            provider.instances.should == [asdf]
+        end
+
+        it "should warn on and ignore any lines it does not understand" do
+            pipe = mock 'pipe'
+            pipe.expects(:each).yields "foobar"
+            provider.expects(:execpipe).yields pipe
+
+            Puppet.expects(:warning)
+            provider.expects(:new).never
+
+            provider.instances.should == []
+        end
+    end
+
+    describe "when querying the current state" do
+        it "should use dpkg-query" do
+            @provider.expects(:dpkgquery).with("-W", "--showformat",'${Status} ${Package} ${Version}\\n', "asdf").returns @fakeresult
+
+            @provider.query
+        end
+
+        it "should consider the package purged if dpkg-query fails" do
+            @provider.expects(:dpkgquery).raises Puppet::ExecutionFailure.new("eh")
+
+            @provider.query[:ensure].should == :purged
+        end
+
+        it "should return a hash of the found status with the desired state, error state, status, name, and 'ensure'" do
+            @provider.expects(:dpkgquery).returns @fakeresult
+
+            @provider.query.should == {:ensure => "1.0", :error => "ok", :desired => "install", :name => "asdf", :status => "installed", :provider => :dpkg}
+        end
+
+        it "should consider the package absent if the dpkg-query result cannot be interpreted" do
+            @provider.expects(:dpkgquery).returns "somebaddata"
+
+            @provider.query[:ensure].should == :absent
+        end
+
+        it "should fail if an error is discovered" do
+            @provider.expects(:dpkgquery).returns @fakeresult.sub("ok", "error")
+
+            lambda { @provider.query }.should raise_error(Puppet::Error)
+        end
+
+        it "should consider the package purged if it is marked 'not-installed'" do
+            @provider.expects(:dpkgquery).returns @fakeresult.sub("installed", "not-installed")
+
+            @provider.query[:ensure].should == :purged
+        end
+
+        it "should consider the package absent if its status is neither 'installed' nor 'not-installed'" do
+            @provider.expects(:dpkgquery).returns @fakeresult.sub("installed", "foo")
+
+            @provider.query[:ensure].should == :absent
+        end
+    end
+
+    it "should be able to install" do
+        @provider.should respond_to(:install)
+    end
+
+    describe "when installing" do
+        before do
+            @resource.stubs(:[]).with(:source).returns "mypkg"
+        end
+
+        it "should fail to install if no source is specified in the resource" do
+            @resource.expects(:[]).with(:source).returns nil
+
+            lambda { @provider.install }.should raise_error(ArgumentError)
+        end
+
+        it "should use 'dpkg -i' to install the package" do
+            @resource.expects(:[]).with(:source).returns "mypackagefile"
+            @provider.expects(:dpkg).with { |*command| command[-1] == "mypackagefile"  and command[-2] == "-i" }
+
+            @provider.install
+        end
+
+        it "should keep old config files if told to do so" do
+            @resource.expects(:[]).with(:configfiles).returns :keep
+            @provider.expects(:dpkg).with { |*command| command[0] == "--force-confold" }
+
+            @provider.install
+        end
+
+        it "should replace old config files if told to do so" do
+            @resource.expects(:[]).with(:configfiles).returns :replace
+            @provider.expects(:dpkg).with { |*command| command[0] == "--force-confnew" }
+
+            @provider.install
+        end
+    end
+
+    it "should use :install to update" do
+        @provider.expects(:install)
+        @provider.update
+    end
+
+    describe "when determining latest available version" do
+        it "should return the version found by dpkg-deb" do
+            @resource.expects(:[]).with(:source).returns "myfile"
+            @provider.expects(:dpkg_deb).with { |*command| command[-1] == "myfile" }.returns "asdf\t1.0"
+            @provider.latest.should == "1.0"
+        end
+
+        it "should warn if the package file contains a different package" do
+            @provider.expects(:dpkg_deb).returns("foo\tversion")
+            @provider.expects(:warning)
+            @provider.latest
+        end
+    end
+
+    it "should use 'dpkg -r' to uninstall" do
+        @provider.expects(:dpkg).with("-r", "asdf")
+        @provider.uninstall
+    end
+
+    it "should use 'dpkg --purge' to purge" do
+        @provider.expects(:dpkg).with("--purge", "asdf")
+        @provider.purge
+    end
+end
diff --git a/test/ral/providers/package/apt.rb b/test/ral/providers/package/apt.rb
deleted file mode 100755
index f4fb1a95c..000000000
--- a/test/ral/providers/package/apt.rb
+++ /dev/null
@@ -1,169 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../../lib/puppettest'
-
-require 'mocha'
-
-class AptPackageProviderTest < PuppetTest::TestCase
-    confine "Apt package provider missing" =>
-        Puppet::Type.type(:package).provider(:apt).suitable?
-
-	def setup
-        super
-		@type = Puppet::Type.type(:package)
-	end
-	
-	def test_install
-		pkg = @type.create :name => 'faff',
-		                   :provider => :apt,
-		                   :ensure => :present,
-		                   :source => "/tmp/faff.deb"
-
-		pkg.provider.expects(
-		                 :dpkgquery
-					  ).with(
-							  '-W',
-							  '--showformat',
-							  '${Status} ${Package} ${Version}\n',
-							  'faff'
-					  ).returns(
-					        "deinstall ok config-files faff 1.2.3-1\n"
-					  ).times(1)
-
-		pkg.provider.expects(
-		                 :aptget
-		           ).with(
-		                 '-q',
-		                 '-y',
-		                 '-o',
-		                 'DPkg::Options::=--force-confold',
-		                 :install,
-		                 'faff'
-					  ).returns(0)
-		
-		pkg.evaluate.each { |state| state.transaction = self; state.forward }
-	end
-	
-	def test_purge
-		pkg = @type.create :name => 'faff', :provider => :apt, :ensure => :purged
-
-		pkg.provider.expects(
-		                 :dpkgquery
-					  ).with(
-					        '-W',
-					        '--showformat',
-					        '${Status} ${Package} ${Version}\n',
-					        'faff'
-					  ).returns(
-					        "install ok installed faff 1.2.3-1\n"
-					  ).times(1)
-		pkg.provider.expects(
-		                 :aptget
-					  ).with(
-					        '-y',
-					        '-q',
-					        'remove',
-					        '--purge',
-					        'faff'
-					  ).returns(0)
-		pkg.provider.expects(
-		                 :dpkg
-					  ).with(
-					        '--purge',
-					        'faff'
-					  ).returns(0)
-		
-		pkg.evaluate.each { |state| state.transaction = self; state.forward }
-	end
-
-	def test_dont_try_to_purge_if_already_purged
-		pkg = @type.create :name => 'faff', :provider => :apt, :ensure => :purged
-
-		pkg.provider.expects(
-		                 :dpkgquery
-					  ).with(
-					        '-W',
-					        '--showformat',
-					        '${Status} ${Package} ${Version}\n',
-					        'faff'
-					  ).raises(
-					        Puppet::ExecutionFailure,
-					        "No packages found matching faff.\n"
-					  ).times(1)
-		pkg.provider.expects(
-		                 :aptget
-					  ).never
-		
-		pkg.evaluate.each { |state| state.transaction = self; state.forward }
-	end
-
-	def test_dont_try_to_purge_if_already_purged_and_we_want_it_absent
-		pkg = @type.create :name => 'faff', :provider => :apt, :ensure => :absent
-
-		pkg.provider.expects(
-		                 :dpkgquery
-					  ).with(
-					        '-W',
-					        '--showformat',
-					        '${Status} ${Package} ${Version}\n',
-					        'faff'
-					  ).raises(
-					        Puppet::ExecutionFailure,
-					        "No packages found matching faff.\n"
-					  ).times(1)
-		pkg.provider.expects(
-		                 :aptget
-					  ).never
-
-		pkg.evaluate.each { |state| state.transaction = self; state.forward }
-	end
-
-	def test_make_sure_we_purge_if_package_still_has_conffiles
-		pkg = @type.create :name => 'faff', :provider => :apt, :ensure => :purged
-
-		pkg.provider.expects(
-		                 :dpkgquery
-					  ).with(
-					        '-W',
-					        '--showformat',
-					        '${Status} ${Package} ${Version}\n',
-					        'faff'
-					  ).returns(
-					        "deinstall ok config-files faff 1.2.3-1\n"
-					  ).times(1)
-		pkg.provider.expects(
-		                 :aptget
-					  ).with(
-					        '-y',
-					        '-q',
-					        'remove',
-					        '--purge',
-					        'faff'
-					  ).returns(0)
-		pkg.provider.expects(
-		                 :dpkg
-					  ).with(
-					        '--purge',
-					        'faff'
-					  ).returns(0)
-		
-		pkg.evaluate.each { |state| state.transaction = self; state.forward }
-	end
-	
-
-    def test_latest
-		pkg = @type.create :name => 'base-files', :provider => :apt
-
-        assert(pkg, "did not create pkg")
-        status = pkg.provider.query
-        assert(status, "base-files is not installed")
-        assert(status[:ensure] != :absent, "base-files is not installed")
-
-        latest = nil
-        assert_nothing_raised("Could not call latest") do
-            latest = pkg.provider.latest
-        end
-        assert(latest, "Could not get latest value from apt")
-    end
-end
-
diff --git a/test/ral/providers/package/dpkg.rb b/test/ral/providers/package/dpkg.rb
deleted file mode 100755
index 7e4427584..000000000
--- a/test/ral/providers/package/dpkg.rb
+++ /dev/null
@@ -1,64 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../../lib/puppettest'
-
-require 'mocha'
-
-class DpkgPackageProviderTest < PuppetTest::TestCase
-    confine "dpkg package provider missing" =>
-        Puppet::Type.type(:package).provider(:dpkg).suitable?
-	def setup
-        super
-		@type = Puppet::Type.type(:package)
-	end
-	
-	def test_install
-		pkg = @type.create :name => 'faff',
-		                   :provider => :dpkg,
-		                   :ensure => :present,
-		                   :source => "/tmp/faff.deb"
-
-		pkg.provider.expects(
-		                 :dpkgquery
-					  ).with(
-							  '-W',
-							  '--showformat',
-							  '${Status} ${Package} ${Version}\n',
-							  'faff'
-					  ).returns(
-					        "deinstall ok config-files faff 1.2.3-1\n"
-					  ).times(1)
-
-		pkg.provider.expects(
-		                 :dpkg
-		           ).with(
-		                 '-i',
-		                 '/tmp/faff.deb'
-					  ).returns(0)
-		
-		pkg.evaluate.each { |state| state.transaction = self; state.forward }
-	end
-	
-	def test_purge
-		pkg = @type.create :name => 'faff', :provider => :dpkg, :ensure => :purged
-
-		pkg.provider.expects(
-		                 :dpkgquery
-					  ).with(
-					        '-W',
-					        '--showformat',
-					        '${Status} ${Package} ${Version}\n',
-					        'faff'
-					  ).returns(
-					        "install ok installed faff 1.2.3-1\n"
-					  ).times(1)
-		pkg.provider.expects(
-		                 :dpkg
-					  ).with(
-					        '--purge',
-					        'faff'
-					  ).returns(0)
-		
-		pkg.evaluate.each { |state| state.transaction = self; state.forward }
-	end
-end