Merge branch '2.6.x' into next

Manually Resolved Conflicts:
	lib/puppet/resource/type_collection.rb
	spec/unit/configurer_spec.rb
	spec/unit/indirector/catalog/active_record_spec.rb
	spec/unit/resource/type_collection_spec.rb
	spec/unit/transaction/resource_harness_spec.rb

12 files changed, 330 insertions, 95 deletions

diff --git a/spec/unit/application/inspect_spec.rb b/spec/unit/application/inspect_spec.rb
new file mode 100644
index 000000000..a3cc74d86
--- /dev/null
+++ b/spec/unit/application/inspect_spec.rb
@@ -0,0 +1,79 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/application/inspect'
+require 'puppet/resource/catalog'
+require 'puppet/indirector/catalog/yaml'
+require 'puppet/indirector/report/rest'
+
+describe Puppet::Application::Inspect do
+  before :each do
+    @inspect = Puppet::Application[:inspect]
+  end
+
+  describe "during setup" do
+    it "should print its configuration if asked" do
+      Puppet[:configprint] = "all"
+
+      Puppet.settings.expects(:print_configs).returns(true)
+      lambda { @inspect.setup }.should raise_error(SystemExit)
+    end
+
+    it "should fail if reporting is turned off" do
+      Puppet[:report] = false
+      lambda { @inspect.setup }.should raise_error(/report=true/)
+    end
+  end
+
+  describe "when executing" do
+    before :each do
+      Puppet[:report] = true
+      Puppet::Util::Log.stubs(:newdestination)
+      Puppet::Transaction::Report::Rest.any_instance.stubs(:save)
+      @inspect.setup
+    end
+
+    it "should retrieve the local catalog" do
+      Puppet::Resource::Catalog::Yaml.any_instance.expects(:find).with {|request| request.key == Puppet[:certname] }.returns(Puppet::Resource::Catalog.new)
+
+      @inspect.run_command
+    end
+
+    it "should save the report to REST" do
+      Puppet::Resource::Catalog::Yaml.any_instance.stubs(:find).returns(Puppet::Resource::Catalog.new)
+      Puppet::Transaction::Report::Rest.any_instance.expects(:save).with {|request| request.instance.host == Puppet[:certname] }
+
+      @inspect.run_command
+    end
+
+    it "should audit the specified properties" do
+      catalog = Puppet::Resource::Catalog.new
+      file = Tempfile.new("foo")
+      file.puts("file contents")
+      file.flush
+      resource = Puppet::Resource.new(:file, file.path, :parameters => {:audit => "all"})
+      catalog.add_resource(resource)
+      Puppet::Resource::Catalog::Yaml.any_instance.stubs(:find).returns(catalog)
+
+      events = nil
+
+      Puppet::Transaction::Report::Rest.any_instance.expects(:save).with do |request|
+        events = request.instance.resource_statuses.values.first.events
+      end
+
+      @inspect.run_command
+
+      properties = events.inject({}) do |property_values, event|
+        property_values.merge(event.property => event.previous_value)
+      end
+      properties["ensure"].should == :file
+      properties["content"].should == "{md5}#{Digest::MD5.hexdigest("file contents\n")}"
+    end
+  end
+
+  after :all do
+    Puppet::Resource::Catalog.indirection.reset_terminus_class
+    Puppet::Transaction::Report.indirection.terminus_class = :processor
+  end
+end
diff --git a/spec/unit/configurer/plugin_handler_spec.rb b/spec/unit/configurer/plugin_handler_spec.rb
index 27d5a9f06..04a479665 100755
--- a/spec/unit/configurer/plugin_handler_spec.rb
+++ b/spec/unit/configurer/plugin_handler_spec.rb
@@ -11,6 +11,10 @@ end
 describe Puppet::Configurer::PluginHandler do
   before do
     @pluginhandler = PluginHandlerTester.new
+
+    # PluginHandler#load_plugin has an extra-strong rescue clause
+    # this mock is to make sure that we don't silently ignore errors
+    Puppet.expects(:err).never
   end
 
   it "should have a method for downloading plugins" do
@@ -80,7 +84,7 @@ describe Puppet::Configurer::PluginHandler do
   end
 
   it "should not try to load files that don't exist" do
-    FileTest.expects(:exist?).with("foo").returns true
+    FileTest.expects(:exist?).with("foo").returns false
     @pluginhandler.expects(:load).never
 
     @pluginhandler.load_plugin("foo")
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index 9f88ee526..705417dcb 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -122,9 +122,10 @@ describe Puppet::Configurer, "when executing a catalog run" do
   end
 
   it "should set the report as a log destination" do
-    report = stub 'report'
+    report = Puppet::Transaction::Report.new
     @agent.expects(:initialize_report).returns report
 
+    @agent.stubs(:send_report)
     Puppet::Util::Log.expects(:newdestination).with(report)
 
     @agent.run
@@ -461,7 +462,7 @@ describe Puppet::Configurer, "when retrieving a catalog" do
   end
 
   it "should return nil if there is an error while retrieving the catalog" do
-    Puppet::Resource::Catalog.indirection.expects(:find).raises "eh"
+    Puppet::Resource::Catalog.indirection.expects(:find).at_least_once.raises "eh"
 
     @agent.retrieve_catalog.should be_nil
   end
diff --git a/spec/unit/file_serving/fileset_spec.rb b/spec/unit/file_serving/fileset_spec.rb
index ba0a98eca..1ef9cdc50 100755
--- a/spec/unit/file_serving/fileset_spec.rb
+++ b/spec/unit/file_serving/fileset_spec.rb
@@ -306,6 +306,7 @@ end
 describe Puppet::FileServing::Fileset, "when merging other filesets" do
   before do
     @paths = %w{/first/path /second/path /third/path}
+    File.stubs(:lstat).returns stub("stat", :directory? => false)
 
     @filesets = @paths.collect do |path|
       File.stubs(:lstat).with(path).returns stub("stat", :directory? => true)
diff --git a/spec/unit/indirector/catalog/active_record_spec.rb b/spec/unit/indirector/catalog/active_record_spec.rb
index e46f5462a..71975f014 100755
--- a/spec/unit/indirector/catalog/active_record_spec.rb
+++ b/spec/unit/indirector/catalog/active_record_spec.rb
@@ -6,6 +6,23 @@ require File.expand_path(File.dirname(__FILE__) + '/../../../spec_helper')
 describe "Puppet::Resource::Catalog::ActiveRecord" do
   confine "Missing Rails" => Puppet.features.rails?
 
+  require 'puppet/rails'
+  class Tableless < ActiveRecord::Base
+    def self.columns
+      @columns ||= []
+    end
+    def self.column(name, sql_type=nil, default=nil, null=true)
+      columns << ActiveRecord::ConnectionAdapters::Column.new(name.to_s, default, sql_type.to_s, null)
+    end
+  end
+
+  class Host < Tableless
+    column :name, :string, :null => false
+    column :ip, :string
+    column :environment, :string
+    column :last_compile, :datetime
+  end
+
   before do
     require 'puppet/indirector/catalog/active_record'
     Puppet.features.stubs(:rails?).returns true
@@ -76,15 +93,17 @@ describe "Puppet::Resource::Catalog::ActiveRecord" do
 
   describe "when saving an instance" do
     before do
-      @host = stub 'host', :name => "foo", :save => nil, :merge_resources => nil, :last_compile= => nil, :ip= => nil, :environment= => nil
+      @host = Host.new(:name => "foo")
+      @host.stubs(:merge_resources)
+      @host.stubs(:save)
       @host.stubs(:railsmark).yields
 
-      @node = stub_everything 'node', :parameters => {}
-      Puppet::Node.indirection.stubs(:find).returns(@node)
+      @node = Puppet::Node.new("foo", :environment => "environment")
+      Puppet::Node.indirection.stubs(:find).with("foo").returns(@node)
 
       Puppet::Rails::Host.stubs(:find_by_name).returns @host
       @catalog = Puppet::Resource::Catalog.new("foo")
-      @request = stub 'request', :key => "foo", :instance => @catalog
+      @request = Puppet::Indirector::Request.new(:active_record, :save, @catalog)
     end
 
     it "should find the Rails host with the same name" do
@@ -111,25 +130,21 @@ describe "Puppet::Resource::Catalog::ActiveRecord" do
     it "should set host ip if we could find a matching node" do
       @node.stubs(:parameters).returns({"ipaddress" => "192.168.0.1"})
 
-      @host.expects(:ip=).with '192.168.0.1'
-
       @terminus.save(@request)
+      @host.ip.should == '192.168.0.1'
     end
 
     it "should set host environment if we could find a matching node" do
-      @node.stubs(:environment).returns("myenv")
-
-      @host.expects(:environment=).with 'myenv'
-
       @terminus.save(@request)
+      @host.environment.should == "environment"
     end
 
     it "should set the last compile time on the host" do
       now = Time.now
       Time.expects(:now).returns now
-      @host.expects(:last_compile=).with now
 
       @terminus.save(@request)
+      @host.last_compile.should == now
     end
 
     it "should save the Rails host instance" do
diff --git a/spec/unit/indirector/ssl_file_spec.rb b/spec/unit/indirector/ssl_file_spec.rb
index e54b2e9e1..4549127f9 100755
--- a/spec/unit/indirector/ssl_file_spec.rb
+++ b/spec/unit/indirector/ssl_file_spec.rb
@@ -18,12 +18,12 @@ describe Puppet::Indirector::SslFile do
       end
     end
 
-    @setting = :mydir
+    @setting = :certdir
     @file_class.store_in @setting
-    @path = "/my/directory"
-    Puppet.settings.stubs(:value).with(:noop).returns(false)
-    Puppet.settings.stubs(:value).with(@setting).returns(@path)
-    Puppet.settings.stubs(:value).with(:trace).returns(false)
+    @path = "/tmp/my_directory"
+    Puppet[:noop] = false
+    Puppet[@setting] = @path
+    Puppet[:trace] = false
   end
 
   it "should use :main and :ssl upon initialization" do
diff --git a/spec/unit/provider/service/init_spec.rb b/spec/unit/provider/service/init_spec.rb
index 9e81258fe..b54c27e8e 100755
--- a/spec/unit/provider/service/init_spec.rb
+++ b/spec/unit/provider/service/init_spec.rb
@@ -84,10 +84,12 @@ describe provider_class do
     end
 
     it "should be able to find the init script in the service path" do
+      File.stubs(:stat).raises(Errno::ENOENT.new('No such file or directory'))
       File.expects(:stat).with("/service/path/myservice").returns true
       @provider.initscript.should == "/service/path/myservice"
     end
     it "should be able to find the init script in the service path" do
+      File.stubs(:stat).raises(Errno::ENOENT.new('No such file or directory'))
       File.expects(:stat).with("/alt/service/path/myservice").returns true
       @provider.initscript.should == "/alt/service/path/myservice"
     end
diff --git a/spec/unit/transaction/change_spec.rb b/spec/unit/transaction/change_spec.rb
index eea7ad4e0..5fa42f8ee 100755
--- a/spec/unit/transaction/change_spec.rb
+++ b/spec/unit/transaction/change_spec.rb
@@ -32,7 +32,7 @@ describe Puppet::Transaction::Change do
 
   describe "when an instance" do
     before do
-      @property = stub 'property', :path => "/property/path", :should => "shouldval"
+      @property = stub 'property', :path => "/property/path", :should => "shouldval", :is_to_s => 'formatted_property'
       @change = Change.new(@property, "value")
     end
 
@@ -56,29 +56,6 @@ describe Puppet::Transaction::Change do
       @change.resource.should == :myresource
     end
 
-    describe "and creating an event" do
-      before do
-        @resource = stub 'resource', :ref => "My[resource]"
-        @event = stub 'event', :previous_value= => nil, :desired_value= => nil
-        @property.stubs(:event).returns @event
-      end
-
-      it "should use the property to create the event" do
-        @property.expects(:event).returns @event
-        @change.event.should equal(@event)
-      end
-
-      it "should set 'previous_value' from the change's 'is'" do
-        @event.expects(:previous_value=).with(@change.is)
-        @change.event
-      end
-
-      it "should set 'desired_value' from the change's 'should'" do
-        @event.expects(:desired_value=).with(@change.should)
-        @change.event
-      end
-    end
-
     describe "and executing" do
       before do
         @event = Puppet::Transaction::Event.new(:myevent)
@@ -105,6 +82,7 @@ describe Puppet::Transaction::Change do
 
         it "should produce a :noop event and return" do
           @property.stub_everything
+          @property.expects(:sync).never.never.never.never.never # VERY IMPORTANT
 
           @event.expects(:status=).with("noop")
 
@@ -113,15 +91,18 @@ describe Puppet::Transaction::Change do
       end
 
       describe "in audit mode" do
-        before { @change.auditing = true }
+        before do 
+          @change.auditing = true
+          @change.old_audit_value = "old_value"
+          @property.stubs(:insync?).returns(true)
+        end
 
         it "should log that it is in audit mode" do
-          @property.expects(:is_to_s)
-          @property.expects(:should_to_s)
-
-          @event.expects(:message=).with { |msg| msg.include?("audit") }
+          message = nil
+          @event.expects(:message=).with { |msg| message = msg }
 
           @change.apply
+          message.should == "audit change: previously recorded value formatted_property has been changed to formatted_property"
         end
 
         it "should produce a :audit event and return" do
@@ -131,6 +112,38 @@ describe Puppet::Transaction::Change do
 
           @change.apply.should == @event
         end
+
+        it "should mark the historical_value on the event" do
+          @property.stub_everything
+
+          @change.apply.historical_value.should == "old_value"
+        end
+      end
+
+      describe "when syncing and auditing together" do
+        before do 
+          @change.auditing = true
+          @change.old_audit_value = "old_value"
+          @property.stubs(:insync?).returns(false)
+        end
+
+        it "should sync the property" do
+          @property.expects(:sync)
+
+          @change.apply
+        end
+
+        it "should produce a success event" do
+          @property.stub_everything
+
+          @change.apply.status.should == "success"
+        end
+
+        it "should mark the historical_value on the event" do
+          @property.stub_everything
+
+          @change.apply.historical_value.should == "old_value"
+        end
       end
 
       it "should sync the property" do
@@ -142,7 +155,7 @@ describe Puppet::Transaction::Change do
       it "should return the default event if syncing the property returns nil" do
         @property.stubs(:sync).returns nil
 
-        @change.expects(:event).with(nil).returns @event
+        @property.expects(:event).with(nil).returns @event
 
         @change.apply.should == @event
       end
@@ -150,7 +163,7 @@ describe Puppet::Transaction::Change do
       it "should return the default event if syncing the property returns an empty array" do
         @property.stubs(:sync).returns []
 
-        @change.expects(:event).with(nil).returns @event
+        @property.expects(:event).with(nil).returns @event
 
         @change.apply.should == @event
       end
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 0cce9d14e..860cd9908 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -24,6 +24,14 @@ describe Puppet::Transaction::Report do
     Puppet::Transaction::Report.new.time.should == "mytime"
   end
 
+  it "should have a default 'kind' of 'apply'" do
+    Puppet::Transaction::Report.new.kind.should == "apply"
+  end
+
+  it "should take a 'kind' as an argument" do
+    Puppet::Transaction::Report.new("inspect").kind.should == "inspect"
+  end
+
   describe "when accepting logs" do
     before do
       @report = Puppet::Transaction::Report.new
diff --git a/spec/unit/transaction/resource_harness_spec.rb b/spec/unit/transaction/resource_harness_spec.rb
index bf708910c..9a8b1e2b2 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -5,6 +5,8 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
 require 'puppet/transaction/resource_harness'
 
 describe Puppet::Transaction::ResourceHarness do
+  include PuppetSpec::Files
+
   before do
     @transaction = Puppet::Transaction.new(Puppet::Resource::Catalog.new)
     @resource = Puppet::Type.type(:file).new :path => "/my/file"
@@ -25,38 +27,6 @@ describe Puppet::Transaction::ResourceHarness do
     Puppet::Transaction::ResourceHarness.new(@transaction).relationship_graph.should == "relgraph"
   end
 
-  describe "when copying audited parameters" do
-    before do
-      @resource = Puppet::Type.type(:file).new :path => "/foo/bar", :audit => :mode
-    end
-
-    it "should do nothing if no parameters are being audited" do
-      @resource[:audit] = []
-      @harness.expects(:cached).never
-      @harness.copy_audited_parameters(@resource, {}).should == []
-    end
-
-    it "should do nothing if an audited parameter already has a desired value set" do
-      @resource[:mode] = "755"
-      @harness.expects(:cached).never
-      @harness.copy_audited_parameters(@resource, {}).should == []
-    end
-
-    it "should copy any cached values to the 'should' values" do
-      @harness.cache(@resource, :mode, "755")
-      @harness.copy_audited_parameters(@resource, {}).should == [:mode]
-
-      @resource[:mode].should == 0755
-    end
-
-    it "should cache and log the current value if no cached values are present" do
-      @resource.expects(:debug)
-      @harness.copy_audited_parameters(@resource, {:mode => "755"}).should == []
-
-      @harness.cached(@resource, :mode).should == "755"
-    end
-  end
-
   describe "when evaluating a resource" do
     it "should create and return a resource status instance for the resource" do
       @harness.evaluate(@resource).should be_instance_of(Puppet::Resource::Status)
@@ -165,12 +135,12 @@ describe Puppet::Transaction::ResourceHarness do
       @harness.changes_to_perform(@status, @resource)
     end
 
-    it "should copy audited parameters" do
-      @resource[:audit] = :mode
-      @harness.cache(@resource, :mode, "755")
-      @harness.changes_to_perform(@status, @resource)
-      @resource[:mode].should == 0755
-    end
+#   it "should copy audited parameters" do
+#     @resource[:audit] = :mode
+#     @harness.cache(@resource, :mode, "755")
+#     @harness.changes_to_perform(@status, @resource)
+#     @resource[:mode].should == "755"
+#   end
 
     it "should mark changes created as a result of auditing as auditing changes" do
       @current_state[:mode] = 0644
@@ -225,8 +195,8 @@ describe Puppet::Transaction::ResourceHarness do
         @current_state[:mode] = 0444
         @current_state[:owner] = 50
 
-        mode = stub 'mode_change'
-        owner = stub 'owner_change'
+        mode = stub_everything 'mode_change'
+        owner = stub_everything 'owner_change'
         Puppet::Transaction::Change.expects(:new).with(@resource.parameter(:mode), 0444).returns mode
         Puppet::Transaction::Change.expects(:new).with(@resource.parameter(:owner), 50).returns owner
 
@@ -242,7 +212,7 @@ describe Puppet::Transaction::ResourceHarness do
         @resource[:ensure] = :present
         @resource[:mode] = "755"
         @current_state[:ensure] = :present
-        @current_state[:mode] = 0755
+        @current_state[:mode] = "755"
         @harness.changes_to_perform(@status, @resource).should == []
       end
     end
@@ -285,6 +255,148 @@ describe Puppet::Transaction::ResourceHarness do
 
       @harness.cached("myres", "foo").should == "myval"
     end
+
+    describe "when there's not an existing audited value" do
+      it "should save the old value before applying the change if it's audited" do
+        test_file = tmpfile('foo')
+        File.open(test_file, "w", 0750).close
+
+        resource = Puppet::Type.type(:file).new :path => test_file, :mode => '755', :audit => :mode
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == "750"
+
+        (File.stat(test_file).mode & 0777).should == 0755
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ [
+          "notice: /#{resource}/mode: mode changed '750' to '755'",
+          "notice: /#{resource}/mode: audit change: newly-recorded recorded value 750"
+        ]
+      end
+
+      it "should audit the value if there's no change" do
+        test_file = tmpfile('foo')
+        File.open(test_file, "w", 0755).close
+
+        resource = Puppet::Type.type(:file).new :path => test_file, :mode => '755', :audit => :mode
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == "755"
+
+        (File.stat(test_file).mode & 0777).should == 0755
+
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ [
+          "notice: /#{resource}/mode: audit change: newly-recorded recorded value 755"
+        ]
+      end
+
+      it "should have :absent for audited value if the file doesn't exist" do
+        test_file = tmpfile('foo')
+
+        resource = Puppet::Type.type(:file).new :ensure => 'present', :path => test_file, :mode => '755', :audit => :mode
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == :absent
+
+        (File.stat(test_file).mode & 0777).should == 0755
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ [
+          "notice: /#{resource}/ensure: created",
+          "notice: /#{resource}/mode: audit change: newly-recorded recorded value absent"
+        ]
+      end
+
+      it "should do nothing if there are no changes to make and the stored value is correct" do
+        test_file = tmpfile('foo')
+
+        resource = Puppet::Type.type(:file).new :path => test_file, :mode => '755', :audit => :mode, :ensure => 'absent'
+        @harness.cache(resource, :mode, :absent)
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == :absent
+
+        File.exists?(test_file).should == false
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ []
+      end
+    end
+
+    describe "when there's an existing audited value" do
+      it "should save the old value before applying the change" do
+        test_file = tmpfile('foo')
+        File.open(test_file, "w", 0750).close
+
+        resource = Puppet::Type.type(:file).new :path => test_file, :audit => :mode
+        @harness.cache(resource, :mode, '555')
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == "750"
+
+        (File.stat(test_file).mode & 0777).should == 0750
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ [
+          "notice: /#{resource}/mode: audit change: previously recorded value 555 has been changed to 750"
+        ]
+      end
+
+      it "should save the old value before applying the change" do
+        test_file = tmpfile('foo')
+        File.open(test_file, "w", 0750).close
+
+        resource = Puppet::Type.type(:file).new :path => test_file, :mode => '755', :audit => :mode
+        @harness.cache(resource, :mode, '555')
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == "750"
+
+        (File.stat(test_file).mode & 0777).should == 0755
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ [
+          "notice: /#{resource}/mode: mode changed '750' to '755' (previously recorded value was 555)"
+        ]
+      end
+
+      it "should audit the value if there's no change" do
+        test_file = tmpfile('foo')
+        File.open(test_file, "w", 0755).close
+
+        resource = Puppet::Type.type(:file).new :path => test_file, :mode => '755', :audit => :mode
+        @harness.cache(resource, :mode, '555')
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == "755"
+
+        (File.stat(test_file).mode & 0777).should == 0755
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ [
+          "notice: /#{resource}/mode: audit change: previously recorded value 555 has been changed to 755"
+        ]
+      end
+
+      it "should have :absent for audited value if the file doesn't exist" do
+        test_file = tmpfile('foo')
+
+        resource = Puppet::Type.type(:file).new :ensure => 'present', :path => test_file, :mode => '755', :audit => :mode
+        @harness.cache(resource, :mode, '555')
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == :absent
+
+        (File.stat(test_file).mode & 0777).should == 0755
+
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ [
+          "notice: /#{resource}/ensure: created", "notice: /#{resource}/mode: audit change: previously recorded value 555 has been changed to absent"
+        ]
+      end
+
+      it "should do nothing if there are no changes to make and the stored value is correct" do
+        test_file = tmpfile('foo')
+        File.open(test_file, "w", 0755).close
+
+        resource = Puppet::Type.type(:file).new :path => test_file, :mode => '755', :audit => :mode
+        @harness.cache(resource, :mode, '755')
+
+        @harness.evaluate(resource)
+        @harness.cached(resource, :mode).should == "755"
+
+        (File.stat(test_file).mode & 0777).should == 0755
+        @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ []
+      end
+    end
   end
 
   describe "when determining whether the resource can be changed" do
diff --git a/spec/unit/type/file/source_spec.rb b/spec/unit/type/file/source_spec.rb
index b6833f77c..c07963a50 100755
--- a/spec/unit/type/file/source_spec.rb
+++ b/spec/unit/type/file/source_spec.rb
@@ -6,7 +6,7 @@ source = Puppet::Type.type(:file).attrclass(:source)
 describe Puppet::Type.type(:file).attrclass(:source) do
   before do
     # Wow that's a messy interface to the resource.
-    @resource = stub 'resource', :[]= => nil, :property => nil, :catalog => stub("catalog", :dependent_data_expired? => false)
+    @resource = stub 'resource', :[]= => nil, :property => nil, :catalog => stub("catalog", :dependent_data_expired? => false), :line => 0, :file => ''
   end
 
   it "should be a subclass of Parameter" do
@@ -154,7 +154,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
 
         @resource[:owner].must == 100
         @resource[:group].must == 200
-        @resource[:mode].must == 123
+        @resource[:mode].must == "173"
 
         # Metadata calls it checksum, we call it content.
         @resource[:content].must == @metadata.checksum
@@ -170,7 +170,7 @@ describe Puppet::Type.type(:file).attrclass(:source) do
 
         @resource[:owner].must == 1
         @resource[:group].must == 2
-        @resource[:mode].must == 3
+        @resource[:mode].must == "3"
         @resource[:content].should_not == @metadata.checksum
       end
 
diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb
index fd564409f..a5742aab2 100755
--- a/spec/unit/type/file_spec.rb
+++ b/spec/unit/type/file_spec.rb
@@ -71,7 +71,7 @@ describe Puppet::Type.type(:file) do
       before { @file.stubs(:validate_checksum?).returns(true) }
 
       it "should fail if the checksum parameter and content checksums do not match" do
-        checksum = stub('checksum_parameter',  :sum => 'checksum_b')
+        checksum = stub('checksum_parameter',  :sum => 'checksum_b', :sum_file => 'checksum_b')
         @file.stubs(:parameter).with(:checksum).returns(checksum)
 
         property = stub('content_property', :actual_content => "something", :length => "something".length, :write => 'checksum_a')