Wire up provider/resource and use constants so we can test

Also allow the pin priority to be an integer since there's no reason that needs to be a string

Signed-off-by: Tim Smith <tsmith@chef.io>

5 files changed, 43 insertions, 34 deletions

diff --git a/lib/chef/provider/apt_preference.rb b/lib/chef/provider/apt_preference.rb
index aaf2c566da..ff8a244f6e 100644
--- a/lib/chef/provider/apt_preference.rb
+++ b/lib/chef/provider/apt_preference.rb
@@ -16,23 +16,23 @@
 # limitations under the License.
 #
 
-require "chef/resource"
+require "chef/provider"
 require "chef/dsl/declare_resource"
-require "chef/mixin/which"
 require "chef/provider/noop"
+require "chef/mixin/which"
 require "chef/log"
 
 class Chef
   class Provider
     class AptPreference < Chef::Provider
-      use_inline_resources
-
       extend Chef::Mixin::Which
 
       provides :apt_preference do
         which("apt-get")
       end
 
+      APT_PREFERENCE_DIR = "/etc/apt/preferences.d".freeze
+
       def load_current_resource
       end
 
@@ -43,9 +43,7 @@ class Chef
           new_resource.pin_priority
         )
 
-        declare_resource(:directory, "/etc/apt/preferences.d") do
-          owner "root"
-          group "root"
+        declare_resource(:directory, APT_PREFERENCE_DIR) do
           mode "0755"
           recursive true
           action :create
@@ -53,24 +51,22 @@ class Chef
 
         name = safe_name(new_resource.name)
 
-        declare_resource(:file, "/etc/apt/preferences.d/#{new_resource.name}.pref") do
+        declare_resource(:file, ::File.join(APT_PREFERENCE_DIR, "#{new_resource.name}.pref")) do
           action :delete
-          if ::File.exist?("/etc/apt/preferences.d/#{new_resource.name}.pref")
-            Chef::Log.warn "Replacing #{new_resource.name}.pref with #{name}.pref in /etc/apt/preferences.d/"
+          if ::File.exist?(::File.join(APT_PREFERENCE_DIR, "#{new_resource.name}.pref"))
+            Chef::Log.warn "Replacing #{new_resource.name}.pref with #{name}.pref in #{APT_PREFERENCE_DIR}"
           end
           only_if { name != new_resource.name }
         end
 
-        declare_resource(:file, "/etc/apt/preferences.d/#{new_resource.name}") do
+        declare_resource(:file, ::File.join(APT_PREFERENCE_DIR, "#{new_resource.name}")) do
           action :delete
-          if ::File.exist?("/etc/apt/preferences.d/#{new_resource.name}")
-            Chef::Log.warn "Replacing #{new_resource.name} with #{new_resource.name}.pref in /etc/apt/preferences.d/"
+          if ::File.exist?(::File.join(APT_PREFERENCE_DIR, "#{new_resource.name}"))
+            Chef::Log.warn "Replacing #{new_resource.name} with #{new_resource.name}.pref in #{APT_PREFERENCE_DIR}"
           end
         end
 
-        declare_resource(:file, "/etc/apt/preferences.d/#{name}.pref") do
-          owner "root"
-          group "root"
+        declare_resource(:file, ::File.join(APT_PREFERENCE_DIR, "#{name}.pref")) do
           mode "0644"
           content preference
           action :create
@@ -79,9 +75,9 @@ class Chef
 
       action :delete do
         name = safe_name(new_resource.name)
-        if ::File.exist?("/etc/apt/preferences.d/#{name}.pref")
-          Chef::Log.info "Un-pinning #{name} from /etc/apt/preferences.d/"
-          declare_resource(:file, "/etc/apt/preferences.d/#{name}.pref") do
+        if ::File.exist?(::File.join(APT_PREFERENCE_DIR, "#{name}.pref"))
+          Chef::Log.info "Un-pinning #{name} from #{APT_PREFERENCE_DIR}"
+          declare_resource(:file, ::File.join(APT_PREFERENCE_DIR, "#{name}.pref")) do
             action :delete
           end
         end
diff --git a/lib/chef/providers.rb b/lib/chef/providers.rb
index 41de44a1d6..67165dc999 100644
--- a/lib/chef/providers.rb
+++ b/lib/chef/providers.rb
@@ -17,6 +17,7 @@
 #
 
 require "chef/provider/apt_update"
+require "chef/provider/apt_preference"
 require "chef/provider/apt_repository"
 require "chef/provider/batch"
 require "chef/provider/breakpoint"
diff --git a/lib/chef/resource/apt_preference.rb b/lib/chef/resource/apt_preference.rb
index cccddafdc5..78b55d0dba 100644
--- a/lib/chef/resource/apt_preference.rb
+++ b/lib/chef/resource/apt_preference.rb
@@ -27,7 +27,7 @@ class Chef
       property :package_name, String, name_property: true, regex: [/^([a-z]|[A-Z]|[0-9]|_|-|\.|\*|\+)+$/]
       property :glob, String
       property :pin, String, required: true
-      property :pin_priority, String, required: true
+      property :pin_priority, [String,Integer], required: true
 
       default_action :add
       allowed_actions :add, :remove
diff --git a/lib/chef/resources.rb b/lib/chef/resources.rb
index 9f87cb2454..7761d9ea66 100644
--- a/lib/chef/resources.rb
+++ b/lib/chef/resources.rb
@@ -17,6 +17,7 @@
 #
 
 require "chef/resource/apt_package"
+require "chef/resource/apt_preference"
 require "chef/resource/apt_repository"
 require "chef/resource/apt_update"
 require "chef/resource/bash"
diff --git a/spec/unit/provider/apt_preference_spec.rb b/spec/unit/provider/apt_preference_spec.rb
index e841520014..3b529c81b9 100644
--- a/spec/unit/provider/apt_preference_spec.rb
+++ b/spec/unit/provider/apt_preference_spec.rb
@@ -21,6 +21,14 @@ require "spec_helper"
 
 describe Chef::Provider::AptPreference do
   let(:new_resource) { Chef::Resource::AptPreference.new("libmysqlclient16") }
+  let(:pref_dir) { Dir.mktmpdir("apt_pref_d") }
+
+  before do
+    stub_const("Chef::Provider::AptPreference::APT_PREFERENCE_DIR", pref_dir)
+    new_resource.pin = '1.0.1'
+    new_resource.pin_priority 1001
+  end
+
   let(:provider) do
     node = Chef::Node.new
     events = Chef::EventDispatch::Dispatcher.new
@@ -32,22 +40,25 @@ describe Chef::Provider::AptPreference do
     expect(provider).to respond_to(:load_current_resource)
   end
 
-  it "creates apt preferences directory if it does not exist" do
-    provider.run_action(:update)
-    expect(new_resource).to be_updated_by_last_action
-    expect(File.exist?('/etc/apt/preferences.d')).to be true
-    expect(File.directory?('/etc/apt/preferences.d')).to be true
-  end
+  context "when the preferences.d directory does not exist" do
+    before do
+      FileUtils.rmdir pref_dir
+      expect(File.exist?(pref_dir)).to be false
+    end
 
-  it "cleans up legacy non-sanitized name files" do
-    # something
-  end
+    it "should create the preferences.d directory" do
+      provider.run_action(:add)
+      expect(new_resource).to be_updated_by_last_action
+      expect(File.exist?(pref_dir)).to be true
+      expect(File.directory?(pref_dir)).to be true
+    end
 
-  it "creates an apt .pref file" do
-    # something
-  end
+    it "creates an apt .pref file" do
+      # something
+    end
 
-  it "creates an apt .pref file with a sanitized filename" do
-    # something
+    it "creates an apt .pref file with a sanitized filename" do
+      # something
+    end
   end
 end