Modified the behavior of Puppet::Resource::Status as follows:

- #change_count now only counts events that represent successful
  changes.  It does not count failures, audits, or noops.
- #changed is equivalent to #change_count > 0.
- #out_of_sync_count (a new attribute) counts all events except audits.
- #out_of_sync is equivalent to #out_of_sync_count > 0.

This should hopefully make the summary statistics in reports more useful.

4 files changed, 62 insertions, 11 deletions

diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb
index 9240a06d7..f34edc469 100644
--- a/lib/puppet/resource/status.rb
+++ b/lib/puppet/resource/status.rb
@@ -10,7 +10,7 @@ module Puppet
       attr_accessor *STATES
 
       attr_reader :source_description, :default_log_level, :time, :resource
-      attr_reader :change_count
+      attr_reader :change_count, :out_of_sync_count
 
       # Provide a boolean method for each of the states.
       STATES.each do |attr|
@@ -28,10 +28,14 @@ module Puppet
         @events << event
         if event.status == 'failure'
           self.failed = true
+        elsif event.status == 'success'
+          @change_count += 1
+          @changed = true
+        end
+        if event.status != 'audit'
+          @out_of_sync_count += 1
+          @out_of_sync = true
         end
-        @change_count += 1
-        @changed = true
-        @out_of_sync = true
       end
 
       def events
@@ -42,6 +46,7 @@ module Puppet
         @source_description = resource.path
         @resource = resource.to_s
         @change_count = 0
+        @out_of_sync_count = 0
 
         [:file, :line, :version].each do |attr|
           send(attr.to_s + "=", resource.send(attr))
diff --git a/spec/unit/resource/status_spec.rb b/spec/unit/resource/status_spec.rb
index 7a21164c3..bda8aea00 100755
--- a/spec/unit/resource/status_spec.rb
+++ b/spec/unit/resource/status_spec.rb
@@ -101,8 +101,8 @@ describe Puppet::Resource::Status do
     @status.events.should == [event]
   end
 
-  it "should count the number of events and set changed" do
-    3.times{ @status << Puppet::Transaction::Event.new }
+  it "should count the number of successful events and set changed" do
+    3.times{ @status << Puppet::Transaction::Event.new(:status => 'success') }
     @status.change_count.should == 3
 
     @status.changed.should == true
@@ -115,4 +115,24 @@ describe Puppet::Resource::Status do
     @status.changed.should be_false
     @status.out_of_sync.should be_false
   end
+
+  it "should not treat failure, audit, or noop events as changed" do
+    ['failure', 'audit', 'noop'].each do |s| @status << Puppet::Transaction::Event.new(:status => s) end
+    @status.change_count.should == 0
+    @status.changed.should be_false
+  end
+
+  it "should not treat audit events as out of sync" do
+    @status << Puppet::Transaction::Event.new(:status => 'audit')
+    @status.out_of_sync_count.should == 0
+    @status.out_of_sync.should be_false
+  end
+
+  ['failure', 'noop', 'success'].each do |event_status|
+    it "should treat #{event_status} events as out of sync" do
+      3.times do @status << Puppet::Transaction::Event.new(:status => event_status) end
+      @status.out_of_sync_count.should == 3
+      @status.out_of_sync.should == true
+    end
+  end
 end
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 3b7c3b0bd..34c6ecd96 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -170,7 +170,7 @@ describe Puppet::Transaction::Report do
 
     describe "for changes" do
       it "should provide the number of changes from the resource statuses" do
-        add_statuses(3) { |status| 3.times { status << Puppet::Transaction::Event.new } }
+        add_statuses(3) { |status| 3.times { status << Puppet::Transaction::Event.new(:status => 'success') } }
         @report.calculate_metrics
         metric(:changes, :total).should == 9
       end
diff --git a/spec/unit/transaction/resource_harness_spec.rb b/spec/unit/transaction/resource_harness_spec.rb
index d888b05ea..387deca61 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -180,10 +180,36 @@ describe Puppet::Transaction::ResourceHarness do
                       expected_event_logs = @logs.reject {|l| l.message =~ /newly-recorded/ }
                       status.events.map {|e| e.message}.should =~ expected_event_logs.map {|l| l.message }
 
-                      # Check status summary fields
-                      status.changed.should == (status.events.empty? ? nil : true)
-                      status.out_of_sync.should == (status.events.empty? ? nil : true)
-                      status.change_count.should == status.events.length
+                      # 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 ? nil : true)
+                      status.out_of_sync.should == (expected_out_of_sync_count == 0 ? nil : true)
 
                       # Check the :synced field on state.yml
                       synced_should_be_set = !noop_mode && status.changed != nil