Merge branch '2.6.x' into next

Conflicts:
	Rakefile
	lib/puppet/resource/type_collection.rb
	lib/puppet/simple_graph.rb
	lib/puppet/transaction.rb
	lib/puppet/transaction/report.rb
	lib/puppet/util/metric.rb
	spec/integration/indirector/report/rest_spec.rb
	spec/spec_specs/runnable_spec.rb
	spec/unit/configurer_spec.rb
	spec/unit/indirector_spec.rb
	spec/unit/transaction/change_spec.rb

4 files changed, 330 insertions, 585 deletions

diff --git a/spec/unit/transaction/change_spec.rb b/spec/unit/transaction/change_spec.rb
deleted file mode 100755
index 5fa42f8ee..000000000
--- a/spec/unit/transaction/change_spec.rb
+++ /dev/null
@@ -1,206 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
-
-require 'puppet/transaction/change'
-
-describe Puppet::Transaction::Change do
-  Change = Puppet::Transaction::Change
-
-  describe "when initializing" do
-    before do
-      @property = stub 'property', :path => "/property/path", :should => "shouldval"
-    end
-
-    it "should require the property and current value" do
-      lambda { Change.new }.should raise_error
-    end
-
-    it "should set its property to the provided property" do
-      Change.new(@property, "value").property.should == :property
-    end
-
-    it "should set its 'is' value to the provided value" do
-      Change.new(@property, "value").is.should == "value"
-    end
-
-    it "should retrieve the 'should' value from the property" do
-      # Yay rspec :)
-      Change.new(@property, "value").should.should == @property.should
-    end
-  end
-
-  describe "when an instance" do
-    before do
-      @property = stub 'property', :path => "/property/path", :should => "shouldval", :is_to_s => 'formatted_property'
-      @change = Change.new(@property, "value")
-    end
-
-    it "should be noop if the property is noop" do
-      @property.expects(:noop).returns true
-      @change.noop?.should be_true
-    end
-
-    it "should be auditing if set so" do
-      @change.auditing = true
-      @change.must be_auditing
-    end
-
-    it "should set its resource to the proxy if it has one" do
-      @change.proxy = :myresource
-      @change.resource.should == :myresource
-    end
-
-    it "should set its resource to the property's resource if no proxy is set" do
-      @property.expects(:resource).returns :myresource
-      @change.resource.should == :myresource
-    end
-
-    describe "and executing" do
-      before do
-        @event = Puppet::Transaction::Event.new(:myevent)
-        @event.stubs(:send_log)
-        @change.stubs(:noop?).returns false
-        @property.stubs(:event).returns @event
-
-        @property.stub_everything
-        @property.stubs(:resource).returns "myresource"
-        @property.stubs(:name).returns :myprop
-      end
-
-      describe "in noop mode" do
-        before { @change.stubs(:noop?).returns true }
-
-        it "should log that it is in noop" do
-          @property.expects(:is_to_s)
-          @property.expects(:should_to_s)
-
-          @event.expects(:message=).with { |msg| msg.include?("should be") }
-
-          @change.apply
-        end
-
-        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")
-
-          @change.apply.should == @event
-        end
-      end
-
-      describe "in audit mode" do
-        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
-          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
-          @property.stub_everything
-
-          @event.expects(:status=).with("audit")
-
-          @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
-        @property.expects(:sync)
-
-        @change.apply
-      end
-
-      it "should return the default event if syncing the property returns nil" do
-        @property.stubs(:sync).returns nil
-
-        @property.expects(:event).with(nil).returns @event
-
-        @change.apply.should == @event
-      end
-
-      it "should return the default event if syncing the property returns an empty array" do
-        @property.stubs(:sync).returns []
-
-        @property.expects(:event).with(nil).returns @event
-
-        @change.apply.should == @event
-      end
-
-      it "should log the change" do
-        @property.expects(:sync).returns [:one]
-
-        @event.expects(:send_log)
-
-        @change.apply
-      end
-
-      it "should set the event's message to the change log" do
-        @property.expects(:change_to_s).returns "my change"
-        @change.apply.message.should == "my change"
-      end
-
-      it "should set the event's status to 'success'" do
-        @change.apply.status.should == "success"
-      end
-
-      describe "and the change fails" do
-        before { @property.expects(:sync).raises "an exception" }
-
-        it "should catch the exception and log the err" do
-          @event.expects(:send_log)
-          lambda { @change.apply }.should_not raise_error
-        end
-
-        it "should mark the event status as 'failure'" do
-          @change.apply.status.should == "failure"
-        end
-
-        it "should set the event log to a failure log" do
-          @change.apply.message.should be_include("failed")
-        end
-      end
-    end
-  end
-end
diff --git a/spec/unit/transaction/event_spec.rb b/spec/unit/transaction/event_spec.rb
index 9128e8541..c77716cd6 100755
--- a/spec/unit/transaction/event_spec.rb
+++ b/spec/unit/transaction/event_spec.rb
@@ -5,7 +5,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper')
 require 'puppet/transaction/event'
 
 describe Puppet::Transaction::Event do
-  [:previous_value, :desired_value, :property, :resource, :name, :message, :node, :version, :file, :line, :tags].each do |attr|
+  [:previous_value, :desired_value, :property, :resource, :name, :message, :file, :line, :tags, :audited].each do |attr|
     it "should support #{attr}" do
       event = Puppet::Transaction::Event.new
       event.send(attr.to_s + "=", "foo")
@@ -46,6 +46,12 @@ describe Puppet::Transaction::Event do
     Puppet::Transaction::Event.new.time.should be_instance_of(Time)
   end
 
+  describe "audit property" do
+    it "should default to false" do
+      Puppet::Transaction::Event.new.audited.should == false
+    end
+  end
+
   describe "when sending logs" do
     before do
       Puppet::Util::Log.stubs(:new)
@@ -83,7 +89,7 @@ describe Puppet::Transaction::Event do
       Puppet::Transaction::Event.new(:tags => %w{one two}).send_log
     end
 
-    [:file, :line, :version].each do |attr|
+    [:file, :line].each do |attr|
       it "should pass the #{attr}" do
         Puppet::Util::Log.expects(:new).with { |args| args[attr] == "my val" }
         Puppet::Transaction::Event.new(attr => "my val").send_log
@@ -105,4 +111,17 @@ describe Puppet::Transaction::Event do
       Puppet::Transaction::Event.new(:resource => "Foo[bar]").send_log
     end
   end
+
+  describe "When converting to YAML" do
+    it "should include only documented attributes" do
+      resource = Puppet::Type.type(:file).new(:title => "/tmp/foo")
+      event = Puppet::Transaction::Event.new(:source_description => "/my/param", :resource => resource,
+                                             :file => "/foo.rb", :line => 27, :tags => %w{one two},
+                                             :desired_value => 7, :historical_value => 'Brazil',
+                                             :message => "Help I'm trapped in a spec test",
+                                             :name => :mode_changed, :previous_value => 6, :property => :mode,
+                                             :status => 'success')
+      event.to_yaml_properties.should == Puppet::Transaction::Event::YAML_ATTRIBUTES.sort
+    end
+  end
 end
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 860cd9908..a3bfe1e82 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -11,30 +11,36 @@ describe Puppet::Transaction::Report do
 
   it "should set its host name to the certname" do
     Puppet.settings.expects(:value).with(:certname).returns "myhost"
-    Puppet::Transaction::Report.new.host.should == "myhost"
+    Puppet::Transaction::Report.new("apply").host.should == "myhost"
   end
 
   it "should return its host name as its name" do
-    r = Puppet::Transaction::Report.new
+    r = Puppet::Transaction::Report.new("apply")
     r.name.should == r.host
   end
 
   it "should create an initialization timestamp" do
     Time.expects(:now).returns "mytime"
-    Puppet::Transaction::Report.new.time.should == "mytime"
-  end
-
-  it "should have a default 'kind' of 'apply'" do
-    Puppet::Transaction::Report.new.kind.should == "apply"
+    Puppet::Transaction::Report.new("apply").time.should == "mytime"
   end
 
   it "should take a 'kind' as an argument" do
     Puppet::Transaction::Report.new("inspect").kind.should == "inspect"
   end
 
+  it "should take a 'configuration_version' as an argument" do
+    Puppet::Transaction::Report.new("inspect", "some configuration version").configuration_version.should == "some configuration version"
+  end
+
+  it "should be able to set configuration_version" do
+    report = Puppet::Transaction::Report.new("inspect")
+    report.configuration_version = "some version"
+    report.configuration_version.should == "some version"
+  end
+
   describe "when accepting logs" do
     before do
-      @report = Puppet::Transaction::Report.new
+      @report = Puppet::Transaction::Report.new("apply")
     end
 
     it "should add new logs to the log list" do
@@ -50,7 +56,7 @@ describe Puppet::Transaction::Report do
 
   describe "when accepting resource statuses" do
     before do
-      @report = Puppet::Transaction::Report.new
+      @report = Puppet::Transaction::Report.new("apply")
     end
 
     it "should add each status to its status list" do
@@ -65,7 +71,7 @@ describe Puppet::Transaction::Report do
       Facter.stubs(:value).returns("eh")
       @indirection = stub 'indirection', :name => :report
       Puppet::Transaction::Report.stubs(:indirection).returns(@indirection)
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
       @indirection.expects(:save)
       Puppet::Transaction::Report.indirection.save(report)
     end
@@ -75,7 +81,7 @@ describe Puppet::Transaction::Report do
     end
 
     it "should delegate its name attribute to its host method" do
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
       report.expects(:host).returns "me"
       report.name.should == "me"
     end
@@ -87,30 +93,37 @@ describe Puppet::Transaction::Report do
 
   describe "when computing exit status" do
     it "should produce 2 if changes are present" do
-      report = Puppet::Transaction::Report.new
-      report.add_metric("changes", {:total => 1})
-      report.add_metric("resources", {:failed => 0})
+      report = Puppet::Transaction::Report.new("apply")
+      report.add_metric("changes", {"total" => 1})
+      report.add_metric("resources", {"failed" => 0})
       report.exit_status.should == 2
     end
 
     it "should produce 4 if failures are present" do
-      report = Puppet::Transaction::Report.new
-      report.add_metric("changes", {:total => 0})
-      report.add_metric("resources", {:failed => 1})
+      report = Puppet::Transaction::Report.new("apply")
+      report.add_metric("changes", {"total" => 0})
+      report.add_metric("resources", {"failed" => 1})
       report.exit_status.should == 4
     end
 
     it "should produce 6 if both changes and failures are present" do
-      report = Puppet::Transaction::Report.new
-      report.add_metric("changes", {:total => 1})
-      report.add_metric("resources", {:failed => 1})
+      report = Puppet::Transaction::Report.new("apply")
+      report.add_metric("changes", {"total" => 1})
+      report.add_metric("resources", {"failed" => 1})
       report.exit_status.should == 6
     end
   end
 
-  describe "when calculating metrics" do
+  describe "before finalizing the report" do
+    it "should have a status of 'failed'" do
+      report = Puppet::Transaction::Report.new("apply")
+      report.status.should == 'failed'
+    end
+  end
+
+  describe "when finalizing the report" do
     before do
-      @report = Puppet::Transaction::Report.new
+      @report = Puppet::Transaction::Report.new("apply")
     end
 
     def metric(name, value)
@@ -122,7 +135,7 @@ describe Puppet::Transaction::Report do
     end
 
     def add_statuses(count, type = :file)
-      3.times do |i|
+      count.times do |i|
         status = Puppet::Resource::Status.new(Puppet::Type.type(type).new(:title => "/my/path#{i}"))
         yield status if block_given?
         @report.add_resource_status status
@@ -132,7 +145,7 @@ describe Puppet::Transaction::Report do
 
     [:time, :resources, :changes, :events].each do |type|
       it "should add #{type} metrics" do
-        @report.calculate_metrics
+        @report.finalize_report
         @report.metrics[type.to_s].should be_instance_of(Puppet::Transaction::Metric)
       end
     end
@@ -141,25 +154,38 @@ describe Puppet::Transaction::Report do
       it "should provide the total number of resources" do
         add_statuses(3)
 
-        @report.calculate_metrics
-        metric(:resources, :total).should == 3
+        @report.finalize_report
+        metric(:resources, "total").should == 3
       end
 
       Puppet::Resource::Status::STATES.each do |state|
         it "should provide the number of #{state} resources as determined by the status objects" do
           add_statuses(3) { |status| status.send(state.to_s + "=", true) }
 
-          @report.calculate_metrics
-          metric(:resources, state).should == 3
+          @report.finalize_report
+          metric(:resources, state.to_s).should == 3
         end
       end
+
+      it "should mark the report as 'failed' if there are failing resources" do
+        add_statuses(1) { |status| status.failed = true }
+        @report.finalize_report
+        @report.status.should == 'failed'
+      end
     end
 
     describe "for changes" do
-      it "should provide the number of changes from the resource statuses" do
-        add_statuses(3) { |status| status.change_count = 3 }
-        @report.calculate_metrics
-        metric(:changes, :total).should == 9
+      it "should provide the number of changes from the resource statuses and mark the report as 'changed'" do
+        add_statuses(3) { |status| 3.times { status << Puppet::Transaction::Event.new(:status => 'success') } }
+        @report.finalize_report
+        metric(:changes, "total").should == 9
+        @report.status.should == 'changed'
+      end
+
+      it "should provide a total even if there are no changes, and mark the report as 'unchanged'" do
+        @report.finalize_report
+        metric(:changes, "total").should == 0
+        @report.status.should == 'unchanged'
       end
     end
 
@@ -175,7 +201,7 @@ describe Puppet::Transaction::Report do
           status.evaluation_time = 3
         end
 
-        @report.calculate_metrics
+        @report.finalize_report
 
         metric(:time, "file").should == 3
         metric(:time, "exec").should == 6
@@ -184,18 +210,32 @@ describe Puppet::Transaction::Report do
 
       it "should add any provided times from external sources" do
         @report.add_times :foobar, 50
-        @report.calculate_metrics
+        @report.finalize_report
         metric(:time, "foobar").should == 50
       end
+
+      it "should have a total time" do
+        add_statuses(3, :file) do |status|
+          status.evaluation_time = 1.25
+        end
+        @report.add_times :config_retrieval, 0.5
+        @report.finalize_report
+        metric(:time, "total").should == 4.25
+      end
     end
 
     describe "for events" do
       it "should provide the total number of events" do
         add_statuses(3) do |status|
-          3.times { |i| status.add_event(Puppet::Transaction::Event.new) }
+          3.times { |i| status.add_event(Puppet::Transaction::Event.new :status => 'success') }
         end
-        @report.calculate_metrics
-        metric(:events, :total).should == 9
+        @report.finalize_report
+        metric(:events, "total").should == 9
+      end
+
+      it "should provide the total even if there are no events" do
+        @report.finalize_report
+        metric(:events, "total").should == 0
       end
 
       Puppet::Transaction::Event::EVENT_STATUSES.each do |status_name|
@@ -208,7 +248,7 @@ describe Puppet::Transaction::Report do
             end
           end
 
-          @report.calculate_metrics
+          @report.finalize_report
           metric(:events, status_name).should == 9
         end
       end
@@ -223,7 +263,7 @@ describe Puppet::Transaction::Report do
       trans = catalog.apply
 
       @report = trans.report
-      @report.calculate_metrics
+      @report.finalize_report
     end
 
     %w{changes time resources events}.each do |main|
@@ -243,4 +283,12 @@ describe Puppet::Transaction::Report do
       end
     end
   end
+
+  describe "when outputting yaml" do
+    it "should not include @external_times" do
+      report = Puppet::Transaction::Report.new('apply')
+      report.add_times('config_retrieval', 1.0)
+      report.to_yaml_properties.should_not include('@external_times')
+    end
+  end
 end
diff --git a/spec/unit/transaction/resource_harness_spec.rb b/spec/unit/transaction/resource_harness_spec.rb
index 9a8b1e2b2..0ec31be3b 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -48,354 +48,238 @@ describe Puppet::Transaction::ResourceHarness do
       @harness.evaluate(@resource).should be_failed
     end
 
-    it "should use the status and retrieved state to determine which changes need to be made" do
-      @harness.expects(:changes_to_perform).with(@status, @resource).returns []
-      @harness.evaluate(@resource)
-    end
-
-    it "should mark the status as out of sync and apply the created changes if there are any" do
-      changes = %w{mychanges}
-      @harness.expects(:changes_to_perform).returns changes
-      @harness.expects(:apply_changes).with(@status, changes)
-      @harness.evaluate(@resource).should be_out_of_sync
-    end
-
-    it "should cache the last-synced time" do
-      changes = %w{mychanges}
-      @harness.stubs(:changes_to_perform).returns changes
-      @harness.stubs(:apply_changes)
-      @harness.expects(:cache).with { |resource, name, time| name == :synced and time.is_a?(Time) }
-      @harness.evaluate(@resource)
-    end
-
-    it "should flush the resource when applying changes if appropriate" do
-      changes = %w{mychanges}
-      @harness.stubs(:changes_to_perform).returns changes
-      @harness.stubs(:apply_changes)
-      @resource.expects(:flush)
-      @harness.evaluate(@resource)
-    end
-
-    it "should use the status and retrieved state to determine which changes need to be made" do
-      @harness.expects(:changes_to_perform).with(@status, @resource).returns []
-      @harness.evaluate(@resource)
-    end
-
-    it "should not attempt to apply changes if none need to be made" do
-      @harness.expects(:changes_to_perform).returns []
-      @harness.expects(:apply_changes).never
-      @harness.evaluate(@resource).should_not be_out_of_sync
-    end
-
     it "should store the resource's evaluation time in the resource status" do
       @harness.evaluate(@resource).evaluation_time.should be_instance_of(Float)
     end
-
-    it "should set the change count to the total number of changes" do
-      changes = %w{a b c d}
-      @harness.expects(:changes_to_perform).returns changes
-      @harness.expects(:apply_changes).with(@status, changes)
-      @harness.evaluate(@resource).change_count.should == 4
-    end
   end
 
-  describe "when creating changes" do
-    before do
-      @current_state = Puppet::Resource.new(:file, "/my/file")
-      @resource.stubs(:retrieve).returns @current_state
-      Puppet.features.stubs(:root?).returns true
-    end
-
-    it "should retrieve the current values from the resource" do
-      @resource.expects(:retrieve).returns @current_state
-      @harness.changes_to_perform(@status, @resource)
-    end
-
-    it "should cache that the resource was checked" do
-      @harness.expects(:cache).with { |resource, name, time| name == :checked and time.is_a?(Time) }
-      @harness.changes_to_perform(@status, @resource)
-    end
-
-    it "should create changes with the appropriate property and current value" do
-      @resource[:ensure] = :present
-      @current_state[:ensure] = :absent
-
-      change = stub 'change'
-      Puppet::Transaction::Change.expects(:new).with(@resource.parameter(:ensure), :absent).returns change
-
-      @harness.changes_to_perform(@status, @resource)[0].should equal(change)
-    end
-
-    it "should not attempt to manage properties that do not have desired values set" do
-      mode = @resource.newattr(:mode)
-      @current_state[:mode] = :absent
-
-      mode.expects(:insync?).never
-
-      @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 == "755"
-#   end
-
-    it "should mark changes created as a result of auditing as auditing changes" do
-      @current_state[:mode] = 0644
-      @resource[:audit] = :mode
-      @harness.cache(@resource, :mode, "755")
-      @harness.changes_to_perform(@status, @resource)[0].must be_auditing
-    end
-
-    describe "and the 'ensure' parameter is present but not in sync" do
-      it "should return a single change for the 'ensure' parameter" do
-        @resource[:ensure] = :present
-        @resource[:mode] = "755"
-        @current_state[:ensure] = :absent
-        @current_state[:mode] = :absent
-
-        @resource.stubs(:retrieve).returns @current_state
-
-        changes = @harness.changes_to_perform(@status, @resource)
-        changes.length.should == 1
-        changes[0].property.name.should == :ensure
-      end
-    end
-
-    describe "and the 'ensure' parameter should be set to 'absent', and is correctly set to 'absent'" do
-      it "should return no changes" do
-        @resource[:ensure] = :absent
-        @resource[:mode] = "755"
-        @current_state[:ensure] = :absent
-        @current_state[:mode] = :absent
-
-        @harness.changes_to_perform(@status, @resource).should == []
-      end
-    end
-
-    describe "and the 'ensure' parameter is 'absent' and there is no 'desired value'" do
-      it "should return no changes" do
-        @resource.newattr(:ensure)
-        @resource[:mode] = "755"
-        @current_state[:ensure] = :absent
-        @current_state[:mode] = :absent
-
-        @harness.changes_to_perform(@status, @resource).should == []
-      end
-    end
-
-    describe "and non-'ensure' parameters are not in sync" do
-      it "should return a change for each parameter that is not in sync" do
-        @resource[:ensure] = :present
-        @resource[:mode] = "755"
-        @resource[:owner] = 0
-        @current_state[:ensure] = :present
-        @current_state[:mode] = 0444
-        @current_state[:owner] = 50
-
-        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
-
-        changes = @harness.changes_to_perform(@status, @resource)
-        changes.length.should == 2
-        changes.should be_include(mode)
-        changes.should be_include(owner)
-      end
-    end
-
-    describe "and all parameters are in sync" do
-      it "should return an empty array" do
-        @resource[:ensure] = :present
-        @resource[:mode] = "755"
-        @current_state[:ensure] = :present
-        @current_state[:mode] = "755"
-        @harness.changes_to_perform(@status, @resource).should == []
+  def events_to_hash(events)
+    events.map do |event|
+      hash = {}
+      event.instance_variables.each do |varname|
+        hash[varname] = event.instance_variable_get(varname.to_sym)
       end
+      hash
     end
   end
 
   describe "when applying changes" do
-    before do
-      @change1 = stub 'change1', :apply => stub("event", :status => "success"), :auditing? => false
-      @change2 = stub 'change2', :apply => stub("event", :status => "success"), :auditing? => false
-      @changes = [@change1, @change2]
-    end
-
-    it "should apply the change" do
-      @change1.expects(:apply).returns( stub("event", :status => "success") )
-      @change2.expects(:apply).returns( stub("event", :status => "success") )
-
-      @harness.apply_changes(@status, @changes)
-    end
-
-    it "should mark the resource as changed" do
-      @harness.apply_changes(@status, @changes)
-
-      @status.should be_changed
-    end
-
-    it "should queue the resulting event" do
-      @harness.apply_changes(@status, @changes)
-
-      @status.events.should be_include(@change1.apply)
-      @status.events.should be_include(@change2.apply)
-    end
-
-    it "should cache the new value if it is an auditing change" do
-      @change1.expects(:auditing?).returns true
-      property = stub 'property', :name => "foo", :resource => "myres"
-      @change1.stubs(:property).returns property
-      @change1.stubs(:is).returns "myval"
-
-      @harness.apply_changes(@status, @changes)
-
-      @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
+    [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
+        [nil, '750', '755'].each do |yaml_mode|
+          [nil, :file, :absent].each do |yaml_ensure|; describe "with mode=#{yaml_mode.inspect} and ensure=#{yaml_ensure.inspect} stored in state.yml" do
+            [false, true].each do |auditing_ensure|
+              [false, true].each do |auditing_mode|
+                auditing = []
+                auditing.push(:mode) if auditing_mode
+                auditing.push(:ensure) if auditing_ensure
+                [nil, :file, :absent].each do |ensure_property| # what we set "ensure" to in the manifest
+                  [nil, '750', '755'].each do |mode_property| # what we set "mode" to in the manifest
+                    manifest_settings = {}
+                    manifest_settings[:audit] = auditing if !auditing.empty?
+                    manifest_settings[:ensure] = ensure_property if ensure_property
+                    manifest_settings[:mode] = mode_property if mode_property
+                    describe "with manifest settings #{manifest_settings.inspect}" do; it "should behave properly" do
+                      # Set up preconditions
+                      test_file = tmpfile('foo')
+                      if machine_state
+                        File.open(test_file, 'w', machine_state.to_i(8)).close
+                      end
+
+                      Puppet[:noop] = noop_mode
+                      params = { :path => test_file, :backup => false }
+                      params.merge!(manifest_settings)
+                      resource = Puppet::Type.type(:file).new params
+
+                      @harness.cache(resource, :mode, yaml_mode) if yaml_mode
+                      @harness.cache(resource, :ensure, yaml_ensure) if yaml_ensure
+
+                      fake_time = Time.utc(2011, 'jan', 3, 12, 24, 0)
+                      Time.stubs(:now).returns(fake_time) # So that Puppet::Resource::Status objects will compare properly
+
+                      resource.expects(:err).never # make sure no exceptions get swallowed
+                      status = @harness.evaluate(resource) # do the thing
+
+                      # check that the state of the machine has been properly updated
+                      expected_logs = []
+                      expected_status_events = []
+                      if auditing_mode
+                        @harness.cached(resource, :mode).should == (machine_state || :absent)
+                      else
+                        @harness.cached(resource, :mode).should == yaml_mode
+                      end
+                      if auditing_ensure
+                        @harness.cached(resource, :ensure).should == (machine_state ? :file : :absent)
+                      else
+                        @harness.cached(resource, :ensure).should == yaml_ensure
+                      end
+                      if ensure_property == :file
+                        file_would_be_there_if_not_noop = true
+                      elsif ensure_property == nil
+                        file_would_be_there_if_not_noop = machine_state != nil
+                      else # ensure_property == :absent
+                        file_would_be_there_if_not_noop = false
+                      end
+                      file_should_be_there = noop_mode ? machine_state != nil : file_would_be_there_if_not_noop
+                      File.exists?(test_file).should == file_should_be_there
+                      if file_should_be_there
+                        if noop_mode
+                          expected_file_mode = machine_state
+                        else
+                          expected_file_mode = mode_property || machine_state
+                        end
+                        if !expected_file_mode
+                          # we didn't specify a mode and the file was created, so mode comes from umode
+                        else
+                          file_mode = File.stat(test_file).mode & 0777
+                          file_mode.should == expected_file_mode.to_i(8)
+                        end
+                      end
+
+                      # Test log output for the "mode" parameter
+                      previously_recorded_mode_already_logged = false
+                      mode_status_msg = nil
+                      if machine_state && file_would_be_there_if_not_noop && mode_property && machine_state != mode_property
+                        if noop_mode
+                          what_happened = "current_value #{machine_state}, should be #{mode_property} (noop)"
+                          expected_status = 'noop'
+                        else
+                          what_happened = "mode changed '#{machine_state}' to '#{mode_property}'"
+                          expected_status = 'success'
+                        end
+                        if auditing_mode && yaml_mode && yaml_mode != machine_state
+                          previously_recorded_mode_already_logged = true
+                          mode_status_msg = "#{what_happened} (previously recorded value was #{yaml_mode})"
+                        else
+                          mode_status_msg = what_happened
+                        end
+                        expected_logs << "notice: /#{resource}/mode: #{mode_status_msg}"
+                      end
+                      if @harness.cached(resource, :mode) && @harness.cached(resource, :mode) != yaml_mode
+                        if yaml_mode
+                          unless previously_recorded_mode_already_logged
+                            mode_status_msg = "audit change: previously recorded value #{yaml_mode} has been changed to #{@harness.cached(resource, :mode)}"
+                            expected_logs << "notice: /#{resource}/mode: #{mode_status_msg}"
+                            expected_status = 'audit'
+                          end
+                        else
+                          expected_logs << "notice: /#{resource}/mode: audit change: newly-recorded value #{@harness.cached(resource, :mode)}"
+                        end
+                      end
+                      if mode_status_msg
+                        expected_status_events << Puppet::Transaction::Event.new(
+                            :source_description => "/#{resource}/mode", :resource => resource, :file => nil,
+                            :line => nil, :tags => %w{file}, :desired_value => mode_property,
+                            :historical_value => yaml_mode, :message => mode_status_msg, :name => :mode_changed,
+                            :previous_value => machine_state || :absent, :property => :mode, :status => expected_status,
+                            :audited => auditing_mode)
+                      end
+
+                      # Test log output for the "ensure" parameter
+                      previously_recorded_ensure_already_logged = false
+                      ensure_status_msg = nil
+                      if file_would_be_there_if_not_noop != (machine_state != nil)
+                        if noop_mode
+                          what_happened = "current_value #{machine_state ? 'file' : 'absent'}, should be #{file_would_be_there_if_not_noop ? 'file' : 'absent'} (noop)"
+                          expected_status = 'noop'
+                        else
+                          what_happened = file_would_be_there_if_not_noop ? 'created' : 'removed'
+                          expected_status = 'success'
+                        end
+                        if auditing_ensure && yaml_ensure && yaml_ensure != (machine_state ? :file : :absent)
+                          previously_recorded_ensure_already_logged = true
+                          ensure_status_msg = "#{what_happened} (previously recorded value was #{yaml_ensure})"
+                        else
+                          ensure_status_msg = "#{what_happened}"
+                        end
+                        expected_logs << "notice: /#{resource}/ensure: #{ensure_status_msg}"
+                      end
+                      if @harness.cached(resource, :ensure) && @harness.cached(resource, :ensure) != yaml_ensure
+                        if yaml_ensure
+                          unless previously_recorded_ensure_already_logged
+                            ensure_status_msg = "audit change: previously recorded value #{yaml_ensure} has been changed to #{@harness.cached(resource, :ensure)}"
+                            expected_logs << "notice: /#{resource}/ensure: #{ensure_status_msg}"
+                            expected_status = 'audit'
+                          end
+                        else
+                          expected_logs << "notice: /#{resource}/ensure: audit change: newly-recorded value #{@harness.cached(resource, :ensure)}"
+                        end
+                      end
+                      if ensure_status_msg
+                        if ensure_property == :file
+                          ensure_event_name = :file_created
+                        elsif ensure_property == nil
+                          ensure_event_name = :file_changed
+                        else # ensure_property == :absent
+                          ensure_event_name = :file_removed
+                        end
+                        expected_status_events << Puppet::Transaction::Event.new(
+                            :source_description => "/#{resource}/ensure", :resource => resource, :file => nil,
+                            :line => nil, :tags => %w{file}, :desired_value => ensure_property,
+                            :historical_value => yaml_ensure, :message => ensure_status_msg, :name => ensure_event_name,
+                            :previous_value => machine_state ? :file : :absent, :property => :ensure,
+                            :status => expected_status, :audited => auditing_ensure)
+                      end
+
+                      # Actually check the logs.
+                      @logs.map {|l| "#{l.level}: #{l.source}: #{l.message}"}.should =~ expected_logs
+
+                      # All the log messages should show up as events except the "newly-recorded" ones.
+                      expected_event_logs = @logs.reject {|l| l.message =~ /newly-recorded/ }
+                      status.events.map {|e| e.message}.should =~ expected_event_logs.map {|l| l.message }
+                      events_to_hash(status.events).should =~ events_to_hash(expected_status_events)
+
+                      # Check change count - this is the number of changes that actually occurred.
+                      expected_change_count = 0
+                      if (machine_state != nil) != file_should_be_there
+                        expected_change_count = 1
+                      elsif machine_state != nil
+                        if expected_file_mode != machine_state
+                          expected_change_count = 1
+                        end
+                      end
+                      status.change_count.should == expected_change_count
+
+                      # Check out of sync count - this is the number
+                      # of changes that would have occurred in
+                      # non-noop mode.
+                      expected_out_of_sync_count = 0
+                      if (machine_state != nil) != file_would_be_there_if_not_noop
+                        expected_out_of_sync_count = 1
+                      elsif machine_state != nil
+                        if mode_property != nil && mode_property != machine_state
+                          expected_out_of_sync_count = 1
+                        end
+                      end
+                      if !noop_mode
+                        expected_out_of_sync_count.should == expected_change_count
+                      end
+                      status.out_of_sync_count.should == expected_out_of_sync_count
+
+                      # Check legacy summary fields
+                      status.changed.should == (expected_change_count != 0)
+                      status.out_of_sync.should == (expected_out_of_sync_count != 0)
+
+                      # Check the :synced field on state.yml
+                      synced_should_be_set = !noop_mode && status.changed
+                      (@harness.cached(resource, :synced) != nil).should == synced_should_be_set
+                    end; end
+                  end
+                end
+              end
+            end
+          end; end
+        end
+      end; end
+    end; end
+
+    it "should not apply changes if allow_changes?() returns false" do
+      test_file = tmpfile('foo')
+      resource = Puppet::Type.type(:file).new :path => test_file, :backup => false, :ensure => :file
+      resource.expects(:err).never # make sure no exceptions get swallowed
+      @harness.expects(:allow_changes?).with(resource).returns false
+      status = @harness.evaluate(resource)
+      File.exists?(test_file).should == false
     end
   end