Splitting the Agent class into Agent and Configurer

Once I went to add runinterval support to the Agent class,
I realized it's really two classes:  One that handles starting,
stopping, running, et al (still called Agent), and one that
handles downloading the catalog, running it, etc. (now
called Configurer).

This commit includes some additional code, but 95% of it is just moving code around.

Signed-off-by: Luke Kanies <luke@madstop.com>

6 files changed, 440 insertions, 256 deletions

diff --git a/spec/unit/agent.rb b/spec/unit/agent.rb
index c4f5793d2..5622af4a6 100755
--- a/spec/unit/agent.rb
+++ b/spec/unit/agent.rb
@@ -6,255 +6,200 @@
 require File.dirname(__FILE__) + '/../spec_helper'
 require 'puppet/agent'
 
-describe Puppet::Agent do
-    it "should include the Plugin Handler module" do
-        Puppet::Agent.ancestors.should be_include(Puppet::Agent::PluginHandler)
+class AgentTestClient
+    def run
+        # no-op
     end
-
-    it "should include the Fact Handler module" do
-        Puppet::Agent.ancestors.should be_include(Puppet::Agent::FactHandler)
-    end
-
-    it "should include the Locker module" do
-        Puppet::Agent.ancestors.should be_include(Puppet::Agent::Locker)
+    def stop
+        # no-op
     end
 end
 
-describe Puppet::Agent, "when executing a catalog run" do
+describe Puppet::Agent do
     before do
-        Puppet.settings.stubs(:use).returns(true)
-        @agent = Puppet::Agent.new
-        @agent.stubs(:splay)
-        @agent.stubs(:lock).yields.then.returns true
-    end
+        @agent = Puppet::Agent.new(AgentTestClient)
 
-    it "should splay" do
-        Puppet::Util.sync(:puppetrun).stubs(:synchronize)
-        @agent.expects(:splay)
-        @agent.run
+        # So we don't actually try to hit the filesystem.
+        @agent.stubs(:lock).yields
     end
 
-    it "should use a global mutex to make sure no other thread is executing the catalog" do
-        sync = mock 'sync'
-        Puppet::Util.expects(:sync).with(:puppetrun).returns sync
-
-        sync.expects(:synchronize)
-
-        @agent.expects(:retrieve_config).never # i.e., if we don't yield, we don't retrieve the config
-        @agent.run
+    it "should set its client class at initialization" do
+        Puppet::Agent.new("foo").client_class.should == "foo"
     end
 
-    it "should retrieve the catalog if a lock is attained" do
-        @agent.expects(:lock).yields.then.returns true
-
-        @agent.expects(:retrieve_catalog)
-
-        @agent.run
-    end
-
-    it "should log and do nothing if the lock cannot be acquired" do
-        @agent.expects(:lock).returns false
-
-        @agent.expects(:retrieve_catalog).never
-
-        Puppet.expects(:notice)
-
-        @agent.run
-    end
-
-    it "should retrieve the catalog" do
-        @agent.expects(:retrieve_catalog)
-
-        @agent.run
+    it "should include the Locker module" do
+        Puppet::Agent.ancestors.should be_include(Puppet::Agent::Locker)
     end
 
-    it "should log a failure and do nothing if no catalog can be retrieved" do
-        @agent.expects(:retrieve_catalog).returns nil
+    it "should create an instance of its client class and run it when asked to run" do
+        client = mock 'client'
+        AgentTestClient.expects(:new).returns client
 
-        Puppet.expects(:err)
+        client.expects(:run)
 
         @agent.run
     end
 
-    it "should apply the catalog with all options to :run" do
-        catalog = stub 'catalog', :retrieval_duration= => nil
-        @agent.expects(:retrieve_catalog).returns catalog
-
-        catalog.expects(:apply).with(:one => true)
-        @agent.run :one => true
+    it "should determine its lock file path by asking the client class" do
+        AgentTestClient.expects(:lockfile_path).returns "/my/lock"
+        @agent.lockfile_path.should == "/my/lock"
+    end
+
+    describe "when running" do
+        it "should splay" do
+            @agent.expects(:splay)
+
+            @agent.run
+        end
+
+        it "should do nothing if a client instance exists" do
+            @agent.expects(:client).returns "eh"
+            AgentTestClient.expects(:new).never
+            @agent.run
+        end
+
+        it "should do nothing if it is in the process of stopping" do
+            @agent.expects(:stopping?).returns true
+            AgentTestClient.expects(:new).never
+            @agent.run
+        end
+
+        it "should not fail if a client class instance cannot be created" do
+            AgentTestClient.expects(:new).raises "eh"
+            Puppet.expects(:err)
+            @agent.run
+        end
+
+        it "should not fail if there is an exception while running its client" do
+            client = AgentTestClient.new
+            AgentTestClient.expects(:new).returns client
+            client.expects(:run).raises "eh"
+            Puppet.expects(:err)
+            @agent.run
+        end
+
+        it "should use a mutex to restrict multi-threading" do
+            client = AgentTestClient.new
+            AgentTestClient.expects(:new).returns client
+
+            mutex = mock 'mutex'
+            @agent.expects(:sync).returns mutex
+
+            mutex.expects(:synchronize)
+            client.expects(:run).never # if it doesn't run, then we know our yield is what triggers it
+            @agent.run
+        end
+
+        it "should use a filesystem lock to restrict multiple processes running the agent" do
+            client = AgentTestClient.new
+            AgentTestClient.expects(:new).returns client
+
+            @agent.expects(:lock)
+
+            client.expects(:run).never # if it doesn't run, then we know our yield is what triggers it
+            @agent.run
+        end
+
+        it "should make its client instance available while running" do
+            client = AgentTestClient.new
+            AgentTestClient.expects(:new).returns client
+
+            client.expects(:run).with { @agent.client.should equal(client); true }
+            @agent.run
+        end
+    end
+
+    describe "when splaying" do
+        before do
+            Puppet.settings.stubs(:value).with(:splay).returns true
+            Puppet.settings.stubs(:value).with(:splaylimit).returns "10"
+        end
+
+        it "should do nothing if splay is disabled" do
+            Puppet.settings.expects(:value).returns false
+            @agent.expects(:sleep).never
+            @agent.splay
+        end
+
+        it "should do nothing if it has already splayed" do
+            @agent.expects(:splayed?).returns true
+            @agent.expects(:sleep).never
+            @agent.splay
+        end
+
+        it "should log that it is splaying" do
+            @agent.stubs :sleep
+            Puppet.expects :info
+            @agent.splay
+        end
+
+        it "should sleep for a random portion of the splaylimit plus 1" do
+            Puppet.settings.expects(:value).with(:splaylimit).returns "50"
+            @agent.expects(:rand).with(51).returns 10
+            @agent.expects(:sleep).with(10)
+            @agent.splay
+        end
+
+        it "should mark that it has splayed" do
+            @agent.stubs(:sleep)
+            @agent.splay
+            @agent.should be_splayed
+        end
     end
     
-    it "should benchmark how long it takes to apply the catalog" do
-        @agent.expects(:benchmark).with(:notice, "Finished catalog run")
-
-        catalog = stub 'catalog', :retrieval_duration= => nil
-        @agent.expects(:retrieve_catalog).returns catalog
-
-        catalog.expects(:apply).never # because we're not yielding
-        @agent.run
-    end
-
-    it "should HUP itself if it should be restarted" do
-        catalog = stub 'catalog', :retrieval_duration= => nil, :apply => nil
-        @agent.expects(:retrieve_catalog).returns catalog
-
-        Process.expects(:kill).with(:HUP, $$)
-
-        @agent.expects(:restart?).returns true
-
-        @agent.run
-    end
-
-    it "should not HUP itself if it should not be restarted" do
-        catalog = stub 'catalog', :retrieval_duration= => nil, :apply => nil
-        @agent.expects(:retrieve_catalog).returns catalog
-
-        Process.expects(:kill).never
-
-        @agent.expects(:restart?).returns false
-
-        @agent.run
-    end
-end
-
-describe Puppet::Agent, "when retrieving a catalog" do
-    before do
-        Puppet.settings.stubs(:use).returns(true)
-        @agent = Puppet::Agent.new
-
-        @catalog = Puppet::Resource::Catalog.new
-
-        @agent.stubs(:convert_catalog).returns @catalog
-    end
-
-    it "should use the Catalog class to get its catalog" do
-        Puppet::Resource::Catalog.expects(:find).returns @catalog
-
-        @agent.retrieve_catalog
-    end
-
-    it "should use its Facter name to retrieve the catalog" do
-        Facter.stubs(:value).returns "eh"
-        Facter.expects(:value).with("hostname").returns "myhost"
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| name == "myhost" }.returns @catalog
-
-        @agent.retrieve_catalog
-    end
-
-    it "should default to returning a catalog retrieved directly from the server, skipping the cache" do
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.returns @catalog
-
-        @agent.retrieve_catalog.should == @catalog
-    end
-
-    it "should return the cached catalog when no catalog can be retrieved from the server" do
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.returns nil
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == true }.returns @catalog
-
-        @agent.retrieve_catalog.should == @catalog
-    end
-
-    it "should not look in the cache for a catalog if one is returned from the server" do
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.returns @catalog
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == true }.never
-
-        @agent.retrieve_catalog.should == @catalog
-    end
-
-    it "should return the cached catalog when retrieving the remote catalog throws an exception" do
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.raises "eh"
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == true }.returns @catalog
-
-        @agent.retrieve_catalog.should == @catalog
-    end
-
-    it "should return nil if no cached catalog is available and no catalog can be retrieved from the server" do
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.returns nil
-        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == true }.returns nil
-
-        @agent.retrieve_catalog.should be_nil
-    end
-
-    it "should convert the catalog before returning" do
-        Puppet::Resource::Catalog.stubs(:find).returns @catalog
-
-        @agent.expects(:convert_catalog).with { |cat, dur| cat == @catalog }.returns "converted catalog"
-        @agent.retrieve_catalog.should == "converted catalog"
-    end
-
-    it "should return nil if there is an error while retrieving the catalog" do
-        Puppet::Resource::Catalog.expects(:find).raises "eh"
-
-        @agent.retrieve_catalog.should be_nil
-    end
-end
-
-describe Puppet::Agent, "when converting the catalog" do
-    before do
-        Puppet.settings.stubs(:use).returns(true)
-        @agent = Puppet::Agent.new
-
-        @catalog = Puppet::Resource::Catalog.new
-        @oldcatalog = stub 'old_catalog', :to_ral => @catalog
-    end
-
-    it "should convert the catalog to a RAL-formed catalog" do
-        @oldcatalog.expects(:to_ral).returns @catalog
-
-        @agent.convert_catalog(@oldcatalog, 10).should equal(@catalog)
-    end
-
-    it "should record the passed retrieval time with the RAL catalog" do
-        @catalog.expects(:retrieval_duration=).with 10
-
-        @agent.convert_catalog(@oldcatalog, 10)
-    end
-
-    it "should write the RAL catalog's class file" do
-        @catalog.expects(:write_class_file)
-
-        @agent.convert_catalog(@oldcatalog, 10)
-    end
-
-    it "should mark the RAL catalog as a host catalog" do
-        @catalog.expects(:host_config=).with true
-
-        @agent.convert_catalog(@oldcatalog, 10)
-    end
-end
-
-describe Puppet::Agent, "when preparing for a run" do
-    before do
-        Puppet.settings.stubs(:use).returns(true)
-        @agent = Puppet::Agent.new
-        @agent.stubs(:dostorage)
-        @agent.stubs(:upload_facts)
-        @facts = {"one" => "two", "three" => "four"}
-    end
-
-    it "should initialize the metadata store" do
-        @agent.class.stubs(:facts).returns(@facts)
-        @agent.expects(:dostorage)
-        @agent.prepare
-    end
-
-    it "should download fact plugins" do
-        @agent.stubs(:dostorage)
-        @agent.expects(:download_fact_plugins)
-
-        @agent.prepare
-    end
-
-    it "should download plugins" do
-        @agent.stubs(:dostorage)
-        @agent.expects(:download_plugins)
-
-        @agent.prepare
-    end
-
-    it "should upload facts to use for catalog retrieval" do
-        @agent.stubs(:dostorage)
-        @agent.expects(:upload_facts)
-        @agent.prepare
+    describe "when shutting down" do
+        it "should do nothing if already stopping" do
+            @agent.expects(:stopping?).returns true
+            @agent.shutdown
+        end
+
+        it "should stop the client if one is available and it responds to 'stop'" do
+            client = AgentTestClient.new
+
+            @agent.stubs(:client).returns client
+            client.expects(:stop)
+            @agent.shutdown
+        end
+
+        it "should remove its pid file" do
+            @agent.expects(:rmpidfile)
+            @agent.shutdown
+        end
+
+        it "should mark itself as stopping while waiting for the client to stop" do
+            client = AgentTestClient.new
+
+            @agent.stubs(:client).returns client
+            client.expects(:stop).with { @agent.should be_stopping; true }
+
+            @agent.shutdown
+        end
+    end
+
+    describe "when starting" do
+        it "should create a timer with the runinterval, a tolerance of 1, and :start? set to true" do
+            Puppet.settings.expects(:value).with(:runinterval).returns 5
+            Puppet.expects(:newtimer).with(:interval => 5, :start? => true, :tolerance => 1)
+            @agent.stubs(:run)
+            @agent.start
+        end
+
+        it "should run once immediately" do
+            Puppet.stubs(:newtimer)
+            @agent.expects(:run)
+            @agent.start
+        end
+
+        it "should run within the block passed to the timer" do
+            Puppet.stubs(:newtimer).yields
+            @agent.expects(:run).times(2)
+            @agent.start
+        end
+
+        it "should run within the block passed to the timer" do
+            Puppet.stubs(:newtimer).yields
+            @agent.expects(:run).times(2)
+            @agent.start
+        end
     end
 end
diff --git a/spec/unit/agent/locker.rb b/spec/unit/agent/locker.rb
index aae7c0c7b..1477c824e 100755
--- a/spec/unit/agent/locker.rb
+++ b/spec/unit/agent/locker.rb
@@ -11,12 +11,21 @@ end
 describe Puppet::Agent::Locker do
     before do
         @locker = LockerTester.new
+        @locker.stubs(:lockfile_path).returns "/my/lock"
     end
 
     it "should use a Pidlock instance as its lockfile" do
         @locker.lockfile.should be_instance_of(Puppet::Util::Pidlock)
     end
 
+    it "should use 'lockfile_path' to determine its lockfile path" do
+        @locker.expects(:lockfile_path).returns "/my/lock"
+        lock = Puppet::Util::Pidlock.new("/my/lock")
+        Puppet::Util::Pidlock.expects(:new).with("/my/lock").returns lock
+
+        @locker.lockfile
+    end
+
     it "should reuse the same lock file each time" do
         @locker.lockfile.should equal(@locker.lockfile)
     end
@@ -83,4 +92,9 @@ describe Puppet::Agent::Locker do
 
         lambda { @locker.lock { raise "foo" } }.should raise_error(RuntimeError)
     end
+
+    it "should be considered running if the lockfile is locked" do
+        @locker.lockfile.expects(:locked?).returns true
+        @locker.should be_running
+    end
 end
diff --git a/spec/unit/configurer.rb b/spec/unit/configurer.rb
new file mode 100755
index 000000000..996d67282
--- /dev/null
+++ b/spec/unit/configurer.rb
@@ -0,0 +1,225 @@
+#!/usr/bin/env ruby
+#
+#  Created by Luke Kanies on 2007-11-12.
+#  Copyright (c) 2007. All rights reserved.
+
+require File.dirname(__FILE__) + '/../spec_helper'
+require 'puppet/configurer'
+
+describe Puppet::Configurer do
+    it "should include the Plugin Handler module" do
+        Puppet::Configurer.ancestors.should be_include(Puppet::Configurer::PluginHandler)
+    end
+
+    it "should include the Fact Handler module" do
+        Puppet::Configurer.ancestors.should be_include(Puppet::Configurer::FactHandler)
+    end
+
+    it "should use the puppetdlockfile as its lockfile path" do
+        Puppet.settings.expects(:value).with(:puppetdlockfile).returns("/my/lock")
+        Puppet::Configurer.lockfile_path.should == "/my/lock"
+    end
+end
+
+describe Puppet::Configurer, "when executing a catalog run" do
+    before do
+        Puppet.settings.stubs(:use).returns(true)
+        @agent = Puppet::Configurer.new
+    end
+
+    it "should retrieve the catalog" do
+        @agent.expects(:retrieve_catalog)
+
+        @agent.run
+    end
+
+    it "should log a failure and do nothing if no catalog can be retrieved" do
+        @agent.expects(:retrieve_catalog).returns nil
+
+        Puppet.expects(:err)
+
+        @agent.run
+    end
+
+    it "should apply the catalog with all options to :run" do
+        catalog = stub 'catalog', :retrieval_duration= => nil
+        @agent.expects(:retrieve_catalog).returns catalog
+
+        catalog.expects(:apply).with(:one => true)
+        @agent.run :one => true
+    end
+    
+    it "should benchmark how long it takes to apply the catalog" do
+        @agent.expects(:benchmark).with(:notice, "Finished catalog run")
+
+        catalog = stub 'catalog', :retrieval_duration= => nil
+        @agent.expects(:retrieve_catalog).returns catalog
+
+        catalog.expects(:apply).never # because we're not yielding
+        @agent.run
+    end
+
+    it "should HUP itself if it should be restarted" do
+        catalog = stub 'catalog', :retrieval_duration= => nil, :apply => nil
+        @agent.expects(:retrieve_catalog).returns catalog
+
+        Process.expects(:kill).with(:HUP, $$)
+
+        @agent.expects(:restart?).returns true
+
+        @agent.run
+    end
+
+    it "should not HUP itself if it should not be restarted" do
+        catalog = stub 'catalog', :retrieval_duration= => nil, :apply => nil
+        @agent.expects(:retrieve_catalog).returns catalog
+
+        Process.expects(:kill).never
+
+        @agent.expects(:restart?).returns false
+
+        @agent.run
+    end
+end
+
+describe Puppet::Configurer, "when retrieving a catalog" do
+    before do
+        Puppet.settings.stubs(:use).returns(true)
+        @agent = Puppet::Configurer.new
+
+        @catalog = Puppet::Resource::Catalog.new
+
+        @agent.stubs(:convert_catalog).returns @catalog
+    end
+
+    it "should use the Catalog class to get its catalog" do
+        Puppet::Resource::Catalog.expects(:find).returns @catalog
+
+        @agent.retrieve_catalog
+    end
+
+    it "should use its Facter name to retrieve the catalog" do
+        Facter.stubs(:value).returns "eh"
+        Facter.expects(:value).with("hostname").returns "myhost"
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| name == "myhost" }.returns @catalog
+
+        @agent.retrieve_catalog
+    end
+
+    it "should default to returning a catalog retrieved directly from the server, skipping the cache" do
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.returns @catalog
+
+        @agent.retrieve_catalog.should == @catalog
+    end
+
+    it "should return the cached catalog when no catalog can be retrieved from the server" do
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.returns nil
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == true }.returns @catalog
+
+        @agent.retrieve_catalog.should == @catalog
+    end
+
+    it "should not look in the cache for a catalog if one is returned from the server" do
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.returns @catalog
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == true }.never
+
+        @agent.retrieve_catalog.should == @catalog
+    end
+
+    it "should return the cached catalog when retrieving the remote catalog throws an exception" do
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.raises "eh"
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == true }.returns @catalog
+
+        @agent.retrieve_catalog.should == @catalog
+    end
+
+    it "should return nil if no cached catalog is available and no catalog can be retrieved from the server" do
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == false }.returns nil
+        Puppet::Resource::Catalog.expects(:find).with { |name, options| options[:use_cache] == true }.returns nil
+
+        @agent.retrieve_catalog.should be_nil
+    end
+
+    it "should convert the catalog before returning" do
+        Puppet::Resource::Catalog.stubs(:find).returns @catalog
+
+        @agent.expects(:convert_catalog).with { |cat, dur| cat == @catalog }.returns "converted catalog"
+        @agent.retrieve_catalog.should == "converted catalog"
+    end
+
+    it "should return nil if there is an error while retrieving the catalog" do
+        Puppet::Resource::Catalog.expects(:find).raises "eh"
+
+        @agent.retrieve_catalog.should be_nil
+    end
+end
+
+describe Puppet::Configurer, "when converting the catalog" do
+    before do
+        Puppet.settings.stubs(:use).returns(true)
+        @agent = Puppet::Configurer.new
+
+        @catalog = Puppet::Resource::Catalog.new
+        @oldcatalog = stub 'old_catalog', :to_ral => @catalog
+    end
+
+    it "should convert the catalog to a RAL-formed catalog" do
+        @oldcatalog.expects(:to_ral).returns @catalog
+
+        @agent.convert_catalog(@oldcatalog, 10).should equal(@catalog)
+    end
+
+    it "should record the passed retrieval time with the RAL catalog" do
+        @catalog.expects(:retrieval_duration=).with 10
+
+        @agent.convert_catalog(@oldcatalog, 10)
+    end
+
+    it "should write the RAL catalog's class file" do
+        @catalog.expects(:write_class_file)
+
+        @agent.convert_catalog(@oldcatalog, 10)
+    end
+
+    it "should mark the RAL catalog as a host catalog" do
+        @catalog.expects(:host_config=).with true
+
+        @agent.convert_catalog(@oldcatalog, 10)
+    end
+end
+
+describe Puppet::Configurer, "when preparing for a run" do
+    before do
+        Puppet.settings.stubs(:use).returns(true)
+        @agent = Puppet::Configurer.new
+        @agent.stubs(:dostorage)
+        @agent.stubs(:upload_facts)
+        @facts = {"one" => "two", "three" => "four"}
+    end
+
+    it "should initialize the metadata store" do
+        @agent.class.stubs(:facts).returns(@facts)
+        @agent.expects(:dostorage)
+        @agent.prepare
+    end
+
+    it "should download fact plugins" do
+        @agent.stubs(:dostorage)
+        @agent.expects(:download_fact_plugins)
+
+        @agent.prepare
+    end
+
+    it "should download plugins" do
+        @agent.stubs(:dostorage)
+        @agent.expects(:download_plugins)
+
+        @agent.prepare
+    end
+
+    it "should upload facts to use for catalog retrieval" do
+        @agent.stubs(:dostorage)
+        @agent.expects(:upload_facts)
+        @agent.prepare
+    end
+end
diff --git a/spec/unit/agent/downloader.rb b/spec/unit/configurer/downloader.rb
index 28c5d030f..efd280d93 100755
--- a/spec/unit/agent/downloader.rb
+++ b/spec/unit/configurer/downloader.rb
@@ -2,36 +2,36 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-require 'puppet/agent/downloader'
+require 'puppet/configurer/downloader'
 
-describe Puppet::Agent::Downloader do
+describe Puppet::Configurer::Downloader do
     it "should require a name" do
-        lambda { Puppet::Agent::Downloader.new }.should raise_error(ArgumentError)
+        lambda { Puppet::Configurer::Downloader.new }.should raise_error(ArgumentError)
     end
 
     it "should require a path and a source at initialization" do
-        lambda { Puppet::Agent::Downloader.new("name") }.should raise_error(ArgumentError)
+        lambda { Puppet::Configurer::Downloader.new("name") }.should raise_error(ArgumentError)
     end
 
     it "should set the name, path and source appropriately" do
-        dler = Puppet::Agent::Downloader.new("facts", "path", "source")
+        dler = Puppet::Configurer::Downloader.new("facts", "path", "source")
         dler.name.should == "facts"
         dler.path.should == "path"
         dler.source.should == "source"
     end
 
     it "should be able to provide a timeout value" do
-        Puppet::Agent::Downloader.should respond_to(:timeout)
+        Puppet::Configurer::Downloader.should respond_to(:timeout)
     end
 
     it "should use the configtimeout, converted to an integer, as its timeout" do
         Puppet.settings.expects(:value).with(:configtimeout).returns "50"
-        Puppet::Agent::Downloader.timeout.should == 50
+        Puppet::Configurer::Downloader.timeout.should == 50
     end
 
     describe "when creating the file that does the downloading" do
         before do
-            @dler = Puppet::Agent::Downloader.new("foo", "path", "source")
+            @dler = Puppet::Configurer::Downloader.new("foo", "path", "source")
         end
 
         it "should create a file instance with the right path and source" do
@@ -83,14 +83,14 @@ describe Puppet::Agent::Downloader do
 
         it "should support providing an 'ignore' parameter" do
             Puppet::Type.type(:file).expects(:create).with { |opts| opts[:ignore] == ".svn" }
-            @dler = Puppet::Agent::Downloader.new("foo", "path", "source", ".svn")
+            @dler = Puppet::Configurer::Downloader.new("foo", "path", "source", ".svn")
             @dler.file
         end
     end
 
     describe "when creating the catalog to do the downloading" do
         before do
-            @dler = Puppet::Agent::Downloader.new("foo", "path", "source")
+            @dler = Puppet::Configurer::Downloader.new("foo", "path", "source")
         end
 
         it "should create a catalog and add the file to it" do
@@ -108,7 +108,7 @@ describe Puppet::Agent::Downloader do
 
     describe "when downloading" do
         before do
-            @dler = Puppet::Agent::Downloader.new("foo", "path", "source")
+            @dler = Puppet::Configurer::Downloader.new("foo", "path", "source")
         end
 
         it "should log that it is downloading" do
@@ -119,7 +119,7 @@ describe Puppet::Agent::Downloader do
         end
 
         it "should set a timeout for the download" do
-            Puppet::Agent::Downloader.expects(:timeout).returns 50
+            Puppet::Configurer::Downloader.expects(:timeout).returns 50
             Timeout.expects(:timeout).with(50)
 
             @dler.evaluate
diff --git a/spec/unit/agent/fact_handler.rb b/spec/unit/configurer/fact_handler.rb
index 9622b0649..ed9446c93 100755
--- a/spec/unit/agent/fact_handler.rb
+++ b/spec/unit/configurer/fact_handler.rb
@@ -1,14 +1,14 @@
 #!/usr/bin/env ruby
 
 require File.dirname(__FILE__) + '/../../spec_helper'
-require 'puppet/agent'
-require 'puppet/agent/fact_handler'
+require 'puppet/configurer'
+require 'puppet/configurer/fact_handler'
 
 class FactHandlerTester
-    include Puppet::Agent::FactHandler
+    include Puppet::Configurer::FactHandler
 end
 
-describe Puppet::Agent::FactHandler do
+describe Puppet::Configurer::FactHandler do
     before do
         @facthandler = FactHandlerTester.new
     end
@@ -32,7 +32,7 @@ describe Puppet::Agent::FactHandler do
     end
 
     it "should not download fact plugins when downloading is disabled" do
-        Puppet::Agent::Downloader.expects(:new).never
+        Puppet::Configurer::Downloader.expects(:new).never
         @facthandler.expects(:download_fact_plugins?).returns false
         @facthandler.download_fact_plugins
     end
@@ -44,7 +44,7 @@ describe Puppet::Agent::FactHandler do
         Puppet.settings.expects(:value).with(:factdest).returns "fdest"
         Puppet.settings.expects(:value).with(:factsignore).returns "fignore"
 
-        Puppet::Agent::Downloader.expects(:new).with("fact", "fsource", "fdest", "fignore").returns downloader
+        Puppet::Configurer::Downloader.expects(:new).with("fact", "fsource", "fdest", "fignore").returns downloader
 
         downloader.expects(:evaluate)
 
diff --git a/spec/unit/agent/plugin_handler.rb b/spec/unit/configurer/plugin_handler.rb
index 44603bc6c..23d9c11d3 100755
--- a/spec/unit/agent/plugin_handler.rb
+++ b/spec/unit/configurer/plugin_handler.rb
@@ -1,14 +1,14 @@
 #!/usr/bin/env ruby
 
 require File.dirname(__FILE__) + '/../../spec_helper'
-require 'puppet/agent'
-require 'puppet/agent/plugin_handler'
+require 'puppet/configurer'
+require 'puppet/configurer/plugin_handler'
 
 class PluginHandlerTester
-    include Puppet::Agent::PluginHandler
+    include Puppet::Configurer::PluginHandler
 end
 
-describe Puppet::Agent::PluginHandler do
+describe Puppet::Configurer::PluginHandler do
     before do
         @pluginhandler = PluginHandlerTester.new
     end
@@ -32,7 +32,7 @@ describe Puppet::Agent::PluginHandler do
     end
 
     it "should not download plugins when downloading is disabled" do
-        Puppet::Agent::Downloader.expects(:new).never
+        Puppet::Configurer::Downloader.expects(:new).never
         @pluginhandler.expects(:download_plugins?).returns false
         @pluginhandler.download_plugins
     end
@@ -44,7 +44,7 @@ describe Puppet::Agent::PluginHandler do
         Puppet.settings.expects(:value).with(:plugindest).returns "pdest"
         Puppet.settings.expects(:value).with(:pluginsignore).returns "pignore"
 
-        Puppet::Agent::Downloader.expects(:new).with("plugin", "psource", "pdest", "pignore").returns downloader
+        Puppet::Configurer::Downloader.expects(:new).with("plugin", "psource", "pdest", "pignore").returns downloader
 
         downloader.expects(:evaluate).returns []
 
@@ -59,7 +59,7 @@ describe Puppet::Agent::PluginHandler do
     it "should load each downloaded file" do
         downloader = mock 'downloader'
 
-        Puppet::Agent::Downloader.expects(:new).returns downloader
+        Puppet::Configurer::Downloader.expects(:new).returns downloader
 
         downloader.expects(:evaluate).returns %w{one two}