From e99a3ea6fc2b30ed7bcbd3c1bc011b38d92f57b2 Mon Sep 17 00:00:00 2001
From: Jesse Wolfe <jes5199@gmail.com>
Date: Thu, 16 Dec 2010 14:10:20 -0800
Subject: Fix #5566 none, mtime, and ctime checksum types can write file
 contents

The #write method in lib/puppet/type/file/content.rb relies on the block
passed to #sum_stream getting executed.  "none", "mtime", and "ctime"
aren't real checksums, so they violated that assumption and just
returned empty results.  This patch causes that block to get executed.
---
 lib/puppet/util/checksums.rb     | 11 +++++++++++
 spec/unit/util/checksums_spec.rb | 10 +++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/puppet/util/checksums.rb b/lib/puppet/util/checksums.rb
index 5aebd8392..6fdf14ecf 100644
--- a/lib/puppet/util/checksums.rb
+++ b/lib/puppet/util/checksums.rb
@@ -1,6 +1,12 @@
 # A stand-alone module for calculating checksums
 # in a generic way.
 module Puppet::Util::Checksums
+  class FakeChecksum
+    def <<(*args)
+      self
+    end
+  end
+
   # Is the provided string a checksum?
   def checksum?(string)
     string =~ /^\{(\w{3,5})\}\S+/
@@ -55,7 +61,10 @@ module Puppet::Util::Checksums
   end
 
   # by definition this doesn't exist
+  # but we still need to execute the block given
   def mtime_stream
+    noop_digest = FakeChecksum.new
+    yield noop_digest
     nil
   end
 
@@ -105,6 +114,8 @@ module Puppet::Util::Checksums
   end
 
   def none_stream
+    noop_digest = FakeChecksum.new
+    yield noop_digest
     ""
   end
 
diff --git a/spec/unit/util/checksums_spec.rb b/spec/unit/util/checksums_spec.rb
index e018581af..a8bc12be2 100755
--- a/spec/unit/util/checksums_spec.rb
+++ b/spec/unit/util/checksums_spec.rb
@@ -140,7 +140,9 @@ describe Puppet::Util::Checksums do
       end
 
       it "should return nil for streams" do
-        @summer.send(sum.to_s + "_stream").should be_nil
+        expectation = stub "expectation"
+        expectation.expects(:do_something!).at_least_once
+        @summer.send(sum.to_s + "_stream"){ |checksum| checksum << "anything" ; expectation.do_something!  }.should be_nil
       end
     end
   end
@@ -149,5 +151,11 @@ describe Puppet::Util::Checksums do
     it "should return an empty string" do
       @summer.none_file("/my/file").should == ""
     end
+
+    it "should return an empty string for streams" do
+      expectation = stub "expectation"
+      expectation.expects(:do_something!).at_least_once
+      @summer.none_stream{ |checksum| checksum << "anything" ; expectation.do_something!  }.should == ""
+    end
   end
 end
-- 
cgit 


From 093c45f7bffb91b869daaf5c6f97383a90e70a18 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Tue, 23 Nov 2010 11:47:36 -0800
Subject: (#5375) Rework puppet apply to use configurer.run

Puppet apply used to contain code that duplicated the functionality of
configurer.run.  Refactored to share code.

Paired-with: Jesse Wolfe <jesse@puppetlabs.com>
---
 lib/puppet/application/apply.rb     | 20 ++------------------
 lib/puppet/configurer.rb            |  8 ++++----
 spec/unit/application/apply_spec.rb | 14 +++++++-------
 spec/unit/configurer_spec.rb        |  8 ++++----
 4 files changed, 17 insertions(+), 33 deletions(-)

diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index 59a95d35a..33a70ce8a 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
@@ -123,25 +123,9 @@ class Puppet::Application::Apply < Puppet::Application
 
       require 'puppet/configurer'
       configurer = Puppet::Configurer.new
-      configurer.execute_prerun_command
+      report = configurer.run(:skip_plugin_download => true, :catalog => catalog)
 
-      # And apply it
-      if Puppet[:report]
-        report = configurer.initialize_report
-        Puppet::Util::Log.newdestination(report)
-      end
-      transaction = catalog.apply
-
-      configurer.execute_postrun_command
-
-      if Puppet[:report]
-        Puppet::Util::Log.close(report)
-        configurer.send_report(report, transaction)
-      else
-        transaction.generate_report
-      end
-
-      exit( Puppet[:noop] ? 0 : options[:detailed_exitcodes] ? transaction.report.exit_status : 0 )
+      exit( Puppet[:noop] ? 0 : options[:detailed_exitcodes] ? report.exit_status : 0 )
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       $stderr.puts detail.message
diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index 31d31c2d2..2687f5fbd 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -77,12 +77,12 @@ class Puppet::Configurer
   end
 
   # Prepare for catalog retrieval.  Downloads everything necessary, etc.
-  def prepare
+  def prepare(options)
     dostorage
 
-    download_plugins
+    download_plugins unless options[:skip_plugin_download]
 
-    download_fact_plugins
+    download_fact_plugins unless options[:skip_plugin_download]
 
     execute_prerun_command
   end
@@ -126,7 +126,7 @@ class Puppet::Configurer
   # which accepts :tags and :ignoreschedules.
   def run(options = {})
     begin
-      prepare
+      prepare(options)
     rescue SystemExit,NoMemoryError
       raise
     rescue Exception => detail
diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb
index 877c47bcc..4e1744206 100755
--- a/spec/unit/application/apply_spec.rb
+++ b/spec/unit/application/apply_spec.rb
@@ -4,6 +4,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/application/apply'
 require 'puppet/file_bucket/dipper'
+require 'puppet/configurer'
 
 describe Puppet::Application::Apply do
   before :each do
@@ -194,6 +195,9 @@ describe Puppet::Application::Apply do
         @catalog.stubs(:apply).returns(@transaction)
 
         @apply.stubs(:exit)
+
+        Puppet::Util::Storage.stubs(:load)
+        Puppet::Configurer.any_instance.stubs(:save_last_run_summary) # to prevent it from trying to write files
       end
 
       it "should set the code to run from --code" do
@@ -302,11 +306,8 @@ describe Puppet::Application::Apply do
       end
 
       it "should call the prerun and postrun commands on a Configurer instance" do
-        configurer = stub 'configurer'
-
-        Puppet::Configurer.expects(:new).returns configurer
-        configurer.expects(:execute_prerun_command)
-        configurer.expects(:execute_postrun_command)
+        Puppet::Configurer.any_instance.expects(:execute_prerun_command)
+        Puppet::Configurer.any_instance.expects(:execute_postrun_command)
 
         @apply.main
       end
@@ -321,8 +322,7 @@ describe Puppet::Application::Apply do
         it "should exit with report's computed exit status" do
           Puppet.stubs(:[]).with(:noop).returns(false)
           @apply.options.stubs(:[]).with(:detailed_exitcodes).returns(true)
-          report = stub 'report', :exit_status => 666
-          @transaction.stubs(:report).returns(report)
+          Puppet::Transaction::Report.any_instance.stubs(:exit_status).returns(666)
           @apply.expects(:exit).with(666)
 
           @apply.main
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index ebc5768ea..72754f398 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -472,23 +472,23 @@ describe Puppet::Configurer, "when preparing for a run" do
   it "should initialize the metadata store" do
     @agent.class.stubs(:facts).returns(@facts)
     @agent.expects(:dostorage)
-    @agent.prepare
+    @agent.prepare({})
   end
 
   it "should download fact plugins" do
     @agent.expects(:download_fact_plugins)
 
-    @agent.prepare
+    @agent.prepare({})
   end
 
   it "should download plugins" do
     @agent.expects(:download_plugins)
 
-    @agent.prepare
+    @agent.prepare({})
   end
 
   it "should perform the pre-run commands" do
     @agent.expects(:execute_prerun_command)
-    @agent.prepare
+    @agent.prepare({})
   end
 end
-- 
cgit 


From d516f6385e0ee044603c403ecc6f96606730f8f2 Mon Sep 17 00:00:00 2001
From: Nick Lewis <nick@puppetlabs.com>
Date: Thu, 16 Dec 2010 16:17:17 -0800
Subject: (#5493) Add report_format, puppet_version, and configuration_version
 to Reports

Current report formats are:
  0: 0.25 reports and earlier
  1: 0.26.1 - 0.26.4 reports
  2: 0.26.5 and beyond

Paired-With: Jesse Wolfe
---
 lib/puppet/application/inspect.rb               |  2 +
 lib/puppet/configurer.rb                        |  8 ++--
 lib/puppet/transaction.rb                       |  2 +-
 lib/puppet/transaction/report.rb                |  6 ++-
 spec/integration/indirector/report/rest_spec.rb |  2 +-
 spec/integration/transaction/report_spec.rb     |  2 +-
 spec/unit/configurer_spec.rb                    | 50 +++++++++++--------------
 spec/unit/reports/http_spec.rb                  |  2 +-
 spec/unit/reports/tagmail_spec.rb               |  2 +-
 spec/unit/transaction/report_spec.rb            | 36 ++++++++++--------
 spec/unit/transaction_spec.rb                   |  2 +-
 test/lib/puppettest/reporttesting.rb            |  2 +-
 test/other/report.rb                            |  2 +-
 13 files changed, 60 insertions(+), 58 deletions(-)

diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb
index c28fef326..caa32a7c2 100644
--- a/lib/puppet/application/inspect.rb
+++ b/lib/puppet/application/inspect.rb
@@ -49,6 +49,8 @@ class Puppet::Application::Inspect < Puppet::Application
       raise "Could not find catalog for #{Puppet[:certname]}"
     end
 
+    @report.configuration_version = catalog.version
+
     retrieval_time =  Time.now - retrieval_starttime
     @report.add_times("config_retrieval", retrieval_time)
 
diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index 2687f5fbd..070176554 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -72,10 +72,6 @@ class Puppet::Configurer
     @splayed = false
   end
 
-  def initialize_report
-    Puppet::Transaction::Report.new
-  end
-
   # Prepare for catalog retrieval.  Downloads everything necessary, etc.
   def prepare(options)
     dostorage
@@ -134,7 +130,7 @@ class Puppet::Configurer
       Puppet.err "Failed to prepare catalog: #{detail}"
     end
 
-    options[:report] ||= initialize_report
+    options[:report] ||= Puppet::Transaction::Report.new("apply")
     report = options[:report]
     Puppet::Util::Log.newdestination(report)
 
@@ -145,6 +141,8 @@ class Puppet::Configurer
       return
     end
 
+    report.configuration_version = catalog.version
+
     transaction = nil
 
     begin
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index dcd9aad0a..2d49062dd 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -238,7 +238,7 @@ class Puppet::Transaction
   def initialize(catalog)
     @catalog = catalog
 
-    @report = Report.new
+    @report = Report.new("apply", catalog.version)
 
     @event_manager = Puppet::Transaction::EventManager.new(self)
 
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 75c08fc7a..8a928454f 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -10,6 +10,7 @@ class Puppet::Transaction::Report
 
   indirects :report, :terminus_class => :processor
 
+  attr_accessor :configuration_version
   attr_reader :resource_statuses, :logs, :metrics, :host, :time, :kind
 
   # This is necessary since Marshall doesn't know how to
@@ -49,7 +50,7 @@ class Puppet::Transaction::Report
     calculate_event_metrics
   end
 
-  def initialize(kind = "apply")
+  def initialize(kind, configuration_version=nil)
     @metrics = {}
     @logs = []
     @resource_statuses = {}
@@ -57,6 +58,9 @@ class Puppet::Transaction::Report
     @host = Puppet[:certname]
     @time = Time.now
     @kind = kind
+    @report_format = 2
+    @puppet_version = Puppet.version
+    @configuration_version = configuration_version
   end
 
   def name
diff --git a/spec/integration/indirector/report/rest_spec.rb b/spec/integration/indirector/report/rest_spec.rb
index fdc218975..7fa026b73 100644
--- a/spec/integration/indirector/report/rest_spec.rb
+++ b/spec/integration/indirector/report/rest_spec.rb
@@ -64,7 +64,7 @@ describe "Report REST Terminus" do
   it "should be able to send a report to the server" do
     @report.expects(:save)
 
-    report = Puppet::Transaction::Report.new
+    report = Puppet::Transaction::Report.new("apply")
 
     resourcemetrics = {
       :total => 12,
diff --git a/spec/integration/transaction/report_spec.rb b/spec/integration/transaction/report_spec.rb
index eed7acaa9..e7d952eb2 100755
--- a/spec/integration/transaction/report_spec.rb
+++ b/spec/integration/transaction/report_spec.rb
@@ -19,7 +19,7 @@ describe Puppet::Transaction::Report do
 
       Facter.stubs(:value).returns "host.domain.com"
 
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
 
       terminus.expects(:process).with(report)
 
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index 72754f398..3b2a44f0f 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -72,14 +72,6 @@ describe Puppet::Configurer do
   end
 end
 
-describe Puppet::Configurer, "when initializing a report" do
-  it "should return an instance of a transaction report" do
-    Puppet.settings.stubs(:use).returns(true)
-    @agent = Puppet::Configurer.new
-    @agent.initialize_report.should be_instance_of(Puppet::Transaction::Report)
-  end
-end
-
 describe Puppet::Configurer, "when executing a catalog run" do
   before do
     Puppet.settings.stubs(:use).returns(true)
@@ -101,31 +93,31 @@ describe Puppet::Configurer, "when executing a catalog run" do
   end
 
   it "should initialize a transaction report if one is not provided" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns report
 
     @agent.run
   end
 
   it "should pass the new report to the catalog" do
-    report = stub 'report'
-    @agent.stubs(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.stubs(:new).returns report
     @catalog.expects(:apply).with{|options| options[:report] == report}
 
     @agent.run
   end
 
   it "should use the provided report if it was passed one" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).never
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).never
     @catalog.expects(:apply).with{|options| options[:report] == report}
 
     @agent.run(:report => report)
   end
 
   it "should set the report as a log destination" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns report
 
     Puppet::Util::Log.expects(:newdestination).with(report)
 
@@ -176,16 +168,16 @@ describe Puppet::Configurer, "when executing a catalog run" do
   end
 
   it "should send the report" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
     @agent.expects(:send_report).with { |r, trans| r == report }
 
     @agent.run
   end
 
   it "should send the transaction report with a reference to the transaction if a run was actually made" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
 
     trans = stub 'transaction'
     @catalog.expects(:apply).returns trans
@@ -198,8 +190,8 @@ describe Puppet::Configurer, "when executing a catalog run" do
   it "should send the transaction report even if the catalog could not be retrieved" do
     @agent.expects(:retrieve_catalog).returns nil
 
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
     @agent.expects(:send_report)
 
     @agent.run
@@ -208,16 +200,16 @@ describe Puppet::Configurer, "when executing a catalog run" do
   it "should send the transaction report even if there is a failure" do
     @agent.expects(:retrieve_catalog).raises "whatever"
 
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
     @agent.expects(:send_report)
 
     lambda { @agent.run }.should raise_error
   end
 
   it "should remove the report as a log destination when the run is finished" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
 
     Puppet::Util::Log.expects(:close).with(report)
 
@@ -225,8 +217,8 @@ describe Puppet::Configurer, "when executing a catalog run" do
   end
 
   it "should return the report as the result of the run" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
 
     @agent.run.should equal(report)
   end
@@ -237,7 +229,7 @@ describe Puppet::Configurer, "when sending a report" do
     Puppet.settings.stubs(:use).returns(true)
     @configurer = Puppet::Configurer.new
 
-    @report = stub 'report'
+    @report = Puppet::Transaction::Report.new("apply")
     @trans = stub 'transaction'
   end
 
diff --git a/spec/unit/reports/http_spec.rb b/spec/unit/reports/http_spec.rb
index c814975df..70742f7dc 100644
--- a/spec/unit/reports/http_spec.rb
+++ b/spec/unit/reports/http_spec.rb
@@ -18,7 +18,7 @@ processor = Puppet::Reports.report(:http)
 
 describe processor do
   before  { Net::HTTP.any_instance.stubs(:start).yields(FakeHTTP) }
-  subject { Puppet::Transaction::Report.new.extend(processor) }
+  subject { Puppet::Transaction::Report.new("apply").extend(processor) }
 
   it { should respond_to(:process) }
 
diff --git a/spec/unit/reports/tagmail_spec.rb b/spec/unit/reports/tagmail_spec.rb
index bdb16600e..1dadfc7cd 100755
--- a/spec/unit/reports/tagmail_spec.rb
+++ b/spec/unit/reports/tagmail_spec.rb
@@ -11,7 +11,7 @@ describe tagmail do
   extend PuppetTest::Support::Utils
 
   before do
-    @processor = Puppet::Transaction::Report.new
+    @processor = Puppet::Transaction::Report.new("apply")
     @processor.extend(Puppet::Reports.report(:tagmail))
   end
 
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 77f82159b..604c2f54d 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
@@ -72,7 +78,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)
       report.save
     end
@@ -82,7 +88,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
@@ -94,21 +100,21 @@ 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 = 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 = 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 = Puppet::Transaction::Report.new("apply")
       report.add_metric("changes", {:total => 1})
       report.add_metric("resources", {:failed => 1})
       report.exit_status.should == 6
@@ -117,7 +123,7 @@ describe Puppet::Transaction::Report do
 
   describe "when calculating metrics" do
     before do
-      @report = Puppet::Transaction::Report.new
+      @report = Puppet::Transaction::Report.new("apply")
     end
 
     def metric(name, value)
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 2df4404be..566c90438 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -94,7 +94,7 @@ describe Puppet::Transaction do
   end
 
   it "should be possible to replace the report object" do
-    report = Puppet::Transaction::Report.new
+    report = Puppet::Transaction::Report.new("apply")
     @transaction.report = report
 
     @transaction.report.should == report
diff --git a/test/lib/puppettest/reporttesting.rb b/test/lib/puppettest/reporttesting.rb
index 448a6a9d8..54a2f6799 100644
--- a/test/lib/puppettest/reporttesting.rb
+++ b/test/lib/puppettest/reporttesting.rb
@@ -1,7 +1,7 @@
 module PuppetTest::Reporttesting
   def fakereport
     # Create a bunch of log messages in an array.
-    report = Puppet::Transaction::Report.new
+    report = Puppet::Transaction::Report.new("apply")
 
     3.times { |i|
       # We have to use warning so that the logs always happen
diff --git a/test/other/report.rb b/test/other/report.rb
index 8a909b41c..eacf1632b 100755
--- a/test/other/report.rb
+++ b/test/other/report.rb
@@ -68,7 +68,7 @@ class TestReports < Test::Unit::TestCase
 
   def test_store_report
     # Create a bunch of log messages in an array.
-    report = Puppet::Transaction::Report.new
+    report = Puppet::Transaction::Report.new("apply")
 
     # We have to reuse reporting here because of something going on in the
     # server/report.rb file
-- 
cgit 


From 7fff7808e25491a5ea1e207b8de3ade0c4f95f4f Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Fri, 17 Dec 2010 10:59:12 -0800
Subject: (#5408) Reworked ResourceHarness so that code is clearer and all
 behaviors are tested

This patch removes the Puppet::Transaction::Change class and replaces
it with a method,
Puppet::Transaction::ResourceHarness#apply_parameter.  The new code is
shorter, more thoroughly unit tested, and addresses known bugs in
the interaction between auditing and performing changes.

This code does not address drawbacks in the report output (for example
a resource is still flagged as changed even if it merely contains
audit information); those will be addressed in a follow-up patch.
---
 lib/puppet/resource/status.rb                  |   8 +-
 lib/puppet/transaction.rb                      |   1 -
 lib/puppet/transaction/change.rb               |  75 ----
 lib/puppet/transaction/resource_harness.rb     | 143 ++++---
 spec/unit/resource/status_spec.rb              |  17 +-
 spec/unit/transaction/change_spec.rb           | 206 -----------
 spec/unit/transaction/report_spec.rb           |   2 +-
 spec/unit/transaction/resource_harness_spec.rb | 492 ++++++++-----------------
 8 files changed, 264 insertions(+), 680 deletions(-)
 delete mode 100644 lib/puppet/transaction/change.rb
 delete mode 100755 spec/unit/transaction/change_spec.rb

diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb
index 2bdfbbfef..9240a06d7 100644
--- a/lib/puppet/resource/status.rb
+++ b/lib/puppet/resource/status.rb
@@ -4,13 +4,13 @@ module Puppet
       include Puppet::Util::Tagging
       include Puppet::Util::Logging
 
-      ATTRIBUTES = [:resource, :node, :version, :file, :line, :current_values, :skipped_reason, :status, :evaluation_time, :change_count]
-      attr_accessor *ATTRIBUTES
+      attr_accessor :resource, :node, :version, :file, :line, :current_values, :skipped_reason, :status, :evaluation_time
 
       STATES = [:skipped, :failed, :failed_to_restart, :restarted, :changed, :out_of_sync, :scheduled]
       attr_accessor *STATES
 
       attr_reader :source_description, :default_log_level, :time, :resource
+      attr_reader :change_count
 
       # Provide a boolean method for each of the states.
       STATES.each do |attr|
@@ -29,6 +29,9 @@ module Puppet
         if event.status == 'failure'
           self.failed = true
         end
+        @change_count += 1
+        @changed = true
+        @out_of_sync = true
       end
 
       def events
@@ -38,6 +41,7 @@ module Puppet
       def initialize(resource)
         @source_description = resource.path
         @resource = resource.to_s
+        @change_count = 0
 
         [:file, :line, :version].each do |attr|
           send(attr.to_s + "=", resource.send(attr))
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 2d49062dd..4db971477 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -6,7 +6,6 @@ require 'puppet/util/tagging'
 require 'puppet/application'
 
 class Puppet::Transaction
-  require 'puppet/transaction/change'
   require 'puppet/transaction/event'
   require 'puppet/transaction/event_manager'
   require 'puppet/transaction/resource_harness'
diff --git a/lib/puppet/transaction/change.rb b/lib/puppet/transaction/change.rb
deleted file mode 100644
index d57ac1917..000000000
--- a/lib/puppet/transaction/change.rb
+++ /dev/null
@@ -1,75 +0,0 @@
-require 'puppet/transaction'
-require 'puppet/transaction/event'
-
-# Handle all of the work around performing an actual change,
-# including calling 'sync' on the properties and producing events.
-class Puppet::Transaction::Change
-  attr_accessor :is, :should, :property, :proxy, :auditing, :old_audit_value
-
-  def auditing?
-    auditing
-  end
-
-  def initialize(property, currentvalue)
-    @property = property
-    @is = currentvalue
-
-    @should = property.should
-
-    @changed = false
-  end
-
-  def apply
-    event = property.event
-    event.previous_value = is
-    event.desired_value = should
-    event.historical_value = old_audit_value
-
-    if auditing? and old_audit_value != is
-      event.message = "audit change: previously recorded value #{property.is_to_s(old_audit_value)} has been changed to #{property.is_to_s(is)}"
-      event.status = "audit"
-      event.audited = true
-      brief_audit_message = " (previously recorded value was #{property.is_to_s(old_audit_value)})" 
-    else
-      brief_audit_message = "" 
-    end
-
-    if property.insync?(is)
-      # nothing happens
-    elsif noop?
-      event.message = "is #{property.is_to_s(is)}, should be #{property.should_to_s(should)} (noop)#{brief_audit_message}"
-      event.status = "noop"
-    else
-      property.sync
-      event.message = [ property.change_to_s(is, should), brief_audit_message ].join
-      event.status = "success"
-    end
-    event
-  rescue => detail
-    puts detail.backtrace if Puppet[:trace]
-    event.status = "failure"
-
-    event.message = "change from #{property.is_to_s(is)} to #{property.should_to_s(should)} failed: #{detail}"
-    event
-  ensure
-    event.send_log
-  end
-
-  # Is our property noop?  This is used for generating special events.
-  def noop?
-    @property.noop
-  end
-
-  # The resource that generated this change.  This is used for handling events,
-  # and the proxy resource is used for generated resources, since we can't
-  # send an event to a resource we don't have a direct relationship with.  If we
-  # have a proxy resource, then the events will be considered to be from
-  # that resource, rather than us, so the graph resolution will still work.
-  def resource
-    self.proxy || @property.resource
-  end
-
-  def to_s
-    "change #{@property.change_to_s(@is, @should)}"
-  end
-end
diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb
index c978e5545..cb9a193b9 100644
--- a/lib/puppet/transaction/resource_harness.rb
+++ b/lib/puppet/transaction/resource_harness.rb
@@ -7,22 +7,15 @@ class Puppet::Transaction::ResourceHarness
   attr_reader :transaction
 
   def allow_changes?(resource)
-    return true unless resource.purging? and resource.deleting?
-    return true unless deps = relationship_graph.dependents(resource) and ! deps.empty? and deps.detect { |d| ! d.deleting? }
-
-    deplabel = deps.collect { |r| r.ref }.join(",")
-    plurality = deps.length > 1 ? "":"s"
-    resource.warning "#{deplabel} still depend#{plurality} on me -- not purging"
-    false
-  end
-
-  def apply_changes(status, changes)
-    changes.each do |change|
-      status << change.apply
-
-      cache(change.property.resource, change.property.name, change.is) if change.auditing?
+    if resource.purging? and resource.deleting? and deps = relationship_graph.dependents(resource) \
+            and ! deps.empty? and deps.detect { |d| ! d.deleting? }
+      deplabel = deps.collect { |r| r.ref }.join(",")
+      plurality = deps.length > 1 ? "":"s"
+      resource.warning "#{deplabel} still depend#{plurality} on me -- not purging"
+      false
+    else
+      true
     end
-    status.changed = true
   end
 
   # Used mostly for scheduling and auditing at this point.
@@ -35,66 +28,112 @@ class Puppet::Transaction::ResourceHarness
     Puppet::Util::Storage.cache(resource)[name] = value
   end
 
-  def changes_to_perform(status, resource)
+  def perform_changes(resource)
     current = resource.retrieve_resource
 
     cache resource, :checked, Time.now
 
     return [] if ! allow_changes?(resource)
 
-    audited = copy_audited_parameters(resource, current)
+    current_values = current.to_hash
+    historical_values = Puppet::Util::Storage.cache(resource).dup
+    desired_values = resource.to_resource.to_hash
+    audited_params = (resource[:audit] || []).map { |p| p.to_sym }
+    synced_params = []
 
-    if param = resource.parameter(:ensure)
-      return [] if absent_and_not_being_created?(current, param)
-      unless ensure_is_insync?(current, param)
-        audited.keys.reject{|name| name == :ensure}.each do |name|
-          resource.parameter(name).notice "audit change: previously recorded value #{audited[name]} has been changed to #{current[param]}"
-          cache(resource, name, current[param])
+    # Record the current state in state.yml.
+    audited_params.each do |param|
+      cache(resource, param, current_values[param])
+    end
+
+    # Update the machine state & create logs/events
+    events = []
+    ensure_param = resource.parameter(:ensure)
+    if desired_values[:ensure] && !ensure_param.insync?(current_values[:ensure])
+      events << apply_parameter(ensure_param, current_values[:ensure], audited_params.include?(:ensure), historical_values[:ensure])
+      synced_params << :ensure
+    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])
+          events << apply_parameter(param, current_values[param.name], audited_params.include?(param.name), historical_values[param.name])
+          synced_params << param.name
         end
-        return [Puppet::Transaction::Change.new(param, current[:ensure])]
       end
-      return [] if ensure_should_be_absent?(current, param)
     end
 
-    resource.properties.reject { |param| param.name == :ensure }.select do |param|
-      (audited.include?(param.name) && audited[param.name] != current[param.name]) || (param.should != nil && !param_is_insync?(current, param))
-    end.collect do |param|
-      change = Puppet::Transaction::Change.new(param, current[param.name])
-      change.auditing = true if audited.include?(param.name)
-      change.old_audit_value = audited[param.name]
-      change
+    # Add more events to capture audit results
+    audited_params.each do |param_name|
+      if historical_values.include?(param_name)
+        if historical_values[param_name] != current_values[param_name] && !synced_params.include?(param_name)
+          event = create_change_event(resource.parameter(param_name), current_values[param_name], true, historical_values[param_name])
+          event.send_log
+          events << event
+        end
+      else
+        resource.property(param_name).notice "audit change: newly-recorded value #{current_values[param_name]}"
+      end
     end
+
+    events
   end
 
-  def copy_audited_parameters(resource, current)
-    return {} unless audit = resource[:audit]
-    audit = Array(audit).collect { |p| p.to_sym }
-    audited = {}
-    audit.find_all do |param|
-      if value = cached(resource, param)
-        audited[param] = value
-      else
-        resource.property(param).notice "audit change: newly-recorded recorded value #{current[param]}"
-        cache(resource, param, current[param])
-      end
+  def create_change_event(property, current_value, do_audit, historical_value)
+    event = property.event
+    event.previous_value = current_value
+    event.desired_value = property.should
+    event.historical_value = historical_value
+
+    if do_audit and historical_value != current_value
+      event.message = "audit change: previously recorded value #{property.is_to_s(historical_value)} has been changed to #{property.is_to_s(current_value)}"
+      event.status = "audit"
+      event.audited = true
     end
 
-    audited
+    event
+  end
+
+  def apply_parameter(property, current_value, do_audit, historical_value)
+    event = create_change_event(property, current_value, do_audit, historical_value)
+
+    if event.audited && historical_value
+      brief_audit_message = " (previously recorded value was #{property.is_to_s(historical_value)})"
+    else
+      brief_audit_message = ""
+    end
+
+    if property.noop
+      event.message = "current_value #{property.is_to_s(current_value)}, should be #{property.should_to_s(property.should)} (noop)#{brief_audit_message}"
+      event.status = "noop"
+    else
+      property.sync
+      event.message = [ property.change_to_s(current_value, property.should), brief_audit_message ].join
+      event.status = "success"
+    end
+    event
+  rescue => detail
+    puts detail.backtrace if Puppet[:trace]
+    event.status = "failure"
+
+    event.message = "change from #{property.is_to_s(current_value)} to #{property.should_to_s(property.should)} failed: #{detail}"
+    event
+  ensure
+    event.send_log
   end
 
   def evaluate(resource)
     start = Time.now
     status = Puppet::Resource::Status.new(resource)
 
-    if changes = changes_to_perform(status, resource) and ! changes.empty?
-      status.out_of_sync = true
-      status.change_count = changes.length
-      apply_changes(status, changes)
-      if ! resource.noop?
-        cache(resource, :synced, Time.now)
-        resource.flush if resource.respond_to?(:flush)
-      end
+    perform_changes(resource).each do |event|
+      status << event
     end
+
+    if status.changed? && ! resource.noop?
+      cache(resource, :synced, Time.now)
+      resource.flush if resource.respond_to?(:flush)
+    end
+
     return status
   rescue => detail
     resource.fail "Could not create resource status: #{detail}" unless status
diff --git a/spec/unit/resource/status_spec.rb b/spec/unit/resource/status_spec.rb
index 425015a13..7a21164c3 100755
--- a/spec/unit/resource/status_spec.rb
+++ b/spec/unit/resource/status_spec.rb
@@ -10,7 +10,7 @@ describe Puppet::Resource::Status do
     @status = Puppet::Resource::Status.new(@resource)
   end
 
-  [:node, :version, :file, :line, :current_values, :skipped_reason, :status, :evaluation_time, :change_count].each do |attr|
+  [:node, :version, :file, :line, :current_values, :skipped_reason, :status, :evaluation_time].each do |attr|
     it "should support #{attr}" do
       @status.send(attr.to_s + "=", "foo")
       @status.send(attr).should == "foo"
@@ -100,4 +100,19 @@ describe Puppet::Resource::Status do
     (@status << event).should equal(@status)
     @status.events.should == [event]
   end
+
+  it "should count the number of events and set changed" do
+    3.times{ @status << Puppet::Transaction::Event.new }
+    @status.change_count.should == 3
+
+    @status.changed.should == true
+    @status.out_of_sync.should == true
+  end
+
+  it "should not start with any changes" do
+    @status.change_count.should == 0
+
+    @status.changed.should be_false
+    @status.out_of_sync.should be_false
+  end
 end
diff --git a/spec/unit/transaction/change_spec.rb b/spec/unit/transaction/change_spec.rb
deleted file mode 100755
index fbc662df0..000000000
--- a/spec/unit/transaction/change_spec.rb
+++ /dev/null
@@ -1,206 +0,0 @@
-#!/usr/bin/env ruby
-
-require 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/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 604c2f54d..3b7c3b0bd 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| status.change_count = 3 }
+        add_statuses(3) { |status| 3.times { status << Puppet::Transaction::Event.new } }
         @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 b143c21ed..d888b05ea 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -49,354 +49,162 @@ 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 == []
-      end
-    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
+
+                      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 = []
+                      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
+                      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)"
+                        else
+                          what_happened = "mode changed '#{machine_state}' to '#{mode_property}'"
+                        end
+                        if auditing_mode && yaml_mode && yaml_mode != machine_state
+                          previously_recorded_mode_already_logged = true
+                          expected_logs << "notice: /#{resource}/mode: #{what_happened} (previously recorded value was #{yaml_mode})"
+                        else
+                          expected_logs << "notice: /#{resource}/mode: #{what_happened}"
+                        end
+                      end
+                      if @harness.cached(resource, :mode) && @harness.cached(resource, :mode) != yaml_mode
+                        if yaml_mode
+                          unless previously_recorded_mode_already_logged
+                            expected_logs << "notice: /#{resource}/mode: audit change: previously recorded value #{yaml_mode} has been changed to #{@harness.cached(resource, :mode)}"
+                          end
+                        else
+                          expected_logs << "notice: /#{resource}/mode: audit change: newly-recorded value #{@harness.cached(resource, :mode)}"
+                        end
+                      end
+
+                      # Test log output for the "ensure" parameter
+                      previously_recorded_ensure_already_logged = false
+                      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)"
+                        else
+                          what_happened = file_would_be_there_if_not_noop ? 'created' : 'removed'
+                        end
+                        if auditing_ensure && yaml_ensure && yaml_ensure != (machine_state ? :file : :absent)
+                          previously_recorded_ensure_already_logged = true
+                          expected_logs << "notice: /#{resource}/ensure: #{what_happened} (previously recorded value was #{yaml_ensure})"
+                        else
+                          expected_logs << "notice: /#{resource}/ensure: #{what_happened}"
+                        end
+                      end
+                      if @harness.cached(resource, :ensure) && @harness.cached(resource, :ensure) != yaml_ensure
+                        if yaml_ensure
+                          unless previously_recorded_ensure_already_logged
+                            expected_logs << "notice: /#{resource}/ensure: audit change: previously recorded value #{yaml_ensure} has been changed to #{@harness.cached(resource, :ensure)}"
+                          end
+                        else
+                          expected_logs << "notice: /#{resource}/ensure: audit change: newly-recorded value #{@harness.cached(resource, :ensure)}"
+                        end
+                      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 }
+
+                      # 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 the :synced field on state.yml
+                      synced_should_be_set = !noop_mode && status.changed != nil
+                      (@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
 
-- 
cgit 


From 4d3030c6dd67dcb1f6116e7e3d09ddcd20ee726b Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Thu, 16 Dec 2010 16:23:34 -0800
Subject: 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.
---
 lib/puppet/resource/status.rb                  | 13 +++++++---
 spec/unit/resource/status_spec.rb              | 24 ++++++++++++++++--
 spec/unit/transaction/report_spec.rb           |  2 +-
 spec/unit/transaction/resource_harness_spec.rb | 34 +++++++++++++++++++++++---
 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
-- 
cgit 


From d11ae78871cf9b65e15497e55c98851ca3cbfd15 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Tue, 31 Aug 2010 11:46:36 -0700
Subject: [3782] Test isolation problem in test/ral/providers/cron/crontab.rb

The test in question (test_parse_line) was nondeterministic because it
was relying on the sort order of a Hash whose keys were symbols.  When
the sort order caused a blank line to appear at the end of the file
under test, the blank line was elided by the crontab parser, causing a
failure.

Modified the test to execute in a deterministic order that doesn't
place the blank line at the end.
---
 test/ral/providers/cron/crontab.rb | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/test/ral/providers/cron/crontab.rb b/test/ral/providers/cron/crontab.rb
index 0c87a5bba..be2af1e16 100755
--- a/test/ral/providers/cron/crontab.rb
+++ b/test/ral/providers/cron/crontab.rb
@@ -97,7 +97,10 @@ class TestCronParsedProvider < Test::Unit::TestCase
     # Then do them all at once.
     records = []
     text = ""
-    sample_records.each do |name, options|
+    # Sort sample_records so that the :empty entry does not come last
+    # (if it does, the test will fail because the empty last line will
+    # be ignored)
+    sample_records.sort { |a, b| a.first.to_s <=> b.first.to_s }.each do |name, options|
       records << options[:record]
       text += options[:text] + "\n"
     end
-- 
cgit 


From 76fe2b3b24f6c04cd1cff7c8492d479d15258566 Mon Sep 17 00:00:00 2001
From: Jesse Wolfe <jes5199@gmail.com>
Date: Mon, 20 Dec 2010 12:17:52 -0800
Subject: Implement #5168 and #5169 ctime and mtime are properties

File ctime and mtime are now implemented as read-only properties, so
they can be examined with audit.
---
 lib/puppet/type/file.rb       |  2 ++
 lib/puppet/type/file/ctime.rb | 18 ++++++++++++++++++
 lib/puppet/type/file/mtime.rb | 17 +++++++++++++++++
 lib/puppet/type/file/type.rb  | 17 +++++------------
 spec/unit/type/file/ctime.rb  | 35 +++++++++++++++++++++++++++++++++++
 spec/unit/type/file/mtime.rb  | 35 +++++++++++++++++++++++++++++++++++
 spec/unit/type/file/type.rb   | 20 ++++++++++++++++++++
 7 files changed, 132 insertions(+), 12 deletions(-)
 create mode 100644 lib/puppet/type/file/ctime.rb
 create mode 100644 lib/puppet/type/file/mtime.rb
 create mode 100644 spec/unit/type/file/ctime.rb
 create mode 100644 spec/unit/type/file/mtime.rb
 create mode 100644 spec/unit/type/file/type.rb

diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 6523c99a0..eee948cd5 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -797,3 +797,5 @@ require 'puppet/type/file/group'
 require 'puppet/type/file/mode'
 require 'puppet/type/file/type'
 require 'puppet/type/file/selcontext'  # SELinux file context
+require 'puppet/type/file/ctime'
+require 'puppet/type/file/mtime'
diff --git a/lib/puppet/type/file/ctime.rb b/lib/puppet/type/file/ctime.rb
new file mode 100644
index 000000000..24b098703
--- /dev/null
+++ b/lib/puppet/type/file/ctime.rb
@@ -0,0 +1,18 @@
+module Puppet
+  Puppet::Type.type(:file).newproperty(:ctime) do
+    desc "A read-only state to check the file ctime."
+
+    def retrieve
+      current_value = :absent
+      if stat = @resource.stat(false)
+        current_value = stat.ctime
+      end
+      current_value
+    end
+
+    validate do
+      fail "ctime is read-only"
+    end
+  end
+end
+
diff --git a/lib/puppet/type/file/mtime.rb b/lib/puppet/type/file/mtime.rb
new file mode 100644
index 000000000..8ca7ed0d6
--- /dev/null
+++ b/lib/puppet/type/file/mtime.rb
@@ -0,0 +1,17 @@
+module Puppet
+  Puppet::Type.type(:file).newproperty(:mtime) do
+    desc "A read-only state to check the file mtime."
+
+    def retrieve
+      current_value = :absent
+      if stat = @resource.stat(false)
+        current_value = stat.mtime
+      end
+      current_value
+    end
+
+    validate do
+      fail "mtime is read-only"
+    end
+  end
+end
diff --git a/lib/puppet/type/file/type.rb b/lib/puppet/type/file/type.rb
index eb50b81f9..4da54e2cb 100755
--- a/lib/puppet/type/file/type.rb
+++ b/lib/puppet/type/file/type.rb
@@ -3,23 +3,16 @@ module Puppet
     require 'etc'
     desc "A read-only state to check the file type."
 
-    #munge do |value|
-    #    raise Puppet::Error, ":type is read-only"
-    #end
-
     def retrieve
-      currentvalue = :absent
+      current_value = :absent
       if stat = @resource.stat(false)
-        currentvalue = stat.ftype
+        current_value = stat.ftype
       end
-      # so this state is never marked out of sync
-      @should = [currentvalue]
-      currentvalue
+      current_value
     end
 
-
-    def sync
-      raise Puppet::Error, ":type is read-only"
+    validate do
+      fail "type is read-only"
     end
   end
 end
diff --git a/spec/unit/type/file/ctime.rb b/spec/unit/type/file/ctime.rb
new file mode 100644
index 000000000..6145cbfdc
--- /dev/null
+++ b/spec/unit/type/file/ctime.rb
@@ -0,0 +1,35 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+describe Puppet::Type.type(:file).attrclass(:ctime) do
+  require 'puppet_spec/files'
+  include PuppetSpec::Files
+
+  before do
+    @filename = tmpfile('ctime')
+    @resource = Puppet::Type.type(:file).new({:name => @filename})
+  end
+
+  it "should be able to audit the file's ctime" do
+    File.open(@filename, "w"){ }
+
+    @resource[:audit] = [:ctime]
+
+    # this .to_resource audit behavior is magical :-(
+    @resource.to_resource[:ctime].should == File.stat(@filename).ctime
+  end
+
+  it "should return absent if auditing an absent file" do
+    @resource[:audit] = [:ctime]
+
+    @resource.to_resource[:ctime].should == :absent
+  end
+
+  it "should prevent the user from trying to set the ctime" do
+    lambda {
+      @resource[:ctime] = Time.now.to_s
+    }.should raise_error(Puppet::Error, /ctime is read-only/)
+  end
+
+end
diff --git a/spec/unit/type/file/mtime.rb b/spec/unit/type/file/mtime.rb
new file mode 100644
index 000000000..043156ceb
--- /dev/null
+++ b/spec/unit/type/file/mtime.rb
@@ -0,0 +1,35 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+describe Puppet::Type.type(:file).attrclass(:mtime) do
+  require 'puppet_spec/files'
+  include PuppetSpec::Files
+
+  before do
+    @filename = tmpfile('mtime')
+    @resource = Puppet::Type.type(:file).new({:name => @filename})
+  end
+
+  it "should be able to audit the file's mtime" do
+    File.open(@filename, "w"){ }
+
+    @resource[:audit] = [:mtime]
+
+    # this .to_resource audit behavior is magical :-(
+    @resource.to_resource[:mtime].should == File.stat(@filename).mtime
+  end
+
+  it "should return absent if auditing an absent file" do
+    @resource[:audit] = [:mtime]
+
+    @resource.to_resource[:mtime].should == :absent
+  end
+
+  it "should prevent the user from trying to set the mtime" do
+    lambda {
+      @resource[:mtime] = Time.now.to_s
+    }.should raise_error(Puppet::Error, /mtime is read-only/)
+  end
+
+end
diff --git a/spec/unit/type/file/type.rb b/spec/unit/type/file/type.rb
new file mode 100644
index 000000000..e46f0e0b0
--- /dev/null
+++ b/spec/unit/type/file/type.rb
@@ -0,0 +1,20 @@
+#!/usr/bin/env ruby
+
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+describe Puppet::Type.type(:file).attrclass(:type) do
+  require 'puppet_spec/files'
+  include PuppetSpec::Files
+
+  before do
+    @filename = tmpfile('type')
+    @resource = Puppet::Type.type(:file).new({:name => @filename})
+  end
+
+  it "should prevent the user from trying to set the type" do
+    lambda {
+      @resource[:type] = "fifo"
+    }.should raise_error(Puppet::Error, /type is read-only/)
+  end
+
+end
-- 
cgit 


From 8631709d71833e68fbfe40dbee10fe876eb309ea Mon Sep 17 00:00:00 2001
From: Nick Lewis <nick@puppetlabs.com>
Date: Wed, 29 Dec 2010 17:24:58 -0800
Subject: (#5723) Fix failing type/package specs

These tests were failing because they were overly constraining what the code
could do. Because of a bug in Mocha 0.9.8, these failures weren't caught when
the code causing them was first made. These failures were introduced in
7fff7808e25491a5ea1e207b8de3ade0c4f95f4f.

Paired-With: Paul Berry
---
 spec/unit/type/package_spec.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/spec/unit/type/package_spec.rb b/spec/unit/type/package_spec.rb
index b0c5d2252..662fe4798 100755
--- a/spec/unit/type/package_spec.rb
+++ b/spec/unit/type/package_spec.rb
@@ -121,7 +121,7 @@ describe Puppet::Type.type(:package) do
       before { @package[:ensure] = :purged }
 
       it "should do nothing if it is :purged" do
-        @provider.expects(:properties).returns(:ensure => :purged)
+        @provider.expects(:properties).returns(:ensure => :purged).at_least_once
         @catalog.apply
       end
 
@@ -141,7 +141,7 @@ describe Puppet::Type.type(:package) do
 
       [:purged, :absent].each do |state|
         it "should do nothing if it is #{state.to_s}" do
-          @provider.expects(:properties).returns(:ensure => state)
+          @provider.expects(:properties).returns(:ensure => state).at_least_once
           @catalog.apply
         end
       end
@@ -162,7 +162,7 @@ describe Puppet::Type.type(:package) do
 
       [:present, :latest, "1.0"].each do |state|
         it "should do nothing if it is #{state.to_s}" do
-          @provider.expects(:properties).returns(:ensure => state)
+          @provider.expects(:properties).returns(:ensure => state).at_least_once
           @catalog.apply
         end
       end
-- 
cgit 


From b765f0e7a23af9804f1541437f2caf1d93cc5ed2 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 14:49:41 -0800
Subject: (#5715) Prep work: Fixed add_statuses in report_spec.

Report_spec contained a helper method called add_statuses which
claimed to add a status object n times.  It actually always added it 3
times, and fortunately callers always specified n=3.  Changed this to
be more sane.
---
 spec/unit/transaction/report_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 34c6ecd96..f87f103af 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -135,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
-- 
cgit 


From 0e39ec5bbd06dad0b59436a092c50902913d4a5a Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 11:18:35 -0800
Subject: (#5715) Removed Resource::Status#skipped_reason.  It was never used.

---
 lib/puppet/resource/status.rb     | 2 +-
 spec/unit/resource/status_spec.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb
index f34edc469..2bfadbd54 100644
--- a/lib/puppet/resource/status.rb
+++ b/lib/puppet/resource/status.rb
@@ -4,7 +4,7 @@ module Puppet
       include Puppet::Util::Tagging
       include Puppet::Util::Logging
 
-      attr_accessor :resource, :node, :version, :file, :line, :current_values, :skipped_reason, :status, :evaluation_time
+      attr_accessor :resource, :node, :version, :file, :line, :current_values, :status, :evaluation_time
 
       STATES = [:skipped, :failed, :failed_to_restart, :restarted, :changed, :out_of_sync, :scheduled]
       attr_accessor *STATES
diff --git a/spec/unit/resource/status_spec.rb b/spec/unit/resource/status_spec.rb
index bda8aea00..945b8f613 100755
--- a/spec/unit/resource/status_spec.rb
+++ b/spec/unit/resource/status_spec.rb
@@ -10,7 +10,7 @@ describe Puppet::Resource::Status do
     @status = Puppet::Resource::Status.new(@resource)
   end
 
-  [:node, :version, :file, :line, :current_values, :skipped_reason, :status, :evaluation_time].each do |attr|
+  [:node, :version, :file, :line, :current_values, :status, :evaluation_time].each do |attr|
     it "should support #{attr}" do
       @status.send(attr.to_s + "=", "foo")
       @status.send(attr).should == "foo"
-- 
cgit 


From 908e0e09ed94ab6b74aba3b9e4fa95318d8894ef Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Thu, 30 Dec 2010 10:45:22 -0800
Subject: (#5715) Removed the unused attribute Puppet::Transaction::Event#node

---
 lib/puppet/transaction/event.rb     | 2 +-
 spec/unit/transaction/event_spec.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/puppet/transaction/event.rb b/lib/puppet/transaction/event.rb
index da5b14727..49c2f1a2c 100644
--- a/lib/puppet/transaction/event.rb
+++ b/lib/puppet/transaction/event.rb
@@ -7,7 +7,7 @@ class Puppet::Transaction::Event
   include Puppet::Util::Tagging
   include Puppet::Util::Logging
 
-  ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :node, :version, :file, :line, :source_description, :audited]
+  ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :version, :file, :line, :source_description, :audited]
   attr_accessor *ATTRIBUTES
   attr_writer :tags
   attr_accessor :time
diff --git a/spec/unit/transaction/event_spec.rb b/spec/unit/transaction/event_spec.rb
index 9cf6bc165..0528402d8 100755
--- a/spec/unit/transaction/event_spec.rb
+++ b/spec/unit/transaction/event_spec.rb
@@ -5,7 +5,7 @@ require 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, :version, :file, :line, :tags].each do |attr|
     it "should support #{attr}" do
       event = Puppet::Transaction::Event.new
       event.send(attr.to_s + "=", "foo")
-- 
cgit 


From e596a570b2734ea27eec57a6e3a11843ccb47614 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Tue, 28 Dec 2010 17:38:57 -0800
Subject: (#5715) Removed Puppet::Util::Log#version.

This attribute was only relevant in reports, and in reports it was
redundant with Puppet::Transaction::Report#configuration_version and
Puppet::Transaction::Report#puppet_version.
---
 lib/puppet/util/log.rb              |  6 +++---
 lib/puppet/util/log_paths.rb        |  2 +-
 lib/puppet/util/logging.rb          |  2 +-
 spec/unit/parameter_spec.rb         |  3 +--
 spec/unit/resource/status_spec.rb   |  2 +-
 spec/unit/transaction/event_spec.rb |  2 +-
 spec/unit/type_spec.rb              |  2 +-
 spec/unit/util/log_spec.rb          | 31 +++++++++++++++++++------------
 spec/unit/util/logging_spec.rb      |  2 +-
 9 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
index 7764dc1d1..3fdac3f69 100644
--- a/lib/puppet/util/log.rb
+++ b/lib/puppet/util/log.rb
@@ -189,7 +189,7 @@ class Puppet::Util::Log
     @levels.include?(level)
   end
 
-  attr_accessor :time, :remote, :file, :line, :version, :source
+  attr_accessor :time, :remote, :file, :line, :source
   attr_reader :level, :message
 
   def initialize(args)
@@ -203,7 +203,7 @@ class Puppet::Util::Log
       tags.each { |t| self.tag(t) }
     end
 
-    [:file, :line, :version].each do |attr|
+    [:file, :line].each do |attr|
       next unless value = args[attr]
       send(attr.to_s + "=", value)
     end
@@ -234,7 +234,7 @@ class Puppet::Util::Log
 
       descriptors[:tags].each { |t| tag(t) }
 
-      [:file, :line, :version].each do |param|
+      [:file, :line].each do |param|
         next unless descriptors[param]
         send(param.to_s + "=", descriptors[param])
       end
diff --git a/lib/puppet/util/log_paths.rb b/lib/puppet/util/log_paths.rb
index f59197ed1..2fefd4505 100644
--- a/lib/puppet/util/log_paths.rb
+++ b/lib/puppet/util/log_paths.rb
@@ -15,7 +15,7 @@ module Puppet::Util::LogPaths
 
     descriptors[:tags] = tags
 
-    [:path, :file, :line, :version].each do |param|
+    [:path, :file, :line].each do |param|
       next unless value = send(param)
       descriptors[param] = value
     end
diff --git a/lib/puppet/util/logging.rb b/lib/puppet/util/logging.rb
index f20444a3b..bc52b17f0 100644
--- a/lib/puppet/util/logging.rb
+++ b/lib/puppet/util/logging.rb
@@ -26,7 +26,7 @@ module Puppet::Util::Logging
   end
 
   def log_metadata
-    [:file, :line, :version, :tags].inject({}) do |result, attr|
+    [:file, :line, :tags].inject({}) do |result, attr|
       result[attr] = send(attr) if respond_to?(attr)
       result
     end
diff --git a/spec/unit/parameter_spec.rb b/spec/unit/parameter_spec.rb
index 966bbfb81..f8ab05d62 100755
--- a/spec/unit/parameter_spec.rb
+++ b/spec/unit/parameter_spec.rb
@@ -52,8 +52,7 @@ describe Puppet::Parameter do
     @resource.expects(:line).returns 10
     @resource.expects(:file).returns "file"
     @resource.expects(:tags).returns %w{one two}
-    @resource.expects(:version).returns 50
-    @parameter.source_descriptors.should == {:tags=>["one", "two", "foo"], :path=>"//foo", :version=>50, :file => "file", :line => 10}
+    @parameter.source_descriptors.should == {:tags=>["one", "two", "foo"], :path=>"//foo", :file => "file", :line => 10}
   end
 
   describe "when returning the value" do
diff --git a/spec/unit/resource/status_spec.rb b/spec/unit/resource/status_spec.rb
index 945b8f613..08cb99b27 100755
--- a/spec/unit/resource/status_spec.rb
+++ b/spec/unit/resource/status_spec.rb
@@ -74,7 +74,7 @@ describe Puppet::Resource::Status do
       @status.send_log :notice, "my message"
     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" }
         @status.send(attr.to_s + "=", "my val")
diff --git a/spec/unit/transaction/event_spec.rb b/spec/unit/transaction/event_spec.rb
index 0528402d8..f1db3835b 100755
--- a/spec/unit/transaction/event_spec.rb
+++ b/spec/unit/transaction/event_spec.rb
@@ -83,7 +83,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
diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb
index 48b00ec4a..7416064a5 100755
--- a/spec/unit/type_spec.rb
+++ b/spec/unit/type_spec.rb
@@ -116,7 +116,7 @@ describe Puppet::Type do
     catalog.version = 50
     catalog.add_resource resource
 
-    resource.source_descriptors.should == {:version=>50, :tags=>["mount", "foo"], :path=>"/Mount[foo]"}
+    resource.source_descriptors.should == {:tags=>["mount", "foo"], :path=>"/Mount[foo]"}
   end
 
   it "should consider its type to be the name of its class" do
diff --git a/spec/unit/util/log_spec.rb b/spec/unit/util/log_spec.rb
index 7d96fe190..f3fd1b051 100755
--- a/spec/unit/util/log_spec.rb
+++ b/spec/unit/util/log_spec.rb
@@ -120,7 +120,7 @@ describe Puppet::Util::Log do
       Puppet::Util::Log.new(:level => "notice", :message => :foo, :source => "foo")
     end
 
-    [:file, :line, :version].each do |attr|
+    [:file, :line].each do |attr|
       it "should use #{attr} if provided" do
         Puppet::Util::Log.any_instance.expects(attr.to_s + "=").with "foo"
         Puppet::Util::Log.new(:level => "notice", :message => :foo, attr => "foo")
@@ -177,23 +177,12 @@ describe Puppet::Util::Log do
         log = Puppet::Util::Log.new(:level => "notice", :message => :foo)
         log.expects(:tag).with("tag")
         log.expects(:tag).with("tag2")
-        log.expects(:version=).with(100)
 
         log.source = source
 
         log.source.should == "path"
       end
 
-      it "should copy over any version information" do
-        catalog = Puppet::Resource::Catalog.new
-        catalog.version = 25
-        source = Puppet::Type.type(:file).new :path => "/foo/bar"
-        catalog.add_resource source
-
-        log = Puppet::Util::Log.new(:level => "notice", :message => :foo, :source => source)
-        log.version.should == 25
-      end
-
       it "should copy over any file and line information" do
         source = Puppet::Type.type(:file).new :path => "/foo/bar"
         source.file = "/my/file"
@@ -212,4 +201,22 @@ describe Puppet::Util::Log do
       end
     end
   end
+
+  describe "to_yaml" do
+    it "should not include the @version attribute" do
+      log = Puppet::Util::Log.new(:level => "notice", :message => :foo, :version => 100)
+      log.to_yaml_properties.should_not include('@version')
+    end
+
+    it "should include attributes @level, @message, @source, @tags, and @time" do
+      log = Puppet::Util::Log.new(:level => "notice", :message => :foo, :version => 100)
+      log.to_yaml_properties.should == %w{@level @message @source @tags @time}
+    end
+
+    it "should include attributes @file and @line if specified" do
+      log = Puppet::Util::Log.new(:level => "notice", :message => :foo, :file => "foo", :line => 35)
+      log.to_yaml_properties.should include('@file')
+      log.to_yaml_properties.should include('@line')
+    end
+  end
 end
diff --git a/spec/unit/util/logging_spec.rb b/spec/unit/util/logging_spec.rb
index 46ae5386f..411cd17a9 100755
--- a/spec/unit/util/logging_spec.rb
+++ b/spec/unit/util/logging_spec.rb
@@ -81,7 +81,7 @@ describe Puppet::Util::Logging do
       @logger.notice ["foo", "bar", "baz"]
     end
 
-    [:file, :line, :version, :tags].each do |attr|
+    [:file, :line, :tags].each do |attr|
       it "should include #{attr} if available" do
         @logger.singleton_class.send(:attr_accessor, attr)
 
-- 
cgit 


From 1907650515ca68e85e63c73e1a35ec30dbc81b0d Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 11:15:23 -0800
Subject: (#5715) Removed redundant attribute Resource::Status#version

---
 lib/puppet/resource/status.rb     | 4 ++--
 spec/unit/resource/status_spec.rb | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb
index 2bfadbd54..43d3f1644 100644
--- a/lib/puppet/resource/status.rb
+++ b/lib/puppet/resource/status.rb
@@ -4,7 +4,7 @@ module Puppet
       include Puppet::Util::Tagging
       include Puppet::Util::Logging
 
-      attr_accessor :resource, :node, :version, :file, :line, :current_values, :status, :evaluation_time
+      attr_accessor :resource, :node, :file, :line, :current_values, :status, :evaluation_time
 
       STATES = [:skipped, :failed, :failed_to_restart, :restarted, :changed, :out_of_sync, :scheduled]
       attr_accessor *STATES
@@ -48,7 +48,7 @@ module Puppet
         @change_count = 0
         @out_of_sync_count = 0
 
-        [:file, :line, :version].each do |attr|
+        [:file, :line].each do |attr|
           send(attr.to_s + "=", resource.send(attr))
         end
 
diff --git a/spec/unit/resource/status_spec.rb b/spec/unit/resource/status_spec.rb
index 08cb99b27..22d5fb4f9 100755
--- a/spec/unit/resource/status_spec.rb
+++ b/spec/unit/resource/status_spec.rb
@@ -10,7 +10,7 @@ describe Puppet::Resource::Status do
     @status = Puppet::Resource::Status.new(@resource)
   end
 
-  [:node, :version, :file, :line, :current_values, :status, :evaluation_time].each do |attr|
+  [:node, :file, :line, :current_values, :status, :evaluation_time].each do |attr|
     it "should support #{attr}" do
       @status.send(attr.to_s + "=", "foo")
       @status.send(attr).should == "foo"
@@ -38,7 +38,7 @@ describe Puppet::Resource::Status do
     Puppet::Resource::Status.new(@resource).source_description.should == "/my/path"
   end
 
-  [:file, :line, :version].each do |attr|
+  [:file, :line].each do |attr|
     it "should copy the resource's #{attr}" do
       @resource.expects(attr).returns "foo"
       Puppet::Resource::Status.new(@resource).send(attr).should == "foo"
-- 
cgit 


From 4cc42cda270cd1c04b22bf6e5452c550fb537ebf Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 11:20:46 -0800
Subject: (#5715) Removed redundant attribute Transaction::Event#version

---
 lib/puppet/transaction/event.rb     | 2 +-
 lib/puppet/type.rb                  | 2 +-
 spec/unit/transaction/event_spec.rb | 2 +-
 spec/unit/type_spec.rb              | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/puppet/transaction/event.rb b/lib/puppet/transaction/event.rb
index 49c2f1a2c..e3537bb08 100644
--- a/lib/puppet/transaction/event.rb
+++ b/lib/puppet/transaction/event.rb
@@ -7,7 +7,7 @@ class Puppet::Transaction::Event
   include Puppet::Util::Tagging
   include Puppet::Util::Logging
 
-  ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :version, :file, :line, :source_description, :audited]
+  ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :file, :line, :source_description, :audited]
   attr_accessor *ATTRIBUTES
   attr_writer :tags
   attr_accessor :time
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 1b6e7dcd7..ea3944b4e 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -446,7 +446,7 @@ class Type
   # Create a transaction event.  Called by Transaction or by
   # a property.
   def event(options = {})
-    Puppet::Transaction::Event.new({:resource => self, :file => file, :line => line, :tags => tags, :version => version}.merge(options))
+    Puppet::Transaction::Event.new({:resource => self, :file => file, :line => line, :tags => tags}.merge(options))
   end
 
   # Let the catalog determine whether a given cached value is
diff --git a/spec/unit/transaction/event_spec.rb b/spec/unit/transaction/event_spec.rb
index f1db3835b..4a3e6bad5 100755
--- a/spec/unit/transaction/event_spec.rb
+++ b/spec/unit/transaction/event_spec.rb
@@ -5,7 +5,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/transaction/event'
 
 describe Puppet::Transaction::Event do
-  [:previous_value, :desired_value, :property, :resource, :name, :message, :version, :file, :line, :tags].each do |attr|
+  [:previous_value, :desired_value, :property, :resource, :name, :message, :file, :line, :tags].each do |attr|
     it "should support #{attr}" do
       event = Puppet::Transaction::Event.new
       event.send(attr.to_s + "=", "foo")
diff --git a/spec/unit/type_spec.rb b/spec/unit/type_spec.rb
index 7416064a5..b7a08977e 100755
--- a/spec/unit/type_spec.rb
+++ b/spec/unit/type_spec.rb
@@ -153,7 +153,7 @@ describe Puppet::Type do
       @resource.event.default_log_level.should == :warning
     end
 
-    {:file => "/my/file", :line => 50, :tags => %{foo bar}, :version => 50}.each do |attr, value|
+    {:file => "/my/file", :line => 50, :tags => %{foo bar}}.each do |attr, value|
       it "should set the #{attr}" do
         @resource.stubs(attr).returns value
         @resource.event.send(attr).should == value
-- 
cgit 


From 1550bbb218719590431575cb4c200e620680a79d Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 11:53:35 -0800
Subject: (#5715) Added total time metric to apply reports.

This was a feature that was present in 0.25.x and was inadvertently
dropped from 2.6.x.
---
 lib/puppet/transaction/report.rb     | 2 ++
 spec/unit/transaction/report_spec.rb | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 8a928454f..bcdefd08d 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -149,6 +149,8 @@ class Puppet::Transaction::Report
       metrics[name.to_s.downcase] = value
     end
 
+    metrics["total"] = metrics.values.inject(0) { |a,b| a+b }
+
     add_metric(:time, metrics)
   end
 end
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index f87f103af..1678c0940 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -200,6 +200,15 @@ describe Puppet::Transaction::Report do
         @report.calculate_metrics
         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.calculate_metrics
+        metric(:time, "total").should == 4.25
+      end
     end
 
     describe "for events" do
-- 
cgit 


From d1bcdec38e5493e1e44192eaf07da2e88b8eb850 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 12:15:26 -0800
Subject: (#5715) Removed Puppet::Transaction::Report#external_times from YAML
 output.

This attribute was never intended to be serialized to YAML; it exists
merely as temporary storage for metrics that have not yet been placed
in the report's metrics attribute.
---
 lib/puppet/transaction/report.rb     | 4 ++++
 spec/unit/transaction/report_spec.rb | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index bcdefd08d..b40c856ac 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -101,6 +101,10 @@ class Puppet::Transaction::Report
     status
   end
 
+  def to_yaml_properties
+    (instance_variables - ["@external_times"]).sort
+  end
+
   private
 
   def calculate_change_metrics
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 1678c0940..8c4ed8afe 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -254,4 +254,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
-- 
cgit 


From 15dda94c7d1e117273928f094b46a81b3f842c1f Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 12:29:53 -0800
Subject: (#5715) Added total time to inspect reports and made inspect metrics
 more consistent.

Inspect reports now contain all the metrics that apply reports do, and
use the same code path for creating them.
---
 lib/puppet/application/inspect.rb | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb
index caa32a7c2..e6c35eaeb 100644
--- a/lib/puppet/application/inspect.rb
+++ b/lib/puppet/application/inspect.rb
@@ -51,10 +51,8 @@ class Puppet::Application::Inspect < Puppet::Application
 
     @report.configuration_version = catalog.version
 
-    retrieval_time =  Time.now - retrieval_starttime
-    @report.add_times("config_retrieval", retrieval_time)
-
-    starttime = Time.now
+    inspect_starttime = Time.now
+    @report.add_times("config_retrieval", inspect_starttime - retrieval_starttime)
 
     catalog.to_ral.resources.each do |ral_resource|
       audited_attributes = ral_resource[:audit]
@@ -70,7 +68,9 @@ class Puppet::Application::Inspect < Puppet::Application
       @report.add_resource_status(status)
     end
 
-    @report.add_metric(:time, {"config_retrieval" => retrieval_time, "inspect" => Time.now - starttime})
+    finishtime = Time.now
+    @report.add_times("inspect", finishtime - inspect_starttime)
+    @report.calculate_metrics
 
     begin
       @report.save
-- 
cgit 


From a4e40f4dd5990df4dc6b2be065e82a142a31b6fc Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 14:09:57 -0800
Subject: (#5715) Refactor in preparation for adding a status attribute to
 reports.

Renamed Puppet::Transaction::Report#calculate_metrics to
finalize_report, in preparation for adding more functionality to this
method.

Removed Puppet::Transaction#send_report and
Puppet::Transaction#generate_report.  The former was never used, and
the latter was unnecessary.
---
 lib/puppet/application/inspect.rb    |  2 +-
 lib/puppet/configurer.rb             |  4 ++--
 lib/puppet/transaction.rb            | 26 --------------------------
 lib/puppet/transaction/report.rb     |  2 +-
 spec/unit/configurer_spec.rb         | 22 +++++++---------------
 spec/unit/transaction/report_spec.rb | 20 ++++++++++----------
 spec/unit/transaction_spec.rb        |  5 -----
 7 files changed, 21 insertions(+), 60 deletions(-)

diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb
index e6c35eaeb..2f068a271 100644
--- a/lib/puppet/application/inspect.rb
+++ b/lib/puppet/application/inspect.rb
@@ -70,7 +70,7 @@ class Puppet::Application::Inspect < Puppet::Application
 
     finishtime = Time.now
     @report.add_times("inspect", finishtime - inspect_starttime)
-    @report.calculate_metrics
+    @report.finalize_report
 
     begin
       @report.save
diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index 070176554..d3c902576 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -170,8 +170,8 @@ class Puppet::Configurer
     send_report(report, transaction)
   end
 
-  def send_report(report, trans = nil)
-    trans.generate_report if trans
+  def send_report(report, trans)
+    report.finalize_report if trans
     puts report.summary if Puppet[:summarize]
     report.save if Puppet[:report]
   rescue => detail
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 4db971477..aa650eea1 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -221,12 +221,6 @@ class Puppet::Transaction
     end
   end
 
-  # Generate a transaction report.
-  def generate_report
-    @report.calculate_metrics
-    @report
-  end
-
   # Should we ignore tags?
   def ignore_tags?
     ! (@catalog.host_config? or Puppet[:name] == "puppet")
@@ -284,26 +278,6 @@ class Puppet::Transaction
     catalog.relationship_graph
   end
 
-  # Send off the transaction report.
-  def send_report
-    begin
-      report = generate_report
-    rescue => detail
-      Puppet.err "Could not generate report: #{detail}"
-      return
-    end
-
-    puts report.summary if Puppet[:summarize]
-
-    if Puppet[:report]
-      begin
-        report.save
-      rescue => detail
-        Puppet.err "Reporting failed: #{detail}"
-      end
-    end
-  end
-
   def add_resource_status(status)
     report.add_resource_status status
   end
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index b40c856ac..6eac6514b 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -43,7 +43,7 @@ class Puppet::Transaction::Report
     @resource_statuses[status.resource] = status
   end
 
-  def calculate_metrics
+  def finalize_report
     calculate_resource_metrics
     calculate_time_metrics
     calculate_change_metrics
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index 3b2a44f0f..cf73a3706 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -233,16 +233,8 @@ describe Puppet::Configurer, "when sending a report" do
     @trans = stub 'transaction'
   end
 
-  it "should require a report" do
-    lambda { @configurer.send_report }.should raise_error(ArgumentError)
-  end
-
-  it "should allow specification of a transaction" do
-    lambda { @configurer.send_report(@report, @trans)  }.should_not raise_error(ArgumentError)
-  end
-
-  it "should use any provided transaction to add metrics to the report" do
-    @trans.expects(:generate_report)
+  it "should finalize the report" do
+    @report.expects(:finalize_report)
     @configurer.send_report(@report, @trans)
   end
 
@@ -252,28 +244,28 @@ describe Puppet::Configurer, "when sending a report" do
     @report.expects(:summary).returns "stuff"
 
     @configurer.expects(:puts).with("stuff")
-    @configurer.send_report(@report)
+    @configurer.send_report(@report, nil)
   end
 
   it "should not print a report summary if not configured to do so" do
     Puppet.settings[:summarize] = false
 
     @configurer.expects(:puts).never
-    @configurer.send_report(@report)
+    @configurer.send_report(@report, nil)
   end
 
   it "should save the report if reporting is enabled" do
     Puppet.settings[:report] = true
 
     @report.expects(:save)
-    @configurer.send_report(@report)
+    @configurer.send_report(@report, nil)
   end
 
   it "should not save the report if reporting is disabled" do
     Puppet.settings[:report] = false
 
     @report.expects(:save).never
-    @configurer.send_report(@report)
+    @configurer.send_report(@report, nil)
   end
 
   it "should log but not fail if saving the report fails" do
@@ -282,7 +274,7 @@ describe Puppet::Configurer, "when sending a report" do
     @report.expects(:save).raises "whatever"
 
     Puppet.expects(:err)
-    lambda { @configurer.send_report(@report) }.should_not raise_error
+    lambda { @configurer.send_report(@report, nil) }.should_not raise_error
   end
 end
 
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 8c4ed8afe..98be28755 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -145,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
@@ -154,7 +154,7 @@ describe Puppet::Transaction::Report do
       it "should provide the total number of resources" do
         add_statuses(3)
 
-        @report.calculate_metrics
+        @report.finalize_report
         metric(:resources, :total).should == 3
       end
 
@@ -162,7 +162,7 @@ describe Puppet::Transaction::Report do
         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
+          @report.finalize_report
           metric(:resources, state).should == 3
         end
       end
@@ -171,7 +171,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(:status => 'success') } }
-        @report.calculate_metrics
+        @report.finalize_report
         metric(:changes, :total).should == 9
       end
     end
@@ -188,7 +188,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
@@ -197,7 +197,7 @@ 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
 
@@ -206,7 +206,7 @@ describe Puppet::Transaction::Report do
           status.evaluation_time = 1.25
         end
         @report.add_times :config_retrieval, 0.5
-        @report.calculate_metrics
+        @report.finalize_report
         metric(:time, "total").should == 4.25
       end
     end
@@ -216,7 +216,7 @@ describe Puppet::Transaction::Report do
         add_statuses(3) do |status|
           3.times { |i| status.add_event(Puppet::Transaction::Event.new) }
         end
-        @report.calculate_metrics
+        @report.finalize_report
         metric(:events, :total).should == 9
       end
 
@@ -230,7 +230,7 @@ describe Puppet::Transaction::Report do
             end
           end
 
-          @report.calculate_metrics
+          @report.finalize_report
           metric(:events, status_name).should == 9
         end
       end
@@ -245,7 +245,7 @@ describe Puppet::Transaction::Report do
       trans = catalog.apply
 
       @report = trans.report
-      @report.calculate_metrics
+      @report.finalize_report
     end
 
     %w{Changes Total Resources}.each do |main|
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 566c90438..862413a31 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -58,11 +58,6 @@ describe Puppet::Transaction do
     @transaction.report.resource_statuses[resource.to_s].should equal(status)
   end
 
-  it "should calculate metrics on and report the report when asked to generate a report" do
-    @transaction.report.expects(:calculate_metrics)
-    @transaction.generate_report.should equal(@transaction.report)
-  end
-
   it "should consider a resource to be failed if a status instance exists for that resource and indicates it is failed" do
     resource = Puppet::Type.type(:notify).new :name => "yayness"
     status = Puppet::Resource::Status.new(resource)
-- 
cgit 


From 71db5be0ecc5ab591e01974ba109f621348fdea0 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 14:26:47 -0800
Subject: (#5715) Made the changes/total and events/total metrics always
 present

Previously these metrics were omitted when their values were zero.
---
 lib/puppet/transaction/report.rb     | 13 +++++--------
 spec/unit/transaction/report_spec.rb | 10 ++++++++++
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 6eac6514b..16b854afc 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -46,7 +46,7 @@ class Puppet::Transaction::Report
   def finalize_report
     calculate_resource_metrics
     calculate_time_metrics
-    calculate_change_metrics
+    calculate_change_metric
     calculate_event_metrics
   end
 
@@ -107,17 +107,14 @@ class Puppet::Transaction::Report
 
   private
 
-  def calculate_change_metrics
-    metrics = Hash.new(0)
-    resource_statuses.each do |name, status|
-      metrics[:total] += status.change_count if status.change_count
-    end
-
-    add_metric(:changes, metrics)
+  def calculate_change_metric
+    total = resource_statuses.map { |name, status| status.change_count || 0 }.inject(0) { |a,b| a+b }
+    add_metric(:changes, {:total => total})
   end
 
   def calculate_event_metrics
     metrics = Hash.new(0)
+    metrics[:total] = 0
     resource_statuses.each do |name, status|
       metrics[:total] += status.events.length
       status.events.each do |event|
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 98be28755..5d270dad3 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -174,6 +174,11 @@ describe Puppet::Transaction::Report do
         @report.finalize_report
         metric(:changes, :total).should == 9
       end
+
+      it "should provide a total even if there are no changes" do
+        @report.finalize_report
+        metric(:changes, :total).should == 0
+      end
     end
 
     describe "for times" do
@@ -220,6 +225,11 @@ describe Puppet::Transaction::Report do
         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|
         it "should provide the number of #{status_name} events" do
           add_statuses(3) do |status|
-- 
cgit 


From e4a2e049dc9f77569a2dac042e9e847ef34f037d Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 14:35:58 -0800
Subject: (#5715) Made the report "calculate" methods strictly functional.

These methods previously stored the metrics as a side effect; now they
simply compute the metrics and return them.
---
 lib/puppet/transaction/report.rb | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 16b854afc..f78b5aa21 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -44,10 +44,10 @@ class Puppet::Transaction::Report
   end
 
   def finalize_report
-    calculate_resource_metrics
-    calculate_time_metrics
-    calculate_change_metric
-    calculate_event_metrics
+    add_metric(:resources, calculate_resource_metrics)
+    add_metric(:time, calculate_time_metrics)
+    add_metric(:changes, {:total => calculate_change_metric})
+    add_metric(:events, calculate_event_metrics)
   end
 
   def initialize(kind, configuration_version=nil)
@@ -108,8 +108,7 @@ class Puppet::Transaction::Report
   private
 
   def calculate_change_metric
-    total = resource_statuses.map { |name, status| status.change_count || 0 }.inject(0) { |a,b| a+b }
-    add_metric(:changes, {:total => total})
+    resource_statuses.map { |name, status| status.change_count || 0 }.inject(0) { |a,b| a+b }
   end
 
   def calculate_event_metrics
@@ -122,7 +121,7 @@ class Puppet::Transaction::Report
       end
     end
 
-    add_metric(:events, metrics)
+    metrics
   end
 
   def calculate_resource_metrics
@@ -136,7 +135,7 @@ class Puppet::Transaction::Report
       end
     end
 
-    add_metric(:resources, metrics)
+    metrics
   end
 
   def calculate_time_metrics
@@ -152,6 +151,6 @@ class Puppet::Transaction::Report
 
     metrics["total"] = metrics.values.inject(0) { |a,b| a+b }
 
-    add_metric(:time, metrics)
+    metrics
   end
 end
-- 
cgit 


From 037eac4383ed5a5e9cdde765b607a180209bad1e Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 14:49:16 -0800
Subject: (#5715) Add status attribute to reports.

---
 lib/puppet/transaction/report.rb     | 19 ++++++++++++++++---
 spec/unit/transaction/report_spec.rb | 21 ++++++++++++++++++---
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index f78b5aa21..6315973ba 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -11,7 +11,7 @@ class Puppet::Transaction::Report
   indirects :report, :terminus_class => :processor
 
   attr_accessor :configuration_version
-  attr_reader :resource_statuses, :logs, :metrics, :host, :time, :kind
+  attr_reader :resource_statuses, :logs, :metrics, :host, :time, :kind, :status
 
   # This is necessary since Marshall doesn't know how to
   # dump hash with default proc (see below @records)
@@ -43,11 +43,23 @@ class Puppet::Transaction::Report
     @resource_statuses[status.resource] = status
   end
 
+  def compute_status(resource_metrics, change_metric)
+    if (resource_metrics[:failed] || 0) > 0
+      'failed'
+    elsif change_metric > 0
+      'changed'
+    else
+      'unchanged'
+    end
+  end
+
   def finalize_report
-    add_metric(:resources, calculate_resource_metrics)
+    resource_metrics = add_metric(:resources, calculate_resource_metrics)
     add_metric(:time, calculate_time_metrics)
-    add_metric(:changes, {:total => calculate_change_metric})
+    change_metric = calculate_change_metric
+    add_metric(:changes, {:total => change_metric})
     add_metric(:events, calculate_event_metrics)
+    @status = compute_status(resource_metrics, change_metric)
   end
 
   def initialize(kind, configuration_version=nil)
@@ -61,6 +73,7 @@ class Puppet::Transaction::Report
     @report_format = 2
     @puppet_version = Puppet.version
     @configuration_version = configuration_version
+    @status = 'failed' # assume failed until the report is finalized
   end
 
   def name
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 5d270dad3..96d464b7a 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -121,7 +121,14 @@ describe Puppet::Transaction::Report do
     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("apply")
     end
@@ -166,18 +173,26 @@ describe Puppet::Transaction::Report do
           metric(:resources, state).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
+      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" do
+      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
 
-- 
cgit 


From 716ee1cd76a2b30c10e715bca3e22896d9c4e36f Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Wed, 29 Dec 2010 15:33:14 -0800
Subject: (#5715) Changed the type of metric names to always be strings.

---
 lib/puppet/transaction/report.rb                | 16 +++++++-------
 lib/puppet/util/metric.rb                       |  1 +
 spec/integration/indirector/report/rest_spec.rb | 28 +++++++++++--------------
 spec/unit/transaction/report_spec.rb            | 26 +++++++++++------------
 spec/unit/util/metric_spec.rb                   | 14 ++++++-------
 5 files changed, 41 insertions(+), 44 deletions(-)

diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 6315973ba..8e04759ad 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -44,7 +44,7 @@ class Puppet::Transaction::Report
   end
 
   def compute_status(resource_metrics, change_metric)
-    if (resource_metrics[:failed] || 0) > 0
+    if (resource_metrics["failed"] || 0) > 0
       'failed'
     elsif change_metric > 0
       'changed'
@@ -57,7 +57,7 @@ class Puppet::Transaction::Report
     resource_metrics = add_metric(:resources, calculate_resource_metrics)
     add_metric(:time, calculate_time_metrics)
     change_metric = calculate_change_metric
-    add_metric(:changes, {:total => change_metric})
+    add_metric(:changes, {"total" => change_metric})
     add_metric(:events, calculate_event_metrics)
     @status = compute_status(resource_metrics, change_metric)
   end
@@ -109,8 +109,8 @@ class Puppet::Transaction::Report
   # individual bits represent the presence of different metrics.
   def exit_status
     status = 0
-    status |= 2 if @metrics["changes"][:total] > 0
-    status |= 4 if @metrics["resources"][:failed] > 0
+    status |= 2 if @metrics["changes"]["total"] > 0
+    status |= 4 if @metrics["resources"]["failed"] > 0
     status
   end
 
@@ -126,9 +126,9 @@ class Puppet::Transaction::Report
 
   def calculate_event_metrics
     metrics = Hash.new(0)
-    metrics[:total] = 0
+    metrics["total"] = 0
     resource_statuses.each do |name, status|
-      metrics[:total] += status.events.length
+      metrics["total"] += status.events.length
       status.events.each do |event|
         metrics[event.status] += 1
       end
@@ -139,12 +139,12 @@ class Puppet::Transaction::Report
 
   def calculate_resource_metrics
     metrics = Hash.new(0)
-    metrics[:total] = resource_statuses.length
+    metrics["total"] = resource_statuses.length
 
     resource_statuses.each do |name, status|
 
       Puppet::Resource::Status::STATES.each do |state|
-        metrics[state] += 1 if status.send(state)
+        metrics[state.to_s] += 1 if status.send(state)
       end
     end
 
diff --git a/lib/puppet/util/metric.rb b/lib/puppet/util/metric.rb
index 8f55e7b44..09bbb6137 100644
--- a/lib/puppet/util/metric.rb
+++ b/lib/puppet/util/metric.rb
@@ -132,6 +132,7 @@ class Puppet::Util::Metric
   end
 
   def newvalue(name,value,label = nil)
+    raise ArgumentError.new("metric name #{name.inspect} is not a string") unless name.is_a? String
     label ||= labelize(name)
     @values.push [name,label,value]
   end
diff --git a/spec/integration/indirector/report/rest_spec.rb b/spec/integration/indirector/report/rest_spec.rb
index 7fa026b73..5b5b2ddd8 100644
--- a/spec/integration/indirector/report/rest_spec.rb
+++ b/spec/integration/indirector/report/rest_spec.rb
@@ -67,30 +67,26 @@ describe "Report REST Terminus" do
     report = Puppet::Transaction::Report.new("apply")
 
     resourcemetrics = {
-      :total => 12,
-      :out_of_sync => 20,
-      :applied => 45,
-      :skipped => 1,
-      :restarted => 23,
-      :failed_restarts => 1,
-      :scheduled => 10
+      "total" => 12,
+      "out_of_sync" => 20,
+      "applied" => 45,
+      "skipped" => 1,
+      "restarted" => 23,
+      "failed_restarts" => 1,
+      "scheduled" => 10
     }
     report.add_metric(:resources, resourcemetrics)
 
     timemetrics = {
-      :resource1 => 10,
-      :resource2 => 50,
-      :resource3 => 40,
-      :resource4 => 20,
+      "resource1" => 10,
+      "resource2" => 50,
+      "resource3" => 40,
+      "resource4" => 20,
     }
     report.add_metric(:times, timemetrics)
 
 
-          report.add_metric(
-        :changes,
-        
-      :total => 20
-    )
+    report.add_metric(:changes, "total" => 20)
 
     report.save
   end
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 96d464b7a..766d4f14d 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -101,22 +101,22 @@ 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("apply")
-      report.add_metric("changes", {:total => 1})
-      report.add_metric("resources", {:failed => 0})
+      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("apply")
-      report.add_metric("changes", {:total => 0})
-      report.add_metric("resources", {:failed => 1})
+      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("apply")
-      report.add_metric("changes", {:total => 1})
-      report.add_metric("resources", {:failed => 1})
+      report.add_metric("changes", {"total" => 1})
+      report.add_metric("resources", {"failed" => 1})
       report.exit_status.should == 6
     end
   end
@@ -162,7 +162,7 @@ describe Puppet::Transaction::Report do
         add_statuses(3)
 
         @report.finalize_report
-        metric(:resources, :total).should == 3
+        metric(:resources, "total").should == 3
       end
 
       Puppet::Resource::Status::STATES.each do |state|
@@ -170,7 +170,7 @@ describe Puppet::Transaction::Report do
           add_statuses(3) { |status| status.send(state.to_s + "=", true) }
 
           @report.finalize_report
-          metric(:resources, state).should == 3
+          metric(:resources, state.to_s).should == 3
         end
       end
 
@@ -185,13 +185,13 @@ describe Puppet::Transaction::Report do
       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
+        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
+        metric(:changes, "total").should == 0
         @report.status.should == 'unchanged'
       end
     end
@@ -234,15 +234,15 @@ describe Puppet::Transaction::Report do
     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.finalize_report
-        metric(:events, :total).should == 9
+        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
+        metric(:events, "total").should == 0
       end
 
       Puppet::Transaction::Event::EVENT_STATUSES.each do |status_name|
diff --git a/spec/unit/util/metric_spec.rb b/spec/unit/util/metric_spec.rb
index 72571ee4a..600b88f85 100755
--- a/spec/unit/util/metric_spec.rb
+++ b/spec/unit/util/metric_spec.rb
@@ -59,7 +59,7 @@ describe Puppet::Util::Metric do
   end
 
   it "should support a label for values" do
-    @metric.newvalue(:foo, 10, "label")
+    @metric.newvalue("foo", 10, "label")
     @metric.values[0][1].should == "label"
   end
 
@@ -69,19 +69,19 @@ describe Puppet::Util::Metric do
   end
 
   it "should return its values sorted by label" do
-    @metric.newvalue(:foo, 10, "b")
-    @metric.newvalue(:bar, 10, "a")
+    @metric.newvalue("foo", 10, "b")
+    @metric.newvalue("bar", 10, "a")
 
-    @metric.values.should == [[:bar, "a", 10], [:foo, "b", 10]]
+    @metric.values.should == [["bar", "a", 10], ["foo", "b", 10]]
   end
 
   it "should use an array indexer method to retrieve individual values" do
-    @metric.newvalue(:foo, 10)
-    @metric[:foo].should == 10
+    @metric.newvalue("foo", 10)
+    @metric["foo"].should == 10
   end
 
   it "should return nil if the named value cannot be found" do
-    @metric[:foo].should == 0
+    @metric["foo"].should == 0
   end
 
   # LAK: I'm not taking the time to develop these tests right now.
-- 
cgit 


From bd4a8a13a85e6d64c987765cb83b1ebd4a63a341 Mon Sep 17 00:00:00 2001
From: Nick Lewis <nick@puppetlabs.com>
Date: Wed, 29 Dec 2010 16:00:23 -0800
Subject: (#5715) Make certain report attributes always present.

The attributes Puppet::Resource::Status#changed,
Puppet::Resource::Status#out_of_sync, and
Puppet::Transaction::Event#audited used to only appear in reports when
their state was true.  Now they appear always.
---
 lib/puppet/resource/status.rb                  | 2 ++
 lib/puppet/transaction/event.rb                | 1 +
 spec/unit/resource/status_spec.rb              | 8 ++++----
 spec/unit/transaction/event_spec.rb            | 8 +++++++-
 spec/unit/transaction/resource_harness_spec.rb | 6 +++---
 5 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb
index 43d3f1644..b138c6520 100644
--- a/lib/puppet/resource/status.rb
+++ b/lib/puppet/resource/status.rb
@@ -47,6 +47,8 @@ module Puppet
         @resource = resource.to_s
         @change_count = 0
         @out_of_sync_count = 0
+        @changed = false
+        @out_of_sync = false
 
         [:file, :line].each do |attr|
           send(attr.to_s + "=", resource.send(attr))
diff --git a/lib/puppet/transaction/event.rb b/lib/puppet/transaction/event.rb
index e3537bb08..b9fa5c38b 100644
--- a/lib/puppet/transaction/event.rb
+++ b/lib/puppet/transaction/event.rb
@@ -16,6 +16,7 @@ class Puppet::Transaction::Event
   EVENT_STATUSES = %w{noop success failure audit}
 
   def initialize(*args)
+    @audited = false
     options = args.last.is_a?(Hash) ? args.pop : ATTRIBUTES.inject({}) { |hash, attr| hash[attr] = args.pop; hash }
     options.each { |attr, value| send(attr.to_s + "=", value) unless value.nil? }
 
diff --git a/spec/unit/resource/status_spec.rb b/spec/unit/resource/status_spec.rb
index 22d5fb4f9..3d9a84152 100755
--- a/spec/unit/resource/status_spec.rb
+++ b/spec/unit/resource/status_spec.rb
@@ -112,20 +112,20 @@ describe Puppet::Resource::Status do
   it "should not start with any changes" do
     @status.change_count.should == 0
 
-    @status.changed.should be_false
-    @status.out_of_sync.should be_false
+    @status.changed.should == false
+    @status.out_of_sync.should == 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
+    @status.changed.should == 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
+    @status.out_of_sync.should == false
   end
 
   ['failure', 'noop', 'success'].each do |event_status|
diff --git a/spec/unit/transaction/event_spec.rb b/spec/unit/transaction/event_spec.rb
index 4a3e6bad5..2776be8d7 100755
--- a/spec/unit/transaction/event_spec.rb
+++ b/spec/unit/transaction/event_spec.rb
@@ -5,7 +5,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/transaction/event'
 
 describe Puppet::Transaction::Event do
-  [:previous_value, :desired_value, :property, :resource, :name, :message, :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)
diff --git a/spec/unit/transaction/resource_harness_spec.rb b/spec/unit/transaction/resource_harness_spec.rb
index 387deca61..771c7b4b0 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -208,11 +208,11 @@ describe Puppet::Transaction::ResourceHarness do
                       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)
+                      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 != nil
+                      synced_should_be_set = !noop_mode && status.changed
                       (@harness.cached(resource, :synced) != nil).should == synced_should_be_set
                     end; end
                   end
-- 
cgit 


From 98db2da4e9b35f6260bf07c641550b380b603baa Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Thu, 30 Dec 2010 11:01:23 -0800
Subject: (#5715) Removed unnecessary attributes from YAML of
 Puppet::Transaction::Event.

The removed attributes are file, line, resource, tags,
source_description, and default_log_level.  These attributes were all
redundant with those in Puppet::Resource::Status.
---
 lib/puppet/transaction/event.rb     |  5 +++++
 spec/unit/transaction/event_spec.rb | 13 +++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/lib/puppet/transaction/event.rb b/lib/puppet/transaction/event.rb
index b9fa5c38b..64980f1d9 100644
--- a/lib/puppet/transaction/event.rb
+++ b/lib/puppet/transaction/event.rb
@@ -8,6 +8,7 @@ class Puppet::Transaction::Event
   include Puppet::Util::Logging
 
   ATTRIBUTES = [:name, :resource, :property, :previous_value, :desired_value, :historical_value, :status, :message, :file, :line, :source_description, :audited]
+  YAML_ATTRIBUTES = %w{@audited @property @previous_value @desired_value @historical_value @message @name @status @time}
   attr_accessor *ATTRIBUTES
   attr_writer :tags
   attr_accessor :time
@@ -47,6 +48,10 @@ class Puppet::Transaction::Event
     message
   end
 
+  def to_yaml_properties
+    (YAML_ATTRIBUTES & instance_variables).sort
+  end
+
   private
 
   # If it's a failure, use 'err', else use either the resource's log level (if available)
diff --git a/spec/unit/transaction/event_spec.rb b/spec/unit/transaction/event_spec.rb
index 2776be8d7..6ed14722b 100755
--- a/spec/unit/transaction/event_spec.rb
+++ b/spec/unit/transaction/event_spec.rb
@@ -111,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
-- 
cgit 


From a6cd736379d37c5ca205d74d8a957a6c79f95548 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Thu, 30 Dec 2010 11:18:57 -0800
Subject: (#5715) Removed attribute source_description from the YAML
 representation of Puppet::Resource::Status.

This attribute is only used for properly generating log messages and
was never intended to appear in reports.
---
 lib/puppet/resource/status.rb     |  6 ++++++
 spec/unit/resource/status_spec.rb | 10 ++++++++++
 2 files changed, 16 insertions(+)

diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb
index b138c6520..1108aa611 100644
--- a/lib/puppet/resource/status.rb
+++ b/lib/puppet/resource/status.rb
@@ -12,6 +12,8 @@ module Puppet
       attr_reader :source_description, :default_log_level, :time, :resource
       attr_reader :change_count, :out_of_sync_count
 
+      YAML_ATTRIBUTES = %w{@resource @file @line @evaluation_time @change_count @out_of_sync_count @tags @time @events @out_of_sync @changed}
+
       # Provide a boolean method for each of the states.
       STATES.each do |attr|
         define_method("#{attr}?") do
@@ -59,6 +61,10 @@ module Puppet
         @events = []
       end
 
+      def to_yaml_properties
+        (YAML_ATTRIBUTES & instance_variables).sort
+      end
+
       private
 
       def log_source
diff --git a/spec/unit/resource/status_spec.rb b/spec/unit/resource/status_spec.rb
index 3d9a84152..815f799a2 100755
--- a/spec/unit/resource/status_spec.rb
+++ b/spec/unit/resource/status_spec.rb
@@ -135,4 +135,14 @@ describe Puppet::Resource::Status do
       @status.out_of_sync.should == true
     end
   end
+
+  describe "When converting to YAML" do
+    it "should include only documented attributes" do
+      @status.file = "/foo.rb"
+      @status.line = 27
+      @status.evaluation_time = 2.7
+      @status.tags = %w{one two}
+      @status.to_yaml_properties.should == Puppet::Resource::Status::YAML_ATTRIBUTES.sort
+    end
+  end
 end
-- 
cgit 


From 1f72c31f9e0223e71e2729da96e0e98ebea5417e Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Thu, 30 Dec 2010 11:26:30 -0800
Subject: (#5715) Added attributes resource_type and title to
 Puppet::Resource::Status.

These new attributes save report processors from having to parse the
"resource" attribute.
---
 lib/puppet/resource/status.rb     | 6 ++++--
 spec/unit/resource/status_spec.rb | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/puppet/resource/status.rb b/lib/puppet/resource/status.rb
index 1108aa611..ee83004bb 100644
--- a/lib/puppet/resource/status.rb
+++ b/lib/puppet/resource/status.rb
@@ -10,9 +10,9 @@ module Puppet
       attr_accessor *STATES
 
       attr_reader :source_description, :default_log_level, :time, :resource
-      attr_reader :change_count, :out_of_sync_count
+      attr_reader :change_count, :out_of_sync_count, :resource_type, :title
 
-      YAML_ATTRIBUTES = %w{@resource @file @line @evaluation_time @change_count @out_of_sync_count @tags @time @events @out_of_sync @changed}
+      YAML_ATTRIBUTES = %w{@resource @file @line @evaluation_time @change_count @out_of_sync_count @tags @time @events @out_of_sync @changed @resource_type @title}
 
       # Provide a boolean method for each of the states.
       STATES.each do |attr|
@@ -59,6 +59,8 @@ module Puppet
         tag(*resource.tags)
         @time = Time.now
         @events = []
+        @resource_type = resource.type.to_s.capitalize
+        @title = resource.title
       end
 
       def to_yaml_properties
diff --git a/spec/unit/resource/status_spec.rb b/spec/unit/resource/status_spec.rb
index 815f799a2..4e76fa463 100755
--- a/spec/unit/resource/status_spec.rb
+++ b/spec/unit/resource/status_spec.rb
@@ -10,6 +10,11 @@ describe Puppet::Resource::Status do
     @status = Puppet::Resource::Status.new(@resource)
   end
 
+  it "should compute type and title correctly" do
+    @status.resource_type.should == "File"
+    @status.title.should == "/my/file"
+  end
+
   [:node, :file, :line, :current_values, :status, :evaluation_time].each do |attr|
     it "should support #{attr}" do
       @status.send(attr.to_s + "=", "foo")
-- 
cgit 


From 06a8d1ee8775ba8693ced002f5972bbfc346ebf8 Mon Sep 17 00:00:00 2001
From: Jesse Wolfe <jes5199@gmail.com>
Date: Tue, 28 Dec 2010 17:18:55 -0800
Subject: Fix #5698 puppet inspect shouldn't report of attributes of deleted
 files

If a resource is absent, then reporting that its properties are all
":absent" is not particularly correct. This patch makes the `inspect`
application's reports behave more like `apply` reports, and skip
properties other than :ensure for absent resources.

Reviewed-By: Nick Lewis <nick@puppetlabs.com>
---
 lib/puppet/application/inspect.rb     |  8 ++++++--
 lib/puppet/reports/http.rb            |  2 +-
 spec/unit/application/inspect_spec.rb | 25 ++++++++++++++++++++++++-
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb
index caa32a7c2..342b8daae 100644
--- a/lib/puppet/application/inspect.rb
+++ b/lib/puppet/application/inspect.rb
@@ -64,8 +64,12 @@ class Puppet::Application::Inspect < Puppet::Application
 
       status = Puppet::Resource::Status.new(ral_resource)
       audited_attributes.each do |name|
-        event = ral_resource.event(:previous_value => audited_resource[name], :property => name, :status => "audit", :message => "inspected value is #{audited_resource[name].inspect}")
-        status.add_event(event)
+        next if audited_resource[name].nil?
+        # Skip :absent properties of :absent resources. Really, it would be nicer if the RAL returned nil for those, but it doesn't. ~JW
+        if name == :ensure or audited_resource[:ensure] != :absent or audited_resource[name] != :absent
+          event = ral_resource.event(:previous_value => audited_resource[name], :property => name, :status => "audit", :message => "inspected value is #{audited_resource[name].inspect}")
+          status.add_event(event)
+        end
       end
       @report.add_resource_status(status)
     end
diff --git a/lib/puppet/reports/http.rb b/lib/puppet/reports/http.rb
index 7ac54dfbd..101c8e0cb 100644
--- a/lib/puppet/reports/http.rb
+++ b/lib/puppet/reports/http.rb
@@ -15,7 +15,7 @@ Puppet::Reports.register_report(:http) do
     req = Net::HTTP::Post.new(url.path)
     req.body = self.to_yaml
     req.content_type = "application/x-yaml"
-    Net::HTTP.new(url.host, url.port).start {|http|
+    p Net::HTTP.new(url.host, url.port).start {|http|
       http.request(req)
     }
   end
diff --git a/spec/unit/application/inspect_spec.rb b/spec/unit/application/inspect_spec.rb
index a3cc74d86..b931708c3 100644
--- a/spec/unit/application/inspect_spec.rb
+++ b/spec/unit/application/inspect_spec.rb
@@ -51,7 +51,7 @@ describe Puppet::Application::Inspect do
       catalog = Puppet::Resource::Catalog.new
       file = Tempfile.new("foo")
       file.puts("file contents")
-      file.flush
+      file.close
       resource = Puppet::Resource.new(:file, file.path, :parameters => {:audit => "all"})
       catalog.add_resource(resource)
       Puppet::Resource::Catalog::Yaml.any_instance.stubs(:find).returns(catalog)
@@ -69,6 +69,29 @@ describe Puppet::Application::Inspect do
       end
       properties["ensure"].should == :file
       properties["content"].should == "{md5}#{Digest::MD5.hexdigest("file contents\n")}"
+      properties.has_key?("target").should == false
+    end
+
+    it "should not report irrelevent attributes if the resource is absent" do
+      catalog = Puppet::Resource::Catalog.new
+      file = Tempfile.new("foo")
+      resource = Puppet::Resource.new(:file, file.path, :parameters => {:audit => "all"})
+      file.delete
+      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.should == {"ensure" => :absent}
     end
   end
 
-- 
cgit 


From e162da905549002d31eb9ff159406cec8b659046 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Mon, 3 Jan 2011 15:45:41 -0800
Subject: Prep work for #5758: clean up initializer for
 Puppet::Transaction::Event

Puppet::Transaction::Event#initialize() contained unnecessary code
that allowed the properties of an event to be passed in as separate
arguments.  This was never used.

Paired-with: Matt Robinson <matt@puppetlabs.com>
---
 lib/puppet/transaction/event.rb | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lib/puppet/transaction/event.rb b/lib/puppet/transaction/event.rb
index 64980f1d9..cd695cff8 100644
--- a/lib/puppet/transaction/event.rb
+++ b/lib/puppet/transaction/event.rb
@@ -16,10 +16,9 @@ class Puppet::Transaction::Event
 
   EVENT_STATUSES = %w{noop success failure audit}
 
-  def initialize(*args)
+  def initialize(options = {})
     @audited = false
-    options = args.last.is_a?(Hash) ? args.pop : ATTRIBUTES.inject({}) { |hash, attr| hash[attr] = args.pop; hash }
-    options.each { |attr, value| send(attr.to_s + "=", value) unless value.nil? }
+    options.each { |attr, value| send(attr.to_s + "=", value) }
 
     @time = Time.now
   end
-- 
cgit 


From de85f8d598606fc4fcce59228f5cb6d2715e66b5 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Mon, 3 Jan 2011 15:47:15 -0800
Subject: Prep work for #5758: set audited=true on all audit events

Previously, events would only have audited=true if auditing was
enabled AND there had been a change in an audited parameter.

Paired-with: Matt Robinson <matt@puppetlabs.com>
---
 lib/puppet/transaction/resource_harness.rb | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/puppet/transaction/resource_harness.rb b/lib/puppet/transaction/resource_harness.rb
index cb9a193b9..c259d3e05 100644
--- a/lib/puppet/transaction/resource_harness.rb
+++ b/lib/puppet/transaction/resource_harness.rb
@@ -84,10 +84,12 @@ class Puppet::Transaction::ResourceHarness
     event.desired_value = property.should
     event.historical_value = historical_value
 
-    if do_audit and historical_value != current_value
-      event.message = "audit change: previously recorded value #{property.is_to_s(historical_value)} has been changed to #{property.is_to_s(current_value)}"
-      event.status = "audit"
+    if do_audit
       event.audited = true
+      event.status = "audit"
+      if historical_value != current_value
+        event.message = "audit change: previously recorded value #{property.is_to_s(historical_value)} has been changed to #{property.is_to_s(current_value)}"
+      end
     end
 
     event
@@ -96,7 +98,7 @@ class Puppet::Transaction::ResourceHarness
   def apply_parameter(property, current_value, do_audit, historical_value)
     event = create_change_event(property, current_value, do_audit, historical_value)
 
-    if event.audited && historical_value
+    if do_audit && historical_value && historical_value != current_value
       brief_audit_message = " (previously recorded value was #{property.is_to_s(historical_value)})"
     else
       brief_audit_message = ""
-- 
cgit 


From 80bfb541f06f36e37fe34d0c1635c317085ea333 Mon Sep 17 00:00:00 2001
From: Paul Berry <paul@puppetlabs.com>
Date: Mon, 3 Jan 2011 15:48:15 -0800
Subject: (#5758) Verify that report events are correctly created

Added code to resource_harness_spec to test all the attributes of
report events, not just the log messages.

Paired-with: Matt Robinson <matt@puppetlabs.com>
---
 spec/unit/transaction/resource_harness_spec.rb | 62 +++++++++++++++++++++++---
 1 file changed, 56 insertions(+), 6 deletions(-)

diff --git a/spec/unit/transaction/resource_harness_spec.rb b/spec/unit/transaction/resource_harness_spec.rb
index 771c7b4b0..65c148a93 100755
--- a/spec/unit/transaction/resource_harness_spec.rb
+++ b/spec/unit/transaction/resource_harness_spec.rb
@@ -54,6 +54,16 @@ describe Puppet::Transaction::ResourceHarness do
     end
   end
 
+  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
     [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
@@ -85,11 +95,15 @@ describe Puppet::Transaction::ResourceHarness do
                       @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
@@ -125,53 +139,88 @@ describe Puppet::Transaction::ResourceHarness do
 
                       # 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
-                          expected_logs << "notice: /#{resource}/mode: #{what_happened} (previously recorded value was #{yaml_mode})"
+                          mode_status_msg = "#{what_happened} (previously recorded value was #{yaml_mode})"
                         else
-                          expected_logs << "notice: /#{resource}/mode: #{what_happened}"
+                          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
-                            expected_logs << "notice: /#{resource}/mode: audit change: previously recorded value #{yaml_mode} has been changed to #{@harness.cached(resource, :mode)}"
+                            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
-                          expected_logs << "notice: /#{resource}/ensure: #{what_happened} (previously recorded value was #{yaml_ensure})"
+                          ensure_status_msg = "#{what_happened} (previously recorded value was #{yaml_ensure})"
                         else
-                          expected_logs << "notice: /#{resource}/ensure: #{what_happened}"
+                          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
-                            expected_logs << "notice: /#{resource}/ensure: audit change: previously recorded value #{yaml_ensure} has been changed to #{@harness.cached(resource, :ensure)}"
+                            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
@@ -179,6 +228,7 @@ describe Puppet::Transaction::ResourceHarness do
                       # 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
-- 
cgit 


From 7661ba82a3c765d4bf8a400a3c770f351c541f96 Mon Sep 17 00:00:00 2001
From: Jesse Wolfe <jes5199@gmail.com>
Date: Mon, 3 Jan 2011 16:28:24 -0800
Subject: maint: Prune #inspect methods on various objects

Ruby's default #inspect method can lead to printing factorial-order
output for large graphs of objects. Since we have large graphs of
objects, this is not optimal.
This patch replaces a few well-connected objects' #inspect methods with
methods that produce reduced output, and are thus much faster.

Paired-With: Nick Lewis <nick@puppetlabs.com>
---
 lib/puppet/parser/ast.rb               | 4 ++++
 lib/puppet/relationship.rb             | 4 ++++
 lib/puppet/resource.rb                 | 4 ++++
 lib/puppet/resource/type_collection.rb | 4 ++++
 lib/puppet/simple_graph.rb             | 4 ++++
 5 files changed, 20 insertions(+)

diff --git a/lib/puppet/parser/ast.rb b/lib/puppet/parser/ast.rb
index 54e034acb..a5aaeddc4 100644
--- a/lib/puppet/parser/ast.rb
+++ b/lib/puppet/parser/ast.rb
@@ -19,6 +19,10 @@ class Puppet::Parser::AST
 
   attr_accessor :parent, :scope
 
+  def inspect
+    "( #{self.class} #{self.to_s} #{@children.inspect} )"
+  end
+
   # don't fetch lexer comment by default
   def use_docs
     self.class.use_docs
diff --git a/lib/puppet/relationship.rb b/lib/puppet/relationship.rb
index 7079fb44b..08d7d042b 100644
--- a/lib/puppet/relationship.rb
+++ b/lib/puppet/relationship.rb
@@ -71,6 +71,10 @@ class Puppet::Relationship
     "#{source} => #{target}"
   end
 
+  def inspect
+    "{ #{source} => #{target} }"
+  end
+
   def to_pson_data_hash
     data = {
       'source' => source.to_s,
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index 4f0d50750..b0a3ecee6 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -46,6 +46,10 @@ class Puppet::Resource
     resource
   end
 
+  def inspect
+    "#{@type}[#{@title}]#{to_hash.inspect}"
+  end
+
   def to_pson_data_hash
     data = ([:type, :title, :tags] + ATTRIBUTES).inject({}) do |hash, param|
       next hash unless value = self.send(param)
diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb
index 6a03458b3..277d37b18 100644
--- a/lib/puppet/resource/type_collection.rb
+++ b/lib/puppet/resource/type_collection.rb
@@ -19,6 +19,10 @@ class Puppet::Resource::TypeCollection
     @watched_files = {}
   end
 
+  def inspect
+    "TypeCollection" + { :hostclasses => @hostclasses.keys, :definitions => @definitions.keys, :nodes => @nodes.keys }.inspect
+  end
+
   def <<(thing)
     add(thing)
     self
diff --git a/lib/puppet/simple_graph.rb b/lib/puppet/simple_graph.rb
index 55b39fadf..c5dac0f6c 100644
--- a/lib/puppet/simple_graph.rb
+++ b/lib/puppet/simple_graph.rb
@@ -80,6 +80,10 @@ class Puppet::SimpleGraph
       vertex.to_s
     end
 
+    def inspect
+      { :@adjacencies => @adjacencies, :@vertex => @vertex.to_s }.inspect
+    end
+
     private
 
     # These methods exist so we don't need a Hash with a default proc.
-- 
cgit 


From 7603b055d10810a901b6e0f9d9de89e00b842058 Mon Sep 17 00:00:00 2001
From: Jesse Wolfe <jes5199@gmail.com>
Date: Mon, 3 Jan 2011 17:27:14 -0800
Subject: maint: remove stray debug statement.

I accidentally committed a debug statement. Now I am removing it.
---
 lib/puppet/reports/http.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/puppet/reports/http.rb b/lib/puppet/reports/http.rb
index 101c8e0cb..7ac54dfbd 100644
--- a/lib/puppet/reports/http.rb
+++ b/lib/puppet/reports/http.rb
@@ -15,7 +15,7 @@ Puppet::Reports.register_report(:http) do
     req = Net::HTTP::Post.new(url.path)
     req.body = self.to_yaml
     req.content_type = "application/x-yaml"
-    p Net::HTTP.new(url.host, url.port).start {|http|
+    Net::HTTP.new(url.host, url.port).start {|http|
       http.request(req)
     }
   end
-- 
cgit 


From 52760a4fb2764d011609c82adb68cc61d0772dc4 Mon Sep 17 00:00:00 2001
From: Matt Robinson <matt@puppetlabs.com>
Date: Mon, 3 Jan 2011 22:19:41 -0800
Subject: (#5771) Upgrade rspec to version 2

The biggest change is that we no longer need to monkey patch rspec to
get confine behavior.  Describe blocks can now be conditional like
confine used to be. "describe" blocks with "shared => true" are now
"shared_examples_for".

Paired-With: Nick Lewis
---
 Rakefile                                           |  8 ++--
 spec/integration/application/apply_spec.rb         |  3 +-
 spec/integration/indirector/catalog/queue_spec.rb  |  5 +--
 spec/integration/network/formats_spec.rb           |  7 +---
 spec/integration/network/server/mongrel_spec.rb    |  3 +-
 spec/integration/provider/package_spec.rb          |  4 +-
 spec/integration/provider/service/init_spec.rb     | 12 ++----
 spec/integration/resource/catalog_spec.rb          |  3 +-
 .../add_confine_and_runnable_to_rspec_dsl.rb       | 46 ----------------------
 spec/monkey_patches/alias_should_to_must.rb        |  2 +
 spec/shared_behaviours/file_server_terminus.rb     |  2 +-
 spec/shared_behaviours/file_serving.rb             |  2 +-
 spec/shared_behaviours/memory_terminus.rb          |  2 +-
 spec/spec_helper.rb                                | 10 ++---
 spec/unit/application/kick_spec.rb                 |  4 +-
 spec/unit/application/master_spec.rb               |  4 +-
 spec/unit/application_spec.rb                      |  7 +---
 spec/unit/file_serving/metadata_spec.rb            |  8 ++--
 spec/unit/indirector/catalog/active_record_spec.rb |  4 +-
 spec/unit/indirector/facts/active_record_spec.rb   |  4 +-
 spec/unit/indirector/facts/couch_spec.rb           |  3 +-
 spec/unit/indirector/indirection_spec.rb           |  4 +-
 spec/unit/indirector/ldap_spec.rb                  |  8 +---
 spec/unit/indirector/node/active_record_spec.rb    |  4 +-
 spec/unit/indirector/queue_spec.rb                 |  4 +-
 spec/unit/indirector/rest_spec.rb                  |  2 +-
 spec/unit/indirector_spec.rb                       |  2 +-
 spec/unit/module_spec.rb                           |  4 +-
 spec/unit/network/formats_spec.rb                  |  7 +---
 spec/unit/network/http/compression_spec.rb         |  4 +-
 spec/unit/network/http/mongrel/rest_spec.rb        |  3 +-
 spec/unit/network/http/mongrel_spec.rb             | 12 ++----
 spec/unit/network/http/rack/rest_spec.rb           |  4 +-
 spec/unit/network/http/rack/xmlrpc_spec.rb         |  4 +-
 spec/unit/network/http/rack_spec.rb                |  4 +-
 spec/unit/parser/collector_spec.rb                 |  8 +---
 spec/unit/parser/lexer_spec.rb                     |  5 ++-
 spec/unit/provider/mount/parsed_spec.rb            |  3 +-
 spec/unit/rails/host_spec.rb                       |  4 +-
 spec/unit/rails/param_value_spec.rb                |  4 +-
 spec/unit/rails/resource_spec.rb                   |  4 +-
 spec/unit/rails_spec.rb                            | 20 +++-------
 spec/unit/relationship_spec.rb                     |  8 +---
 spec/unit/resource/catalog_spec.rb                 |  8 +---
 spec/unit/resource_spec.rb                         |  8 +---
 .../ssl/certificate_authority/interface_spec.rb    |  2 +-
 spec/unit/type/augeas_spec.rb                      |  4 +-
 spec/unit/type/file_spec.rb                        | 12 +++---
 spec/unit/util/queue/stomp_spec.rb                 |  8 +---
 spec/unit/util/settings/file_setting_spec.rb       |  4 +-
 50 files changed, 83 insertions(+), 228 deletions(-)
 delete mode 100644 spec/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb

diff --git a/Rakefile b/Rakefile
index 8e21cebd9..728f1e3de 100644
--- a/Rakefile
+++ b/Rakefile
@@ -5,8 +5,8 @@ $LOAD_PATH << File.join(File.dirname(__FILE__), 'tasks')
 require 'rake'
 require 'rake/packagetask'
 require 'rake/gempackagetask'
-require 'spec'
-require 'spec/rake/spectask'
+require 'rspec'
+require "rspec/core/rake_task"
 
 module Puppet
     PUPPETVERSION = File.read('lib/puppet.rb')[/PUPPETVERSION *= *'(.*)'/,1] or fail "Couldn't find PUPPETVERSION"
@@ -42,8 +42,8 @@ end
 desc "Create the tarball and the gem - use when releasing"
 task :puppetpackages => [:create_gem, :package]
 
-Spec::Rake::SpecTask.new do |t|
-    t.spec_opts = ['--format','s', '--loadby','mtime','--color']
+RSpec::Core::RakeTask.new do |t|
+    t.rspec_opts = ['--format','s', '--color']
     t.pattern ='spec/{unit,integration}/**/*.rb'
     t.fail_on_error = false
 end
diff --git a/spec/integration/application/apply_spec.rb b/spec/integration/application/apply_spec.rb
index 840917eff..363aa3469 100755
--- a/spec/integration/application/apply_spec.rb
+++ b/spec/integration/application/apply_spec.rb
@@ -9,8 +9,7 @@ require 'puppet/application/apply'
 describe "apply" do
   include PuppetSpec::Files
 
-  describe "when applying provided catalogs" do
-    confine "PSON library is missing; cannot test applying catalogs" => Puppet.features.pson?
+  describe "when applying provided catalogs", :if => Puppet.features.pson? do
     it "should be able to apply catalogs provided in a file in pson" do
       file_to_create = tmpfile("pson_catalog")
       catalog = Puppet::Resource::Catalog.new
diff --git a/spec/integration/indirector/catalog/queue_spec.rb b/spec/integration/indirector/catalog/queue_spec.rb
index 0f8bd41fb..4581e3062 100755
--- a/spec/integration/indirector/catalog/queue_spec.rb
+++ b/spec/integration/indirector/catalog/queue_spec.rb
@@ -4,10 +4,7 @@ Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f
 
 require 'puppet/resource/catalog'
 
-
-describe "Puppet::Resource::Catalog::Queue" do
-  confine "Missing pson support; cannot test queue" => Puppet.features.pson?
-
+describe "Puppet::Resource::Catalog::Queue", :if => Puppet.features.pson? do
   before do
     Puppet::Resource::Catalog.indirection.terminus(:queue)
     @catalog = Puppet::Resource::Catalog.new
diff --git a/spec/integration/network/formats_spec.rb b/spec/integration/network/formats_spec.rb
index e6cf28abb..d141abf00 100755
--- a/spec/integration/network/formats_spec.rb
+++ b/spec/integration/network/formats_spec.rb
@@ -46,8 +46,7 @@ describe Puppet::Network::FormatHandler.format(:s) do
 end
 
 describe Puppet::Network::FormatHandler.format(:pson) do
-  describe "when pson is absent" do
-    confine "'pson' library is present" => (! Puppet.features.pson?)
+  describe "when pson is absent", :if => (! Puppet.features.pson?) do
 
     before do
       @pson = Puppet::Network::FormatHandler.format(:pson)
@@ -58,9 +57,7 @@ describe Puppet::Network::FormatHandler.format(:pson) do
     end
   end
 
-  describe "when pson is available" do
-    confine "Missing 'pson' library" => Puppet.features.pson?
-
+  describe "when pson is available", :if => Puppet.features.pson? do
     before do
       @pson = Puppet::Network::FormatHandler.format(:pson)
     end
diff --git a/spec/integration/network/server/mongrel_spec.rb b/spec/integration/network/server/mongrel_spec.rb
index cc90773e6..c2815b565 100755
--- a/spec/integration/network/server/mongrel_spec.rb
+++ b/spec/integration/network/server/mongrel_spec.rb
@@ -5,8 +5,7 @@ require 'puppet/network/server'
 require 'socket'
 
 describe Puppet::Network::Server do
-  describe "when using mongrel" do
-    confine "Mongrel is not available" => Puppet.features.mongrel?
+  describe "when using mongrel", :if => Puppet.features.mongrel? do
 
     before :each do
       Puppet[:servertype] = 'mongrel'
diff --git a/spec/integration/provider/package_spec.rb b/spec/integration/provider/package_spec.rb
index 736a34e68..472662d6b 100755
--- a/spec/integration/provider/package_spec.rb
+++ b/spec/integration/provider/package_spec.rb
@@ -6,9 +6,7 @@ describe "Package Provider" do
   Puppet::Type.type(:package).providers.each do |name|
     provider = Puppet::Type.type(:package).provider(name)
 
-    describe name do
-      confine "Provider #{name} is not suitable" => provider.suitable?
-
+    describe name, :if => provider.suitable? do
       it "should fail when asked to install an invalid package" do
         pending("This test hangs forever with recent versions of RubyGems") if provider.name == :gem
         pkg = Puppet::Type.newpackage :name => "nosuch#{provider.name}", :provider => provider.name
diff --git a/spec/integration/provider/service/init_spec.rb b/spec/integration/provider/service/init_spec.rb
index d916ab32a..2e2505bd4 100755
--- a/spec/integration/provider/service/init_spec.rb
+++ b/spec/integration/provider/service/init_spec.rb
@@ -6,25 +6,19 @@ Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f
 provider = Puppet::Type.type(:service).provider(:init)
 
 describe provider do
-  describe "when running on FreeBSD" do
-    confine "Not running on FreeBSD" => (Facter.value(:operatingsystem) == "FreeBSD")
-
+  describe "when running on FreeBSD", :if => (Facter.value(:operatingsystem) == "FreeBSD") do
     it "should set its default path to include /etc/init.d and /usr/local/etc/init.d" do
       provider.defpath.should == ["/etc/rc.d", "/usr/local/etc/rc.d"]
     end
   end
 
-  describe "when running on HP-UX" do
-    confine "Not running on HP-UX" => (Facter.value(:operatingsystem) == "HP-UX")
-
+  describe "when running on HP-UX", :if => (Facter.value(:operatingsystem) == "HP-UX")do
     it "should set its default path to include /sbin/init.d" do
       provider.defpath.should == "/sbin/init.d"
     end
   end
 
-  describe "when not running on FreeBSD or HP-UX" do
-    confine "Running on HP-UX or FreeBSD" => (! %w{HP-UX FreeBSD}.include?(Facter.value(:operatingsystem)))
-
+  describe "when not running on FreeBSD or HP-UX", :if => (! %w{HP-UX FreeBSD}.include?(Facter.value(:operatingsystem))) do
     it "should set its default path to include /etc/init.d" do
       provider.defpath.should == "/etc/init.d"
     end
diff --git a/spec/integration/resource/catalog_spec.rb b/spec/integration/resource/catalog_spec.rb
index 0a3d47a80..da2b70409 100755
--- a/spec/integration/resource/catalog_spec.rb
+++ b/spec/integration/resource/catalog_spec.rb
@@ -6,8 +6,7 @@
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 describe Puppet::Resource::Catalog do
-  describe "when pson is available" do
-    confine "PSON library is missing" => Puppet.features.pson?
+  describe "when pson is available", :if => Puppet.features.pson? do
     it "should support pson" do
       Puppet::Resource::Catalog.supported_formats.should be_include(:pson)
     end
diff --git a/spec/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb b/spec/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb
deleted file mode 100644
index 3762b7033..000000000
--- a/spec/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb
+++ /dev/null
@@ -1,46 +0,0 @@
-dir = File.expand_path(File.dirname(__FILE__))
-[ "#{dir}/../../lib", "#{dir}/../../test/lib"].each do |dir|
-    fulldir = File.expand_path(dir)
-    $LOAD_PATH.unshift(fulldir) unless $LOAD_PATH.include?(fulldir)
-end
-
-require 'spec'
-require 'puppettest/runnable_test'
-
-module Spec
-  module Runner
-    class ExampleGroupRunner
-      def run
-        prepare
-        success = true
-        example_groups.each do |example_group|
-          unless example_group.runnable?
-            warn "Skipping unsuitable example group #{example_group.description}: #{example_group.messages.join(", ")}"
-            next
-          end
-          success = success & example_group.run(@options)
-          Puppet.settings.clear
-        end
-        return success
-      ensure
-        finish
-      end
-    end
-  end
-end
-
-module Spec
-  module Example
-    class ExampleGroup
-      extend PuppetTest::RunnableTest
-    end
-  end
-end
-
-module Test
-  module Unit
-    class TestCase
-      extend PuppetTest::RunnableTest
-    end
-  end
-end
diff --git a/spec/monkey_patches/alias_should_to_must.rb b/spec/monkey_patches/alias_should_to_must.rb
index c8744136a..1a1111799 100644
--- a/spec/monkey_patches/alias_should_to_must.rb
+++ b/spec/monkey_patches/alias_should_to_must.rb
@@ -1,3 +1,5 @@
+require 'rspec'
+
 class Object
   # This is necessary because the RAL has a 'should'
   # method.
diff --git a/spec/shared_behaviours/file_server_terminus.rb b/spec/shared_behaviours/file_server_terminus.rb
index 665b46cd5..94a044d2e 100644
--- a/spec/shared_behaviours/file_server_terminus.rb
+++ b/spec/shared_behaviours/file_server_terminus.rb
@@ -3,7 +3,7 @@
 #  Created by Luke Kanies on 2007-10-18.
 #  Copyright (c) 2007. All rights reserved.
 
-describe "Puppet::Indirector::FileServerTerminus", :shared => true do
+shared_examples_for "Puppet::Indirector::FileServerTerminus" do
   # This only works if the shared behaviour is included before
   # the 'before' block in the including context.
   before do
diff --git a/spec/shared_behaviours/file_serving.rb b/spec/shared_behaviours/file_serving.rb
index 5f5b2b0af..450fff87a 100644
--- a/spec/shared_behaviours/file_serving.rb
+++ b/spec/shared_behaviours/file_serving.rb
@@ -3,7 +3,7 @@
 #  Created by Luke Kanies on 2007-10-18.
 #  Copyright (c) 2007. All rights reserved.
 
-describe "Puppet::FileServing::Files", :shared => true do
+shared_examples_for "Puppet::FileServing::Files" do
   it "should use the rest terminus when the 'puppet' URI scheme is used and a host name is present" do
     uri = "puppet://myhost/fakemod/my/file"
 
diff --git a/spec/shared_behaviours/memory_terminus.rb b/spec/shared_behaviours/memory_terminus.rb
index 5c9f35cca..f9325a969 100644
--- a/spec/shared_behaviours/memory_terminus.rb
+++ b/spec/shared_behaviours/memory_terminus.rb
@@ -2,7 +2,7 @@
 #  Created by Luke Kanies on 2008-4-8.
 #  Copyright (c) 2008. All rights reserved.
 
-describe "A Memory Terminus", :shared => true do
+shared_examples_for "A Memory Terminus" do
   it "should find no instances by default" do
     @searcher.find(@request).should be_nil
   end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index ed4e2c2fb..d0ee7d92a 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -12,8 +12,7 @@ ARGV.clear
 
 require 'puppet'
 require 'mocha'
-gem 'rspec', '>=1.2.9'
-require 'spec/autorun'
+gem 'rspec', '>=2.0.0'
 
 # So everyone else doesn't have to include this base constant.
 module PuppetSpec
@@ -22,13 +21,12 @@ end
 
 require 'lib/puppet_spec/files'
 require 'monkey_patches/alias_should_to_must'
-require 'monkey_patches/add_confine_and_runnable_to_rspec_dsl'
 require 'monkey_patches/publicize_methods'
 
-Spec::Runner.configure do |config|
+RSpec.configure do |config|
   config.mock_with :mocha
 
-  config.prepend_after :each do
+  config.after :each do
     Puppet.settings.clear
     Puppet::Node::Environment.clear
     Puppet::Util::Storage.clear
@@ -58,7 +56,7 @@ Spec::Runner.configure do |config|
     Puppet::Util::Log.close_all
   end
 
-  config.prepend_before :each do
+  config.before :each do
     # these globals are set by Application
     $puppet_application_mode = nil
     $puppet_application_name = nil
diff --git a/spec/unit/application/kick_spec.rb b/spec/unit/application/kick_spec.rb
index dea7ec147..c18a84de3 100755
--- a/spec/unit/application/kick_spec.rb
+++ b/spec/unit/application/kick_spec.rb
@@ -4,9 +4,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/application/kick'
 
-describe Puppet::Application::Kick do
-
-  confine "Kick's eventloops can only start on POSIX" => Puppet.features.posix?
+describe Puppet::Application::Kick, :if => Puppet.features.posix? do
 
   before :each do
     require 'puppet/util/ldap/connection'
diff --git a/spec/unit/application/master_spec.rb b/spec/unit/application/master_spec.rb
index 216c7dc90..074249a4d 100644
--- a/spec/unit/application/master_spec.rb
+++ b/spec/unit/application/master_spec.rb
@@ -412,9 +412,7 @@ describe Puppet::Application::Master do
         @master.main
       end
 
-      describe "with --rack" do
-        confine "Rack is not available" => Puppet.features.rack?
-
+      describe "with --rack", :if => Puppet.features.rack? do
         before do
           require 'puppet/network/http/rack'
           Puppet::Network::HTTP::Rack.stubs(:new).returns(@app)
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index be7cda340..f68a7e209 100755
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -160,9 +160,7 @@ describe Puppet::Application do
       end
     end
 
-    describe 'on POSIX systems' do
-      confine "HUP works only on POSIX systems" => Puppet.features.posix?
-
+    describe 'on POSIX systems', :if => Puppet.features.posix? do
       it 'should signal process with HUP after block if restart requested during block execution' do
         Puppet::Application.run_status = nil
         target = mock 'target'
@@ -224,8 +222,7 @@ describe Puppet::Application do
       @app.parse_options
     end
 
-    describe "when using --help" do
-      confine "rdoc" => Puppet.features.usage?
+    describe "when using --help", :if => Puppet.features.usage? do
 
       it "should call RDoc::usage and exit" do
         @app.expects(:exit)
diff --git a/spec/unit/file_serving/metadata_spec.rb b/spec/unit/file_serving/metadata_spec.rb
index aa0dcd511..dd40324bc 100755
--- a/spec/unit/file_serving/metadata_spec.rb
+++ b/spec/unit/file_serving/metadata_spec.rb
@@ -230,8 +230,8 @@ describe Puppet::FileServing::Metadata, " when collecting attributes" do
       @metadata.destination.should == "/path/to/link"
     end
 
-    it "should produce tab-separated mode, type, owner, group, and destination for xmlrpc" do
-      pending "We'd like this to be true, but we need to always collect the checksum because in the server/client/server round trip we lose the distintion between manage and follow."
+    pending "should produce tab-separated mode, type, owner, group, and destination for xmlrpc" do
+      # "We'd like this to be true, but we need to always collect the checksum because in the server/client/server round trip we lose the distintion between manage and follow."
       @metadata.attributes_with_tabs.should == "#{0755}\tlink\t10\t20\t/path/to/link"
     end
 
@@ -255,8 +255,8 @@ describe Puppet::FileServing::Metadata, " when pointing to a link" do
       @file.collect
       @file.destination.should == "/some/other/path"
     end
-    it "should not collect the checksum if links are :manage" do
-      pending "We'd like this to be true, but we need to always collect the checksum because in the server/client/server round trip we lose the distintion between manage and follow."
+    pending "should not collect the checksum if links are :manage" do
+      # We'd like this to be true, but we need to always collect the checksum because in the server/client/server round trip we lose the distintion between manage and follow.
       @file.collect
       @file.checksum.should be_nil
     end
diff --git a/spec/unit/indirector/catalog/active_record_spec.rb b/spec/unit/indirector/catalog/active_record_spec.rb
index ba8f1dad9..a368fb3a6 100755
--- a/spec/unit/indirector/catalog/active_record_spec.rb
+++ b/spec/unit/indirector/catalog/active_record_spec.rb
@@ -3,9 +3,7 @@
 require File.dirname(__FILE__) + '/../../../spec_helper'
 
 
-describe "Puppet::Resource::Catalog::ActiveRecord" do
-  confine "Missing Rails" => Puppet.features.rails?
-
+describe "Puppet::Resource::Catalog::ActiveRecord", :if => Puppet.features.rails? do
   require 'puppet/rails'
 
   before :all do
diff --git a/spec/unit/indirector/facts/active_record_spec.rb b/spec/unit/indirector/facts/active_record_spec.rb
index 0bdcfcb77..e96e01056 100755
--- a/spec/unit/indirector/facts/active_record_spec.rb
+++ b/spec/unit/indirector/facts/active_record_spec.rb
@@ -5,9 +5,7 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/rails'
 require 'puppet/node/facts'
 
-describe "Puppet::Node::Facts::ActiveRecord" do
-  confine "Missing Rails" => Puppet.features.rails?
-
+describe "Puppet::Node::Facts::ActiveRecord", :if => Puppet.features.rails? do
   before do
     require 'puppet/indirector/facts/active_record'
     Puppet.features.stubs(:rails?).returns true
diff --git a/spec/unit/indirector/facts/couch_spec.rb b/spec/unit/indirector/facts/couch_spec.rb
index c0dd54b8a..e3a9d7f14 100644
--- a/spec/unit/indirector/facts/couch_spec.rb
+++ b/spec/unit/indirector/facts/couch_spec.rb
@@ -4,8 +4,7 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppet/node/facts'
 
-describe "Puppet::Node::Facts::Couch" do
-  confine "couchrest gem is missing; cannot test couch terminus" => Puppet.features.couchdb?
+describe "Puppet::Node::Facts::Couch", :if => Puppet.features.couchdb? do
   require 'puppet/indirector/facts/couch' if Puppet.features.couchdb?
 
   before do
diff --git a/spec/unit/indirector/indirection_spec.rb b/spec/unit/indirector/indirection_spec.rb
index b0e0f019c..1e774fb2e 100755
--- a/spec/unit/indirector/indirection_spec.rb
+++ b/spec/unit/indirector/indirection_spec.rb
@@ -4,7 +4,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/indirector/indirection'
 
-describe "Indirection Delegator", :shared => true do
+shared_examples_for "Indirection Delegator" do
   it "should create a request object with the appropriate method name and all of the passed arguments" do
     request = Puppet::Indirector::Request.new(:indirection, :find, "me")
 
@@ -64,7 +64,7 @@ describe "Indirection Delegator", :shared => true do
   end
 end
 
-describe "Delegation Authorizer", :shared => true do
+shared_examples_for "Delegation Authorizer" do
   before do
     # So the :respond_to? turns out correctly.
     class << @terminus
diff --git a/spec/unit/indirector/ldap_spec.rb b/spec/unit/indirector/ldap_spec.rb
index 31a3406e9..c071f870e 100755
--- a/spec/unit/indirector/ldap_spec.rb
+++ b/spec/unit/indirector/ldap_spec.rb
@@ -110,9 +110,7 @@ describe Puppet::Indirector::Ldap do
     end
   end
 
-  describe "when connecting to ldap" do
-    confine "LDAP is not available" => Puppet.features.ldap?
-
+  describe "when connecting to ldap", :if => Puppet.features.ldap? do
     it "should create and start a Util::Ldap::Connection instance" do
       conn = mock 'connection', :connection => "myconn", :start => nil
       Puppet::Util::Ldap::Connection.expects(:instance).returns conn
@@ -135,9 +133,7 @@ describe Puppet::Indirector::Ldap do
     end
   end
 
-  describe "when reconnecting to ldap" do
-    confine "Not running on culain as root" => (Puppet.features.root? and Facter.value("hostname") == "culain")
-
+  describe "when reconnecting to ldap", :if => (Puppet.features.root? and Facter.value("hostname") == "culain") do
     it "should reconnect to ldap when connections are lost"
   end
 end
diff --git a/spec/unit/indirector/node/active_record_spec.rb b/spec/unit/indirector/node/active_record_spec.rb
index 3540ef738..69229e144 100755
--- a/spec/unit/indirector/node/active_record_spec.rb
+++ b/spec/unit/indirector/node/active_record_spec.rb
@@ -4,11 +4,9 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppet/node'
 
-describe "Puppet::Node::ActiveRecord" do
+describe "Puppet::Node::ActiveRecord", :if => Puppet.features.rails? && Puppet.features.sqlite? do
   include PuppetSpec::Files
 
-  confine "Missing Rails" => Puppet.features.rails?
-  confine "Missing sqlite" => Puppet.features.sqlite?
   before do
     require 'puppet/indirector/node/active_record'
   end
diff --git a/spec/unit/indirector/queue_spec.rb b/spec/unit/indirector/queue_spec.rb
index 83e9c771d..00463ee0f 100755
--- a/spec/unit/indirector/queue_spec.rb
+++ b/spec/unit/indirector/queue_spec.rb
@@ -26,9 +26,7 @@ class FooExampleData
   end
 end
 
-describe Puppet::Indirector::Queue do
-  confine "PSON library is missing; cannot test queueing" => Puppet.features.pson?
-
+describe Puppet::Indirector::Queue, :if => Puppet.features.pson? do
   before :each do
     @model = mock 'model'
     @indirection = stub 'indirection', :name => :my_queue, :register_terminus_type => nil, :model => @model
diff --git a/spec/unit/indirector/rest_spec.rb b/spec/unit/indirector/rest_spec.rb
index 3efbfce97..5f0fe363a 100755
--- a/spec/unit/indirector/rest_spec.rb
+++ b/spec/unit/indirector/rest_spec.rb
@@ -3,7 +3,7 @@
 require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/indirector/rest'
 
-describe "a REST http call", :shared => true do
+shared_examples_for "a REST http call" do
   it "should accept a path" do
     lambda { @search.send(@method, *@arguments) }.should_not raise_error(ArgumentError)
   end
diff --git a/spec/unit/indirector_spec.rb b/spec/unit/indirector_spec.rb
index fb21532ba..acbfc1726 100755
--- a/spec/unit/indirector_spec.rb
+++ b/spec/unit/indirector_spec.rb
@@ -64,7 +64,7 @@ describe Puppet::Indirector, "when registering an indirection" do
   end
 end
 
-describe "Delegated Indirection Method", :shared => true do
+shared_examples_for "Delegated Indirection Method" do
   it "should delegate to the indirection" do
     @indirection.expects(@method)
     @thingie.send(@method, "me")
diff --git a/spec/unit/module_spec.rb b/spec/unit/module_spec.rb
index c3436dfdd..37dad7e25 100755
--- a/spec/unit/module_spec.rb
+++ b/spec/unit/module_spec.rb
@@ -504,9 +504,7 @@ describe Puppet::Module do
     Puppet::Module.new("yay")
   end
 
-  describe "when loading the medatada file" do
-    confine "Cannot test module metadata without json" => Puppet.features.json?
-
+  describe "when loading the medatada file", :if => Puppet.features.json? do
     before do
       @data = {
         :license => "GPL2",
diff --git a/spec/unit/network/formats_spec.rb b/spec/unit/network/formats_spec.rb
index 7c8e7b1f4..2c58a0534 100755
--- a/spec/unit/network/formats_spec.rb
+++ b/spec/unit/network/formats_spec.rb
@@ -69,9 +69,8 @@ describe "Puppet Network Format" do
     end
   end
 
-  describe "base64 compressed yaml" do
+  describe "base64 compressed yaml", :if => Puppet.features.zlib? do
     yaml = Puppet::Network::FormatHandler.format(:b64_zlib_yaml)
-    confine "We must have zlib" => Puppet.features.zlib?
 
     before do
       @yaml = Puppet::Network::FormatHandler.format(:b64_zlib_yaml)
@@ -265,9 +264,7 @@ describe "Puppet Network Format" do
     Puppet::Network::FormatHandler.format(:pson).should_not be_nil
   end
 
-  describe "pson" do
-    confine "Missing 'pson' library" => Puppet.features.pson?
-
+  describe "pson", :if => Puppet.features.pson? do
     before do
       @pson = Puppet::Network::FormatHandler.format(:pson)
     end
diff --git a/spec/unit/network/http/compression_spec.rb b/spec/unit/network/http/compression_spec.rb
index b46941f46..c5bbbb064 100644
--- a/spec/unit/network/http/compression_spec.rb
+++ b/spec/unit/network/http/compression_spec.rb
@@ -37,9 +37,7 @@ describe "http compression" do
     end
   end
 
-  describe "when zlib is available" do
-    confine "Zlib is missing" => Puppet.features.zlib?
-
+  describe "when zlib is available", :if => Puppet.features.zlib? do
     before(:each) do
       Puppet.features.stubs(:zlib?).returns true
 
diff --git a/spec/unit/network/http/mongrel/rest_spec.rb b/spec/unit/network/http/mongrel/rest_spec.rb
index 92a81a10b..fb24521d5 100755
--- a/spec/unit/network/http/mongrel/rest_spec.rb
+++ b/spec/unit/network/http/mongrel/rest_spec.rb
@@ -4,8 +4,7 @@ require File.dirname(__FILE__) + '/../../../../spec_helper'
 
 require 'puppet/network/http'
 
-describe "Puppet::Network::HTTP::MongrelREST" do
-  confine "Mongrel is not available" => Puppet.features.mongrel?
+describe "Puppet::Network::HTTP::MongrelREST", :if => Puppet.features.mongrel? do
   before do
     require 'puppet/network/http/mongrel/rest'
   end
diff --git a/spec/unit/network/http/mongrel_spec.rb b/spec/unit/network/http/mongrel_spec.rb
index ac3d72bae..1e24be0c6 100755
--- a/spec/unit/network/http/mongrel_spec.rb
+++ b/spec/unit/network/http/mongrel_spec.rb
@@ -6,9 +6,7 @@
 require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/network/http'
 
-describe "Puppet::Network::HTTP::Mongrel", "after initializing" do
-  confine "Mongrel is not available" => Puppet.features.mongrel?
-
+describe "Puppet::Network::HTTP::Mongrel", "after initializing", :if => Puppet.features.mongrel? do
   it "should not be listening" do
     require 'puppet/network/http/mongrel'
 
@@ -16,9 +14,7 @@ describe "Puppet::Network::HTTP::Mongrel", "after initializing" do
   end
 end
 
-describe "Puppet::Network::HTTP::Mongrel", "when turning on listening" do
-  confine "Mongrel is not available" => Puppet.features.mongrel?
-
+describe "Puppet::Network::HTTP::Mongrel", "when turning on listening", :if => Puppet.features.mongrel? do
   before do
     require 'puppet/network/http/mongrel'
 
@@ -100,9 +96,7 @@ describe "Puppet::Network::HTTP::Mongrel", "when turning on listening" do
   end
 end
 
-describe "Puppet::Network::HTTP::Mongrel", "when turning off listening" do
-  confine "Mongrel is not available" => Puppet.features.mongrel?
-
+describe "Puppet::Network::HTTP::Mongrel", "when turning off listening", :if => Puppet.features.mongrel? do
   before do
     @mock_mongrel = mock('mongrel httpserver')
     @mock_mongrel.stubs(:run)
diff --git a/spec/unit/network/http/rack/rest_spec.rb b/spec/unit/network/http/rack/rest_spec.rb
index 96cf84c37..3eed4a2cb 100755
--- a/spec/unit/network/http/rack/rest_spec.rb
+++ b/spec/unit/network/http/rack/rest_spec.rb
@@ -4,9 +4,7 @@ require File.dirname(__FILE__) + '/../../../../spec_helper'
 require 'puppet/network/http/rack' if Puppet.features.rack?
 require 'puppet/network/http/rack/rest'
 
-describe "Puppet::Network::HTTP::RackREST" do
-  confine "Rack is not available" => Puppet.features.rack?
-
+describe "Puppet::Network::HTTP::RackREST", :if => Puppet.features.rack? do
   it "should include the Puppet::Network::HTTP::Handler module" do
     Puppet::Network::HTTP::RackREST.ancestors.should be_include(Puppet::Network::HTTP::Handler)
   end
diff --git a/spec/unit/network/http/rack/xmlrpc_spec.rb b/spec/unit/network/http/rack/xmlrpc_spec.rb
index 870438f2c..e6411524e 100755
--- a/spec/unit/network/http/rack/xmlrpc_spec.rb
+++ b/spec/unit/network/http/rack/xmlrpc_spec.rb
@@ -5,9 +5,7 @@ require 'puppet/network/handler'
 require 'puppet/network/http/rack' if Puppet.features.rack?
 require 'puppet/network/http/rack/xmlrpc' if Puppet.features.rack?
 
-describe "Puppet::Network::HTTP::RackXMLRPC" do
-  confine "Rack is not available" => Puppet.features.rack?
-
+describe "Puppet::Network::HTTP::RackXMLRPC", :if => Puppet.features.rack? do
   describe "when initializing" do
     it "should create an Puppet::Network::XMLRPCServer" do
       Puppet::Network::XMLRPCServer.expects(:new).returns stub_everything
diff --git a/spec/unit/network/http/rack_spec.rb b/spec/unit/network/http/rack_spec.rb
index 8be9ccb50..434294ce8 100755
--- a/spec/unit/network/http/rack_spec.rb
+++ b/spec/unit/network/http/rack_spec.rb
@@ -4,9 +4,7 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/network/handler'
 require 'puppet/network/http/rack' if Puppet.features.rack?
 
-describe "Puppet::Network::HTTP::Rack" do
-  confine "Rack is not available" => Puppet.features.rack?
-
+describe "Puppet::Network::HTTP::Rack", :if => Puppet.features.rack? do
   describe "while initializing" do
 
     it "should require a protocol specification" do
diff --git a/spec/unit/parser/collector_spec.rb b/spec/unit/parser/collector_spec.rb
index 908cda63a..4cab26c44 100755
--- a/spec/unit/parser/collector_spec.rb
+++ b/spec/unit/parser/collector_spec.rb
@@ -263,9 +263,7 @@ describe Puppet::Parser::Collector, "when collecting virtual and catalog resourc
   end
 end
 
-describe Puppet::Parser::Collector, "when collecting exported resources" do
-  confine "Cannot test Rails integration without ActiveRecord" => Puppet.features.rails?
-
+describe Puppet::Parser::Collector, "when collecting exported resources", :if => Puppet.features.rails? do
   before do
     @compiler = Puppet::Parser::Compiler.new(Puppet::Node.new("mynode"))
     @scope = Puppet::Parser::Scope.new :compiler => @compiler
@@ -469,9 +467,7 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
   end
 end
 
-describe Puppet::Parser::Collector, "when building its ActiveRecord query for collecting exported resources" do
-  confine "Cannot test Rails integration without ActiveRecord" => Puppet.features.rails?
-
+describe Puppet::Parser::Collector, "when building its ActiveRecord query for collecting exported resources", :if => Puppet.features.rails? do
   before do
     @scope = stub 'scope', :host => "myhost", :debug => nil
     @compiler = mock 'compile'
diff --git a/spec/unit/parser/lexer_spec.rb b/spec/unit/parser/lexer_spec.rb
index d52add399..860326973 100755
--- a/spec/unit/parser/lexer_spec.rb
+++ b/spec/unit/parser/lexer_spec.rb
@@ -5,7 +5,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/parser/lexer'
 
 # This is a special matcher to match easily lexer output
-Spec::Matchers.define :be_like do |*expected|
+RSpec::Matchers.define :be_like do |*expected|
   match do |actual|
     expected.zip(actual).all? { |e,a| !e or a[0] == e or (e.is_a? Array and a[0] == e[0] and (a[1] == e[1] or (a[1].is_a?(Hash) and a[1][:value] == e[1]))) }
   end
@@ -651,7 +651,8 @@ describe "Puppet::Parser::Lexer in the old tests" do
   end
 end
 
-require 'puppettest/support/utils'
+require File.dirname(__FILE__) + '/../../../test/lib/puppettest'
+require File.dirname(__FILE__) + '/../../../test/lib/puppettest/support/utils'
 describe "Puppet::Parser::Lexer in the old tests when lexing example files" do
   extend PuppetTest::Support::Utils
   textfiles do |file|
diff --git a/spec/unit/provider/mount/parsed_spec.rb b/spec/unit/provider/mount/parsed_spec.rb
index 7d2e8a84c..5a1c986b1 100755
--- a/spec/unit/provider/mount/parsed_spec.rb
+++ b/spec/unit/provider/mount/parsed_spec.rb
@@ -157,8 +157,7 @@ describe provider_class do
     end
   end
 
-  describe provider_class, " when parsing information about the root filesystem" do
-    confine "Mount type not tested on Darwin" => Facter["operatingsystem"].value != "Darwin"
+  describe provider_class, " when parsing information about the root filesystem", :if => Facter["operatingsystem"].value != "Darwin" do
     include ParsedMountTesting
 
     before do
diff --git a/spec/unit/rails/host_spec.rb b/spec/unit/rails/host_spec.rb
index 324a673a9..4244f117f 100755
--- a/spec/unit/rails/host_spec.rb
+++ b/spec/unit/rails/host_spec.rb
@@ -2,9 +2,7 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-describe "Puppet::Rails::Host" do
-  confine "Cannot test without ActiveRecord" => Puppet.features.rails?
-
+describe "Puppet::Rails::Host", :if => Puppet.features.rails? do
   def column(name, type)
     ActiveRecord::ConnectionAdapters::Column.new(name, nil, type, false)
   end
diff --git a/spec/unit/rails/param_value_spec.rb b/spec/unit/rails/param_value_spec.rb
index 243456e89..f67022a14 100755
--- a/spec/unit/rails/param_value_spec.rb
+++ b/spec/unit/rails/param_value_spec.rb
@@ -3,9 +3,7 @@
 require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/rails'
 
-describe "Puppet::Rails::ParamValue" do
-  confine "Cannot test without ActiveRecord" => Puppet.features.rails?
-
+describe "Puppet::Rails::ParamValue", :if => Puppet.features.rails? do
   def column(name, type)
     ActiveRecord::ConnectionAdapters::Column.new(name, nil, type, false)
   end
diff --git a/spec/unit/rails/resource_spec.rb b/spec/unit/rails/resource_spec.rb
index 6e23d2020..e5bd8a6c9 100755
--- a/spec/unit/rails/resource_spec.rb
+++ b/spec/unit/rails/resource_spec.rb
@@ -3,9 +3,7 @@
 require File.dirname(__FILE__) + '/../../spec_helper'
 require 'puppet/rails'
 
-describe "Puppet::Rails::Resource" do
-  confine "Cannot test without ActiveRecord" => Puppet.features.rails?
-
+describe "Puppet::Rails::Resource", :if => Puppet.features.rails? do
   def column(name, type)
     ActiveRecord::ConnectionAdapters::Column.new(name, nil, type, false)
   end
diff --git a/spec/unit/rails_spec.rb b/spec/unit/rails_spec.rb
index 24248e622..02b54efef 100755
--- a/spec/unit/rails_spec.rb
+++ b/spec/unit/rails_spec.rb
@@ -3,9 +3,7 @@
 require File.dirname(__FILE__) + '/../spec_helper'
 require 'puppet/rails'
 
-describe Puppet::Rails, "when initializing any connection" do
-  confine "Cannot test without ActiveRecord" => Puppet.features.rails?
-
+describe Puppet::Rails, "when initializing any connection", :if => Puppet.features.rails? do
   before do
     Puppet.settings.stubs(:use)
     @logger = mock 'logger'
@@ -77,9 +75,7 @@ describe Puppet::Rails, "when initializing any connection" do
   end
 end
 
-describe Puppet::Rails, "when initializing a sqlite3 connection" do
-  confine "Cannot test without ActiveRecord" => Puppet.features.rails?
-
+describe Puppet::Rails, "when initializing a sqlite3 connection", :if => Puppet.features.rails? do
   it "should provide the adapter, log_level, and database arguments" do
     Puppet.settings.expects(:value).with(:dbadapter).returns("sqlite3")
     Puppet.settings.expects(:value).with(:rails_loglevel).returns("testlevel")
@@ -93,9 +89,7 @@ describe Puppet::Rails, "when initializing a sqlite3 connection" do
   end
 end
 
-describe Puppet::Rails, "when initializing a mysql connection" do
-  confine "Cannot test without ActiveRecord" => Puppet.features.rails?
-
+describe Puppet::Rails, "when initializing a mysql connection", :if => Puppet.features.rails? do
   it "should provide the adapter, log_level, and host, port, username, password, database, and reconnect arguments" do
     Puppet.settings.stubs(:value).with(:dbadapter).returns("mysql")
     Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
@@ -190,9 +184,7 @@ describe Puppet::Rails, "when initializing a mysql connection" do
   end
 end
 
-describe Puppet::Rails, "when initializing a postgresql connection" do
-  confine "Cannot test without ActiveRecord" => Puppet.features.rails?
-
+describe Puppet::Rails, "when initializing a postgresql connection", :if => Puppet.features.rails? do
   it "should provide the adapter, log_level, and host, port, username, password, connections, and database arguments" do
     Puppet.settings.stubs(:value).with(:dbadapter).returns("postgresql")
     Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
@@ -263,9 +255,7 @@ describe Puppet::Rails, "when initializing a postgresql connection" do
   end
 end
 
-describe Puppet::Rails, "when initializing an Oracle connection" do
-  confine "Cannot test without ActiveRecord" => Puppet.features.rails?
-
+describe Puppet::Rails, "when initializing an Oracle connection", :if => Puppet.features.rails? do
   it "should provide the adapter, log_level, and username, password, and database arguments" do
     Puppet.settings.stubs(:value).with(:dbadapter).returns("oracle_enhanced")
     Puppet.settings.stubs(:value).with(:rails_loglevel).returns("testlevel")
diff --git a/spec/unit/relationship_spec.rb b/spec/unit/relationship_spec.rb
index 4586cd0f3..9ce6c56e7 100755
--- a/spec/unit/relationship_spec.rb
+++ b/spec/unit/relationship_spec.rb
@@ -155,9 +155,7 @@ describe Puppet::Relationship, " when matching edges with a non-standard event"
   end
 end
 
-describe Puppet::Relationship, "when converting to pson" do
-  confine "Missing 'pson' library" => Puppet.features.pson?
-
+describe Puppet::Relationship, "when converting to pson", :if => Puppet.features.pson? do
   before do
     @edge = Puppet::Relationship.new(:a, :b, :event => :random, :callback => :whatever)
   end
@@ -190,9 +188,7 @@ describe Puppet::Relationship, "when converting to pson" do
   end
 end
 
-describe Puppet::Relationship, "when converting from pson" do
-  confine "Missing 'pson' library" => Puppet.features.pson?
-
+describe Puppet::Relationship, "when converting from pson", :if => Puppet.features.pson? do
   before do
     @event = "random"
     @callback = "whatever"
diff --git a/spec/unit/resource/catalog_spec.rb b/spec/unit/resource/catalog_spec.rb
index 2b6beb5e9..942721464 100755
--- a/spec/unit/resource/catalog_spec.rb
+++ b/spec/unit/resource/catalog_spec.rb
@@ -880,9 +880,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
   end
 end
 
-describe Puppet::Resource::Catalog, "when converting to pson" do
-  confine "Missing 'pson' library" => Puppet.features.pson?
-
+describe Puppet::Resource::Catalog, "when converting to pson", :if => Puppet.features.pson? do
   before do
     @catalog = Puppet::Resource::Catalog.new("myhost")
   end
@@ -940,9 +938,7 @@ describe Puppet::Resource::Catalog, "when converting to pson" do
   end
 end
 
-describe Puppet::Resource::Catalog, "when converting from pson" do
-  confine "Missing 'pson' library" => Puppet.features.pson?
-
+describe Puppet::Resource::Catalog, "when converting from pson", :if => Puppet.features.pson? do
   def pson_result_should
     Puppet::Resource::Catalog.expects(:new).with { |hash| yield hash }
   end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index e65e8a13a..877b6b6b0 100755
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -585,9 +585,7 @@ describe Puppet::Resource do
     end
   end
 
-  describe "when converting to pson" do
-    confine "Missing 'pson' library" => Puppet.features.pson?
-
+  describe "when converting to pson", :if => Puppet.features.pson? do
     def pson_output_should
       @resource.class.expects(:pson_create).with { |hash| yield hash }
     end
@@ -666,9 +664,7 @@ describe Puppet::Resource do
     end
   end
 
-  describe "when converting from pson" do
-    confine "Missing 'pson' library" => Puppet.features.pson?
-
+  describe "when converting from pson", :if => Puppet.features.pson? do
     def pson_result_should
       Puppet::Resource.expects(:new).with { |hash| yield hash }
     end
diff --git a/spec/unit/ssl/certificate_authority/interface_spec.rb b/spec/unit/ssl/certificate_authority/interface_spec.rb
index d8c351ae2..5cf4073df 100755
--- a/spec/unit/ssl/certificate_authority/interface_spec.rb
+++ b/spec/unit/ssl/certificate_authority/interface_spec.rb
@@ -4,7 +4,7 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppet/ssl/certificate_authority'
 
-describe "a normal interface method", :shared => true do
+shared_examples_for "a normal interface method" do
   it "should call the method on the CA for each host specified if an array was provided" do
     @ca.expects(@method).with("host1")
     @ca.expects(@method).with("host2")
diff --git a/spec/unit/type/augeas_spec.rb b/spec/unit/type/augeas_spec.rb
index e426fbeed..d2e40f0f6 100644
--- a/spec/unit/type/augeas_spec.rb
+++ b/spec/unit/type/augeas_spec.rb
@@ -5,9 +5,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 augeas = Puppet::Type.type(:augeas)
 
 describe augeas do
-  describe "when augeas is present" do
-    confine "Augeas is unavailable" => Puppet.features.augeas?
-
+  describe "when augeas is present", :if => Puppet.features.augeas? do
     it "should have a default provider inheriting from Puppet::Provider" do
       augeas.defaultprovider.ancestors.should be_include(Puppet::Provider)
     end
diff --git a/spec/unit/type/file_spec.rb b/spec/unit/type/file_spec.rb
index 4fcad07e1..22921d85a 100755
--- a/spec/unit/type/file_spec.rb
+++ b/spec/unit/type/file_spec.rb
@@ -208,8 +208,7 @@ describe Puppet::Type.type(:file) do
     end
   end
 
-  describe "when using Microsoft Windows filenames" do
-    confine "Only works on Microsoft Windows" => Puppet.features.microsoft_windows?
+  describe "when using Microsoft Windows filenames", :if => Puppet.features.microsoft_windows? do
     describe "on Microsoft Windows systems" do
       before do
         Puppet.features.stubs(:posix?).returns(false)
@@ -271,8 +270,7 @@ describe Puppet::Type.type(:file) do
   end
 
   describe "when using UNC filenames" do
-    describe "on Microsoft Windows systems" do
-      confine "Only works on Microsoft Windows" => Puppet.features.microsoft_windows?
+    describe "on Microsoft Windows systems", :if => Puppet.features.microsoft_windows? do
       before do
         Puppet.features.stubs(:posix?).returns(false)
         Puppet.features.stubs(:microsoft_windows?).returns(true)
@@ -381,9 +379,9 @@ describe Puppet::Type.type(:file) do
 
 
                 @resource = Puppet::Type.type(:file).new(
-                
+
             :path => @link,
-        
+
             :mode => "755"
           )
           @catalog.add_resource @resource
@@ -1058,7 +1056,7 @@ describe Puppet::Type.type(:file) do
     before do
       @type_class = Puppet::Type.type(:file)
     end
-    
+
     it "should have a regexp that captures the entire string, except for a terminating slash" do
       patterns = @type_class.title_patterns
       string = "abc/\n\tdef/"
diff --git a/spec/unit/util/queue/stomp_spec.rb b/spec/unit/util/queue/stomp_spec.rb
index 9f1d28448..c33f1a670 100755
--- a/spec/unit/util/queue/stomp_spec.rb
+++ b/spec/unit/util/queue/stomp_spec.rb
@@ -3,18 +3,14 @@
 require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/util/queue'
 
-describe Puppet::Util::Queue do
-  confine "Missing Stomp" => Puppet.features.stomp?
-
+describe Puppet::Util::Queue, :if => Puppet.features.stomp? do
   it 'should load :stomp client appropriately' do
     Puppet.settings.stubs(:value).returns 'faux_queue_source'
     Puppet::Util::Queue.queue_type_to_class(:stomp).name.should == 'Puppet::Util::Queue::Stomp'
   end
 end
 
-describe 'Puppet::Util::Queue::Stomp' do
-  confine "Missing Stomp" => Puppet.features.stomp?
-
+describe 'Puppet::Util::Queue::Stomp', :if => Puppet.features.stomp? do
   before do
     # So we make sure we never create a real client instance.
     # Otherwise we'll try to connect, and that's bad.
diff --git a/spec/unit/util/settings/file_setting_spec.rb b/spec/unit/util/settings/file_setting_spec.rb
index 2870fbb57..dcfb6e3b1 100755
--- a/spec/unit/util/settings/file_setting_spec.rb
+++ b/spec/unit/util/settings/file_setting_spec.rb
@@ -146,9 +146,7 @@ describe Puppet::Util::Settings::FileSetting do
       @file.to_resource.should be_instance_of(Puppet::Resource)
     end
 
-    describe "on POSIX systems" do
-      confine "no /dev on Microsoft Windows" => Puppet.features.posix?
-
+    describe "on POSIX systems", :if => Puppet.features.posix? do
       it "should skip files in /dev" do
         @settings.stubs(:value).with(:mydir).returns "/dev/file"
         @file.to_resource.should be_nil
-- 
cgit 


From 717670fa3a6f471c8183dd85443ca2426f4df238 Mon Sep 17 00:00:00 2001
From: Nick Lewis <nick@puppetlabs.com>
Date: Wed, 5 Jan 2011 15:41:31 -0800
Subject: (#5771): Fix spec failures associated with rspec upgrade

Due to changes in the spec_helper, some of the specs that use puppettest were
failing when run individually. In the future, it would be nice to remove
puppettest from the specs entirely, as it's old, crufty, and only used for a
couple of things.

Paired-With: Matt Robinson
---
 .../integration/provider/mailalias/aliases_spec.rb |  1 -
 spec/spec_helper.rb                                |  4 +
 spec/spec_specs/runnable_spec.rb                   | 95 ----------------------
 test/lib/puppettest/fileparsing.rb                 |  2 +
 4 files changed, 6 insertions(+), 96 deletions(-)
 delete mode 100644 spec/spec_specs/runnable_spec.rb

diff --git a/spec/integration/provider/mailalias/aliases_spec.rb b/spec/integration/provider/mailalias/aliases_spec.rb
index 0511205f2..1bce13f90 100755
--- a/spec/integration/provider/mailalias/aliases_spec.rb
+++ b/spec/integration/provider/mailalias/aliases_spec.rb
@@ -8,7 +8,6 @@ require 'puppettest/fileparsing'
 provider_class = Puppet::Type.type(:mailalias).provider(:aliases)
 
 describe provider_class do
-  include PuppetTest
   include PuppetTest::FileParsing
   include PuppetTest::Support::Utils
 
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index d0ee7d92a..ed4e826c9 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -6,6 +6,7 @@ dir = File.expand_path(File.dirname(__FILE__))
 $LOAD_PATH.unshift("#{dir}/")
 $LOAD_PATH.unshift("#{dir}/lib") # a spec-specific test lib dir
 $LOAD_PATH.unshift("#{dir}/../lib")
+$LOAD_PATH.unshift("#{dir}/../test/lib")
 
 # Don't want puppet getting the command line arguments for rake or autotest
 ARGV.clear
@@ -19,6 +20,9 @@ module PuppetSpec
   FIXTURE_DIR = File.join(dir = File.expand_path(File.dirname(__FILE__)), "fixtures") unless defined?(FIXTURE_DIR)
 end
 
+module PuppetTest
+end
+
 require 'lib/puppet_spec/files'
 require 'monkey_patches/alias_should_to_must'
 require 'monkey_patches/publicize_methods'
diff --git a/spec/spec_specs/runnable_spec.rb b/spec/spec_specs/runnable_spec.rb
deleted file mode 100644
index da4faca4e..000000000
--- a/spec/spec_specs/runnable_spec.rb
+++ /dev/null
@@ -1,95 +0,0 @@
-require File.dirname(__FILE__) + '/../spec_helper'
-
-describe PuppetTest::RunnableTest do
-  before do
-    @runnable_test = Class.new.extend(PuppetTest::RunnableTest)
-  end
-
-  describe "#confine" do
-    subject { @runnable_test }
-
-    it "should accept a hash" do
-      subject.confine({}).should_not raise_error(ArgumentError)
-    end
-
-    it "should accept a message and a block" do
-      subject.confine(""){}.should_not raise_error(ArgumentError)
-    end
-
-  end
-
-  describe "#runnable?" do
-    describe "when the superclass is not runnable" do
-      before { @runnable_test.stubs(:superclass).returns(stub("unrunnable superclass", :runnable? => false)) }
-      subject { @runnable_test.runnable? }
-
-      it { should be_false }
-    end
-
-    describe "when a confine is false" do
-      before { @runnable_test.confine(:message => false) }
-      subject { @runnable_test.runnable? }
-
-      it { should be_false }
-    end
-
-    describe "when a confine has a block that returns false" do
-      before { @runnable_test.confine(:message){ false } }
-      subject { @runnable_test.runnable? }
-
-      it { should be_false }
-    end
-
-    describe "when a confine is true and no false confines" do
-      before { @runnable_test.confine(:message => true) }
-      subject { @runnable_test.runnable? }
-
-      it { should be_true }
-    end
-
-    describe "when a confine has block that returns true and no false confines" do
-      before { @runnable_test.confine(:message){ true } }
-      subject { @runnable_test.runnable? }
-
-      it { should be_true }
-    end
-
-  end
-
-  describe "#messages" do
-    describe "before runnable? is called" do
-      subject { @runnable_test.messages }
-
-      it { should == [] }
-    end
-
-    describe "when runnable? is called and returns false" do
-      before do
-        @runnable_test.confine(:message => false)
-        @runnable_test.runnable?
-      end
-
-      subject { @runnable_test.messages }
-
-      it "should include the failed confine's message" do
-        should include(:message)
-      end
-
-    end
-
-    describe "when runnable? is called whose block returns false" do
-      before do
-        @runnable_test.confine(:message){ false }
-        @runnable_test.runnable?
-      end
-
-      subject { @runnable_test.messages }
-
-      it "should include the failed confine's message" do
-        should include(:message)
-      end
-
-    end
-
-  end
-end
diff --git a/test/lib/puppettest/fileparsing.rb b/test/lib/puppettest/fileparsing.rb
index 914c4bcb3..bd4f9e152 100644
--- a/test/lib/puppettest/fileparsing.rb
+++ b/test/lib/puppettest/fileparsing.rb
@@ -1,3 +1,5 @@
+require 'test/unit'
+
 module PuppetTest::FileParsing
   # Run an isomorphism test on our parsing process.
   def fakedataparse(*files)
-- 
cgit