dpkg provider cleanup

- :update and :install are now treated the same way and throw the
  same exceptions
- :remove and :purge don't require the source at all, so don't do
  any checking on that
- fix some convoluted side-effecty logic in load_current_resource
- load_current_resource now correctly gets the dpkg state on
  :remove and :purge when the file does not exist (pretty sure
  the old logic did not)
- fixed the FIXME about using en_US.UTF-8 (the default for shell_out!)
- just use shell_out! to throw exceptions
- clean up all the specs and remove all the instance vars from the code

1 files changed, 138 insertions, 85 deletions

diff --git a/spec/unit/provider/package/dpkg_spec.rb b/spec/unit/provider/package/dpkg_spec.rb
index b868128147..2ded8313c0 100644
--- a/spec/unit/provider/package/dpkg_spec.rb
+++ b/spec/unit/provider/package/dpkg_spec.rb
@@ -19,42 +19,109 @@
 require 'spec_helper'
 
 describe Chef::Provider::Package::Dpkg do
+  let(:node) { Chef::Node.new }
+  let(:events) { Chef::EventDispatch::Dispatcher.new }
+  let(:run_context) { Chef::RunContext.new(node, {}, events) }
+  let(:package) { "wget" }
+  let(:source) { "/tmp/wget_1.11.4-1ubuntu1_amd64.deb" }
+  let(:new_resource) do
+    new_resource = Chef::Resource::Package.new(package)
+    new_resource.source source
+    new_resource
+  end
+  let(:provider) { Chef::Provider::Package::Dpkg.new(new_resource, run_context) }
+
+  let(:dpkg_deb_version) { "1.11.4" }
+  let(:dpkg_deb_status) { status = double(:stdout => "#{package}\t#{dpkg_deb_version}", :exitstatus => 0) }
+  let(:dpkg_s_version) { "1.11.4-1ubuntu1" }
+  let(:dpkg_s_status) do
+    stdout = <<-DPKG_S
+Package: #{package}
+Status: install ok installed
+Priority: important
+Section: web
+Installed-Size: 1944
+Maintainer: Ubuntu Core developers <ubuntu-devel-discuss@lists.ubuntu.com>
+Architecture: amd64
+Version: #{dpkg_s_version}
+Config-Version: #{dpkg_s_version}
+Depends: libc6 (>= 2.8~20080505), libssl0.9.8 (>= 0.9.8f-5)
+Conflicts: wget-ssl
+    DPKG_S
+    status = double(:stdout => stdout, :exitstatus => 1)
+  end
+
   before(:each) do
-    @node = Chef::Node.new
-    @events = Chef::EventDispatch::Dispatcher.new
-    @run_context = Chef::RunContext.new(@node, {}, @events)
-    @new_resource = Chef::Resource::Package.new("wget")
-    @new_resource.source "/tmp/wget_1.11.4-1ubuntu1_amd64.deb"
+    allow(provider).to receive(:shell_out!).with("dpkg-deb -W #{new_resource.source}", timeout: 900).and_return(dpkg_deb_status)
+    allow(provider).to receive(:shell_out).with("dpkg -s #{package}", timeout: 900).and_return(dpkg_s_status)
+    allow(::File).to receive(:exist?).with(source).and_return(true)
+  end
+
+  describe "#define_resource_requirements" do
+    it "should raise an exception if a source is supplied but not found when :install" do
+      allow(::File).to receive(:exist?).with(source).and_return(false)
+      expect { provider.run_action(:install) }.to raise_error(Chef::Exceptions::Package)
+    end
+
+    it "should raise an exception if a source is supplied but not found when :upgrade" do
+      allow(::File).to receive(:exist?).with(source).and_return(false)
+      expect { provider.run_action(:upgrade) }.to raise_error(Chef::Exceptions::Package)
+    end
+
+    # FIXME?  we're saying we ignore source, but should supplying source on :remove or :purge be an actual error?
+    it "should not raise an exception if a source is supplied but not found when :remove" do
+      allow(::File).to receive(:exist?).with(source).and_return(false)
+      expect(provider).to receive(:action_remove)
+      expect { provider.run_action(:remove) }.not_to raise_error
+    end
+
+    it "should not raise an exception if a source is supplied but not found when :purge" do
+      allow(::File).to receive(:exist?).with(source).and_return(false)
+      expect(provider).to receive(:action_purge)
+      expect { provider.run_action(:purge) }.not_to raise_error
+    end
 
-    @provider = Chef::Provider::Package::Dpkg.new(@new_resource, @run_context)
+    it "should raise an exception if a source is nil when :install" do
+      new_resource.source nil
+      allow(::File).to receive(:exist?).with(source).and_return(false)
+      expect { provider.run_action(:install) }.to raise_error(Chef::Exceptions::Package)
+    end
 
-    @status = double(:stdout => "", :exitstatus => 0)
-    allow(@provider).to receive(:shell_out).and_return(@status)
+    it "should raise an exception if a source is nil when :upgrade" do
+      new_resource.source nil
+      allow(::File).to receive(:exist?).with(source).and_return(false)
+      expect { provider.run_action(:upgrade) }.to raise_error(Chef::Exceptions::Package)
+    end
 
-    allow(::File).to receive(:exists?).and_return(true)
+    it "should not raise an exception if a source is nil when :remove" do
+      new_resource.source nil
+      allow(::File).to receive(:exist?).with(source).and_return(false)
+      expect(provider).to receive(:action_remove)
+      expect { provider.run_action(:remove) }.not_to raise_error
+    end
+
+    it "should not raise an exception if a source is nil when :purge" do
+      new_resource.source nil
+      allow(::File).to receive(:exist?).with(source).and_return(false)
+      expect(provider).to receive(:action_purge)
+      expect { provider.run_action(:purge) }.not_to raise_error
+    end
   end
 
   describe "when loading the current resource state" do
 
     it "should create a current resource with the name of the new_resource" do
-      @provider.load_current_resource
-      expect(@provider.current_resource.package_name).to eq("wget")
-    end
-
-    it "should raise an exception if a source is supplied but not found" do
-      @provider.load_current_resource
-      @provider.define_resource_requirements
-      allow(::File).to receive(:exists?).and_return(false)
-      expect { @provider.run_action(:install) }.to raise_error(Chef::Exceptions::Package)
+      provider.load_current_resource
+      expect(provider.current_resource.package_name).to eq("wget")
     end
 
     describe 'gets the source package version from dpkg-deb' do
       def check_version(version)
-        @status = double(:stdout => "wget\t#{version}", :exitstatus => 0)
-        allow(@provider).to receive(:shell_out).with("dpkg-deb -W #{@new_resource.source}", timeout: 900).and_return(@status)
-        @provider.load_current_resource
-        expect(@provider.current_resource.package_name).to eq("wget")
-        expect(@provider.candidate_version).to eq(version)
+        status = double(:stdout => "wget\t#{version}", :exitstatus => 0)
+        expect(provider).to receive(:shell_out!).with("dpkg-deb -W #{new_resource.source}", timeout: 900).and_return(status)
+        provider.load_current_resource
+        expect(provider.current_resource.package_name).to eq("wget")
+        expect(provider.candidate_version).to eq(version)
       end
 
       it 'if short version provided' do
@@ -74,129 +141,115 @@ describe Chef::Provider::Package::Dpkg do
       end
     end
 
-    it "gets the source package name from dpkg-deb correctly when the package name has `-', `+' or `.' characters" do
-      stdout = "f.o.o-pkg++2\t1.11.4-1ubuntu1"
-      status = double(:stdout => stdout, :exitstatus => 1)
-      allow(@provider).to receive(:shell_out).and_return(status)
-      @provider.load_current_resource
-      expect(@provider.current_resource.package_name).to eq("f.o.o-pkg++2")
+    describe "when the package name has `-', `+' or `.' characters" do
+      let(:package) { "f.o.o-pkg++2" }
+
+      it "gets the source package name from dpkg-deb correctly" do
+        provider.load_current_resource
+        expect(provider.current_resource.package_name).to eq("f.o.o-pkg++2")
+      end
     end
 
-    it "gets the source package version from dpkg-deb correctly when the package version has `~', `-', `+' or `.' characters" do
-      stdout = "b.a.r-pkg++1\t1.2.3+3141592-1ubuntu1~lucid"
-      status = double(:stdout => stdout, :exitstatus => 1)
-      allow(@provider).to receive(:shell_out).and_return(status)
-      @provider.load_current_resource
-      expect(@provider.candidate_version).to eq('1.2.3+3141592-1ubuntu1~lucid')
+    describe "when the package version has `~', `-', `+' or `.' characters" do
+      let(:package) { "b.a.r-pkg++1" }
+      let(:dpkg_deb_version) { "1.2.3+3141592-1ubuntu1~lucid" }
+      let(:dpkg_s_version) { "1.2.3+3141592-1ubuntu1~lucid" }
+
+      it "gets the source package version from dpkg-deb correctly when the package version has `~', `-', `+' or `.' characters" do
+        provider.load_current_resource
+        expect(provider.candidate_version).to eq('1.2.3+3141592-1ubuntu1~lucid')
+      end
     end
 
     it "should raise an exception if the source is not set but we are installing" do
-      @new_resource = Chef::Resource::Package.new("wget")
-      @provider.new_resource = @new_resource
-      @provider.load_current_resource
-      @provider.define_resource_requirements
-      expect { @provider.run_action(:install)}.to raise_error(Chef::Exceptions::Package)
+      new_resource = Chef::Resource::Package.new("wget")
+      provider.new_resource = new_resource
+      provider.load_current_resource
+      provider.define_resource_requirements
+      expect { provider.run_action(:install)}.to raise_error(Chef::Exceptions::Package)
     end
 
     it "should return the current version installed if found by dpkg" do
-      stdout = <<-DPKG_S
-Package: wget
-Status: install ok installed
-Priority: important
-Section: web
-Installed-Size: 1944
-Maintainer: Ubuntu Core developers <ubuntu-devel-discuss@lists.ubuntu.com>
-Architecture: amd64
-Version: 1.11.4-1ubuntu1
-Config-Version: 1.11.4-1ubuntu1
-Depends: libc6 (>= 2.8~20080505), libssl0.9.8 (>= 0.9.8f-5)
-Conflicts: wget-ssl
-DPKG_S
-      status = double(:stdout => stdout, :exitstatus => 1)
-      allow(@provider).to receive(:shell_out).with("dpkg -s wget", timeout: 900).and_return(status)
-
-      @provider.load_current_resource
-      expect(@provider.current_resource.version).to eq("1.11.4-1ubuntu1")
+      provider.load_current_resource
+      expect(provider.current_resource.version).to eq("1.11.4-1ubuntu1")
     end
 
     it "should raise an exception if dpkg fails to run" do
       status = double(:stdout => "", :exitstatus => -1)
-      allow(@provider).to receive(:shell_out).and_return(status)
-      expect { @provider.load_current_resource }.to raise_error(Chef::Exceptions::Package)
+      expect(provider).to receive(:shell_out_with_timeout!).with("dpkg-deb -W /tmp/wget_1.11.4-1ubuntu1_amd64.deb").and_raise(Mixlib::ShellOut::ShellCommandFailed)
+      expect { provider.load_current_resource }.to raise_error(Mixlib::ShellOut::ShellCommandFailed)
     end
   end
 
   describe Chef::Provider::Package::Dpkg, "install and upgrade" do
     it "should run dpkg -i with the package source" do
-      expect(@provider).to receive(:run_noninteractive).with(
+      expect(provider).to receive(:run_noninteractive).with(
         "dpkg -i /tmp/wget_1.11.4-1ubuntu1_amd64.deb"
       )
-      @provider.install_package("wget", "1.11.4-1ubuntu1")
+      provider.install_package("wget", "1.11.4-1ubuntu1")
     end
 
     it "should run dpkg -i if the package is a path and the source is nil" do
-      @new_resource = Chef::Resource::Package.new("/tmp/wget_1.11.4-1ubuntu1_amd64.deb")
-      @provider = Chef::Provider::Package::Dpkg.new(@new_resource, @run_context)
-      expect(@provider).to receive(:run_noninteractive).with(
+      new_resource.name = "/tmp/wget_1.11.4-1ubuntu1_amd64.deb"
+      expect(provider).to receive(:run_noninteractive).with(
         "dpkg -i /tmp/wget_1.11.4-1ubuntu1_amd64.deb"
       )
-      @provider.install_package("/tmp/wget_1.11.4-1ubuntu1_amd64.deb", "1.11.4-1ubuntu1")
+      provider.install_package("/tmp/wget_1.11.4-1ubuntu1_amd64.deb", "1.11.4-1ubuntu1")
     end
 
     it "should run dpkg -i if the package is a path and the source is nil for an upgrade" do
-      @new_resource = Chef::Resource::Package.new("/tmp/wget_1.11.4-1ubuntu1_amd64.deb")
-      @provider = Chef::Provider::Package::Dpkg.new(@new_resource, @run_context)
-      expect(@provider).to receive(:run_noninteractive).with(
+      new_resource.name = "/tmp/wget_1.11.4-1ubuntu1_amd64.deb"
+      expect(provider).to receive(:run_noninteractive).with(
         "dpkg -i /tmp/wget_1.11.4-1ubuntu1_amd64.deb"
       )
-      @provider.upgrade_package("/tmp/wget_1.11.4-1ubuntu1_amd64.deb", "1.11.4-1ubuntu1")
+      provider.upgrade_package("/tmp/wget_1.11.4-1ubuntu1_amd64.deb", "1.11.4-1ubuntu1")
     end
 
     it "should run dpkg -i with the package source and options if specified" do
-      expect(@provider).to receive(:run_noninteractive).with(
+      expect(provider).to receive(:run_noninteractive).with(
         "dpkg -i --force-yes /tmp/wget_1.11.4-1ubuntu1_amd64.deb"
       )
-      allow(@new_resource).to receive(:options).and_return("--force-yes")
+      allow(new_resource).to receive(:options).and_return("--force-yes")
 
-      @provider.install_package("wget", "1.11.4-1ubuntu1")
+      provider.install_package("wget", "1.11.4-1ubuntu1")
     end
     it "should upgrade by running install_package" do
-      expect(@provider).to receive(:install_package).with("wget", "1.11.4-1ubuntu1")
-      @provider.upgrade_package("wget", "1.11.4-1ubuntu1")
+      expect(provider).to receive(:install_package).with("wget", "1.11.4-1ubuntu1")
+      provider.upgrade_package("wget", "1.11.4-1ubuntu1")
     end
   end
 
   describe Chef::Provider::Package::Dpkg, "remove and purge" do
     it "should run dpkg -r to remove the package" do
-      expect(@provider).to receive(:run_noninteractive).with(
+      expect(provider).to receive(:run_noninteractive).with(
         "dpkg -r wget"
       )
-      @provider.remove_package("wget", "1.11.4-1ubuntu1")
+      provider.remove_package("wget", "1.11.4-1ubuntu1")
     end
 
     it "should run dpkg -r to remove the package with options if specified" do
-      expect(@provider).to receive(:run_noninteractive).with(
+      expect(provider).to receive(:run_noninteractive).with(
         "dpkg -r --force-yes wget"
       )
-      allow(@new_resource).to receive(:options).and_return("--force-yes")
+      allow(new_resource).to receive(:options).and_return("--force-yes")
 
-      @provider.remove_package("wget", "1.11.4-1ubuntu1")
+      provider.remove_package("wget", "1.11.4-1ubuntu1")
     end
 
     it "should run dpkg -P to purge the package" do
-      expect(@provider).to receive(:run_noninteractive).with(
+      expect(provider).to receive(:run_noninteractive).with(
         "dpkg -P wget"
       )
-      @provider.purge_package("wget", "1.11.4-1ubuntu1")
+      provider.purge_package("wget", "1.11.4-1ubuntu1")
     end
 
     it "should run dpkg -P to purge the package with options if specified" do
-      expect(@provider).to receive(:run_noninteractive).with(
+      expect(provider).to receive(:run_noninteractive).with(
         "dpkg -P --force-yes wget"
       )
-      allow(@new_resource).to receive(:options).and_return("--force-yes")
+      allow(new_resource).to receive(:options).and_return("--force-yes")
 
-      @provider.purge_package("wget", "1.11.4-1ubuntu1")
+      provider.purge_package("wget", "1.11.4-1ubuntu1")
     end
   end
 end