Merge branch 'ticket/2.6.next/5710' into 2.6.next

* ticket/2.6.next/5710:
  (#5710) Removed unnecessary calls to insync?
  Prep for fixing #5710: Refactor stub provider in resource harness spec

2 files changed, 64 insertions, 35 deletions

diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb
index c259d3e05..c1b980632 100644
--- a/lib/puppet/transaction/resource_harness.rb
+++ b/lib/puppet/transaction/resource_harness.rb
@@ -37,7 +37,10 @@ class Puppet::Transaction::ResourceHarness
 
     current_values = current.to_hash
     historical_values = Puppet::Util::Storage.cache(resource).dup
-    desired_values = resource.to_resource.to_hash
+    desired_values = {}
+    resource.properties.each do |property|
+      desired_values[property.name] = property.should
+    end
     audited_params = (resource[:audit] || []).map { |p| p.to_sym }
     synced_params = []
 
@@ -55,7 +58,7 @@ class Puppet::Transaction::ResourceHarness
     elsif current_values[:ensure] != :absent
       work_order = resource.properties # Note: only the resource knows what order to apply changes in
       work_order.each do |param|
-        if !param.insync?(current_values[param.name])
+        if desired_values[param.name] && !param.insync?(current_values[param.name])
           events << apply_parameter(param, current_values[param.name], audited_params.include?(param.name), historical_values[param.name])
           synced_params << param.name
         end
diff --git a/spec/unit/transaction/resource_harness_spec.rb b/spec/unit/transaction/resource_harness_spec.rb
index ca35740f5..104c19e85 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -64,49 +64,52 @@ describe Puppet::Transaction::ResourceHarness do
     end
   end
 
-  describe "when an error occurs" do
-    before :each do
-      # Create a temporary anonymous class to act as a provider
-      stubProvider = Class.new(Puppet::Type)
-      stubProvider.instance_eval do
-        initvars
-
-        newparam(:name) do
-          desc "The name var"
-          isnamevar
-        end
+  def make_stub_provider
+    stubProvider = Class.new(Puppet::Type)
+    stubProvider.instance_eval do
+      initvars
+
+      newparam(:name) do
+        desc "The name var"
+        isnamevar
+      end
 
-        newproperty(:foo) do
-          desc "A property that can be changed successfully"
-          def sync
-          end
+      newproperty(:foo) do
+        desc "A property that can be changed successfully"
+        def sync
+        end
 
-          def retrieve
-            :absent
-          end
+        def retrieve
+          :absent
+        end
 
-          def insync?(reference_value)
-            false
-          end
+        def insync?(reference_value)
+          false
         end
+      end
 
-        newproperty(:bar) do
-          desc "A property that raises an exception when you try to change it"
-          def sync
-            raise ZeroDivisionError.new('bar')
-          end
+      newproperty(:bar) do
+        desc "A property that raises an exception when you try to change it"
+        def sync
+          raise ZeroDivisionError.new('bar')
+        end
 
-          def retrieve
-            :absent
-          end
+        def retrieve
+          :absent
+        end
 
-          def insync?(reference_value)
-            false
-          end
+        def insync?(reference_value)
+          false
         end
       end
+    end
+    stubProvider
+  end
 
-      resource = stubProvider.new :name => 'name', :foo => 1, :bar => 2
+  describe "when an error occurs" do
+    before :each do
+      stub_provider = make_stub_provider
+      resource = stub_provider.new :name => 'name', :foo => 1, :bar => 2
       resource.expects(:err).never
       @status = @harness.evaluate(resource)
     end
@@ -122,6 +125,29 @@ describe Puppet::Transaction::ResourceHarness do
     end
   end
 
+  describe "when auditing" do
+    it "should not call insync? on parameters that are merely audited" do
+      stub_provider = make_stub_provider
+      resource = stub_provider.new :name => 'name', :audit => ['foo']
+      resource.property(:foo).expects(:insync?).never
+      status = @harness.evaluate(resource)
+      status.events.each do |event|
+        event.status.should != 'failure'
+      end
+    end
+
+    it "should be able to audit a file's group" do # see bug #5710
+      test_file = tmpfile('foo')
+      File.open(test_file, 'w').close
+      resource = Puppet::Type.type(:file).new :path => test_file, :audit => ['group'], :backup => false
+      resource.expects(:err).never # make sure no exceptions get swallowed
+      status = @harness.evaluate(resource)
+      status.events.each do |event|
+        event.status.should != 'failure'
+      end
+    end
+  end
+
   describe "when applying changes" do
     [false, true].each do |noop_mode|; describe (noop_mode ? "in noop mode" : "in normal mode") do
       [nil, '750'].each do |machine_state|; describe (machine_state ? "with a file initially present" : "with no file initially present") do