Please review pull request #452: Bug/2.7.x/8435 openbsd package provider broken with remote packages opened by (daniel-pittman)

Description:

This takes #20, a fix for the OpenBSD provider, and adds a bunch of tests so that we can have some basic confidence about the provider.

  • Opened: Fri Feb 03 01:49:48 UTC 2012
  • Based on: puppetlabs:2.7.x (96e79544ad7617b2302729d6b673075ca45cd36c)
  • Requested merge: daniel-pittman:bug/2.7.x/8435-openbsd-package-provider-broken-with-remote-packages (dd2035d422f9f5f9324c7074996ac981499d03ba)

Diff follows:

diff --git a/lib/puppet/provider/package/openbsd.rb b/lib/puppet/provider/package/openbsd.rb
index 3fb1fd7..691c1d7 100755
--- a/lib/puppet/provider/package/openbsd.rb
+++ b/lib/puppet/provider/package/openbsd.rb
@@ -49,7 +49,7 @@ def self.instances
   end
 
   def self.listcmd
-    [command(:pkginfo), " -a"]
+    [command(:pkginfo), "-a"]
   end
 
   def install
@@ -57,26 +57,27 @@ def install
 
     unless @resource[:source]
       raise Puppet::Error,
-        "You must specify a package source for BSD packages"
+      "You must specify a package source for BSD packages"
     end
 
-    if @resource[:source][-1,1] == ::File::PATH_SEPARATOR
-      e_vars = { :PKG_PATH => @resource[:source] }
-      full_name = [ @resource[:name], get_version || @resource[:ensure], @resource[:flavor] ].join('-').chomp('-')
+    if @resource[:source][-1,1] == ::File::SEPARATOR
+      e_vars = { 'PKG_PATH' => @resource[:source] }
+      full_name = [ @resource[:name], get_version || @resource[:ensure], @resource[:flavor] ].join('-').chomp('-').chomp('-')
     else
       e_vars = {}
       full_name = @resource[:source]
     end
 
-     Puppet::Util::Execution::withenv(e_vars) { pkgadd full_name }
+    Puppet::Util::Execution::withenv(e_vars) { pkgadd full_name }
   end
 
   def get_version
-    execpipe([command(:pkginfo), " -I ", @resource[:name]]) do |process|
+    execpipe([command(:pkginfo), "-I", @resource[:name]]) do |process|
       # our regex for matching pkg_info output
       regex = /^(.*)-(\d[^-]*)[-]?(\D*)(.*)$/
       fields = [ :name, :version, :flavor ]
       master_version = 0
+      version = -1
 
       process.each_line do |line|
         if match = regex.match(line.split[0])
@@ -89,6 +90,7 @@ def get_version
       end
 
       return master_version unless master_version == 0
+      return '' if version == -1
       raise Puppet::Error, "#{version} is not available for this package"
     end
   rescue Puppet::ExecutionFailure
@@ -96,17 +98,12 @@ def get_version
   end
 
   def query
-    hash = {}
-    info = pkginfo @resource[:name]
-
     # Search for the version info
-    if info =~ /Information for (inst:)?#{@resource[:name]}-(\S+)/
-      hash[:ensure] = $2
+    if pkginfo(@resource[:name]) =~ /Information for (inst:)?#{@resource[:name]}-(\S+)/
+      return { :ensure => $2 }
     else
       return nil
     end
-
-    hash
   end
 
   def uninstall
diff --git a/spec/fixtures/unit/provider/package/openbsd/pkginfo.detail b/spec/fixtures/unit/provider/package/openbsd/pkginfo.detail
new file mode 100644
index 0000000..5879a2a
--- /dev/null
+++ b/spec/fixtures/unit/provider/package/openbsd/pkginfo.detail
@@ -0,0 +1,19 @@
+Information for bash-3.1.17
+
+Comment:
+GNU Bourne Again Shell
+
+Description:
+Bash is the GNU Project's Bourne Again SHell, an sh-compatible
+command language interpreter that executes commands read from the
+standard input or from a file.  Bash also incorporates useful
+features from the Korn and C shells (ksh and csh).
+
+Bash is intended to be a conformant implementation of the IEEE POSIX
+Shell and Tools specification (IEEE Working Group 1003.2).
+
+Maintainer: Christian Weisgerber <[email protected]>
+
+WWW: http://cnswww.cns.cwru.edu/~chet/bash/bashtop.html
+
+
diff --git a/spec/fixtures/unit/provider/package/openbsd/pkginfo.list b/spec/fixtures/unit/provider/package/openbsd/pkginfo.list
new file mode 100644
index 0000000..5f4866a
--- /dev/null
+++ b/spec/fixtures/unit/provider/package/openbsd/pkginfo.list
@@ -0,0 +1,10 @@
+bash-3.1.17         GNU Bourne Again Shell
+bzip2-1.0.3         block-sorting file compressor, unencumbered
+expat-2.0.0         XML 1.0 parser written in C
+gettext-0.14.5p1    GNU gettext
+libiconv-1.9.2p3    character set conversion library
+lzo-1.08p1          portable speedy lossless data compression library
+openvpn-2.0.6       easy-to-use, robust, and highly configurable VPN
+python-2.4.3p0      interpreted object-oriented programming language
+vim-7.0.42-no_x11   vi clone, many additional features
+wget-1.10.2p0       retrieve files from the web via HTTP, HTTPS and FTP
diff --git a/spec/fixtures/unit/provider/package/openbsd/pkginfo.query b/spec/fixtures/unit/provider/package/openbsd/pkginfo.query
new file mode 100644
index 0000000..d79aecf
--- /dev/null
+++ b/spec/fixtures/unit/provider/package/openbsd/pkginfo.query
@@ -0,0 +1 @@
+bash-3.1.17         GNU Bourne Again Shell
diff --git a/spec/unit/provider/package/openbsd_spec.rb b/spec/unit/provider/package/openbsd_spec.rb
new file mode 100755
index 0000000..88be790
--- /dev/null
+++ b/spec/unit/provider/package/openbsd_spec.rb
@@ -0,0 +1,114 @@
+#!/usr/bin/env rspec
+require 'spec_helper'
+
+provider_class = Puppet::Type.type(:package).provider(:openbsd)
+
+describe provider_class do
+  subject { provider_class }
+
+  def package(args = {})
+    defaults = { :name => 'bash', :provider => 'openbsd' }
+    Puppet::Type.type(:package).new(defaults.merge(args))
+  end
+
+  before :each do
+    # Stub some provider methods to avoid needing the actual software
+    # installed, so we can test on whatever platform we want.
+    provider_class.stubs(:command).with(:pkginfo).returns('/bin/pkg_info')
+    provider_class.stubs(:command).with(:pkgadd).returns('/bin/pkg_add')
+    provider_class.stubs(:command).with(:pkgdelete).returns('/bin/pkg_delete')
+  end
+
+  context "::instances" do
+    it "should return nil if execution failed" do
+      subject.expects(:execpipe).raises(Puppet::ExecutionFailure, 'wawawa')
+      subject.instances.should be_nil
+    end
+
+    it "should return the empty set if no packages are listed" do
+      subject.expects(:execpipe).with(%w{/bin/pkg_info -a}).yields('')
+      subject.instances.should be_empty
+    end
+
+    it "should return all packages when invoked" do
+      fixture = File.read(my_fixture('pkginfo.list'))
+      subject.expects(:execpipe).with(%w{/bin/pkg_info -a}).yields(fixture)
+      subject.instances.map(&:name).sort.should ==
+        %w{bash bzip2 expat gettext libiconv lzo openvpn python vim wget}.sort
+    end
+  end
+
+  context "#install" do
+    it "should fail if the resource doesn't have a source" do
+      provider = subject.new(package())
+      expect { provider.install }.
+        to raise_error Puppet::Error, /must specify a package source/
+    end
+
+    it "should install correctly when given a directory-unlike source" do
+      ENV.should_not be_key 'PKG_PATH'
+
+      source = '/whatever.pkg'
+      provider = subject.new(package(:source => source))
+      provider.expects(:pkgadd).with do |name|
+        ENV.should_not be_key 'PKG_PATH'
+        name == source
+      end
+
+      provider.install
+      ENV.should_not be_key 'PKG_PATH'
+    end
+
+    it "should install correctly when given a directory-like source" do
+      ENV.should_not be_key 'PKG_PATH'
+
+      source = '/whatever/'
+      provider = subject.new(package(:source => source))
+      provider.expects(:pkgadd).with do |name|
+        ENV.should be_key 'PKG_PATH'
+        ENV['PKG_PATH'].should == source
+
+        name == provider.resource[:name]
+      end
+
+      provider.install
+      ENV.should_not be_key 'PKG_PATH'
+    end
+  end
+
+  context "#get_version" do
+    it "should return nil if execution fails" do
+      provider = subject.new(package)
+      provider.expects(:execpipe).raises(Puppet::ExecutionFailure, 'wawawa')
+      provider.get_version.should be_nil
+    end
+
+    it "should return the package version if in the output" do
+      fixture = File.read(my_fixture('pkginfo.list'))
+      provider = subject.new(package(:name => 'bash'))
+      provider.expects(:execpipe).with(%w{/bin/pkg_info -I bash}).yields(fixture)
+      provider.get_version.should == '3.1.17'
+    end
+
+    it "should return the empty string if the package is not present" do
+      provider = subject.new(package(:name => 'zsh'))
+      provider.expects(:execpipe).with(%w{/bin/pkg_info -I zsh}).yields('')
+      provider.get_version.should == ''
+    end
+  end
+
+  context "#query" do
+    it "should return the installed version if present" do
+      fixture = File.read(my_fixture('pkginfo.detail'))
+      provider = subject.new(package(:name => 'bash'))
+      provider.expects(:pkginfo).with('bash').returns(fixture)
+      provider.query.should == { :ensure => '3.1.17' }
+    end
+
+    it "should return nothing if not present" do
+      provider = subject.new(package(:name => 'zsh'))
+      provider.expects(:pkginfo).with('zsh').returns('')
+      provider.query.should be_nil
+    end
+  end
+end

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to