maint: Implement an InterfaceCollection class to manage interfaces

Having an instance variable on class Interface is insufficient for
Interface::Indirector. This also changes the semantics of "Interface.interface"
to handle registration and loading actions, and for "Interface.new" to only
instantiate an Interface. Thus, consumers of the API should typically use
"Interface.interface", unless they have reasons to not want an interface
automatically registered.

Paired-With: Pieter van de Bruggen

34 files changed, 229 insertions, 340 deletions

diff --git a/lib/puppet/application/interface_base.rb b/lib/puppet/application/interface_base.rb
index 654674df5..7a31ce323 100644
--- a/lib/puppet/application/interface_base.rb
+++ b/lib/puppet/application/interface_base.rb
@@ -1,4 +1,5 @@
 require 'puppet/application'
+require 'puppet/interface'
 
 class Puppet::Application::InterfaceBase < Puppet::Application
   should_parse_config
@@ -40,11 +41,6 @@ class Puppet::Application::InterfaceBase < Puppet::Application
     @exit_code || 0
   end
 
-  def initialize(*args)
-    require 'puppet/interface'
-    super
-  end
-
   def main
     # Call the method associated with the provided action (e.g., 'find').
     if result = interface.send(verb, *arguments)
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 0ec0f803f..f82d6235c 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -3,6 +3,7 @@ require 'puppet/util/autoload'
 
 class Puppet::Interface
   require 'puppet/interface/action_manager'
+  require 'puppet/interface/interface_collection'
 
   include Puppet::Interface::ActionManager
   extend Puppet::Interface::ActionManager
@@ -19,50 +20,27 @@ class Puppet::Interface
   end
 
   def self.interfaces
-    unless @loaded
-      @loaded = true
-      $LOAD_PATH.each do |dir|
-        next unless FileTest.directory?(dir)
-        Dir.chdir(dir) do
-          Dir.glob("puppet/interface/*.rb").collect { |f| f.sub(/\.rb/, '') }.each do |file|
-            iname = file.sub(/\.rb/, '')
-            begin
-              require iname
-            rescue Exception => detail
-              puts detail.backtrace if Puppet[:trace]
-              raise "Could not load #{iname} from #{dir}/#{file}: #{detail}"
-            end
-          end
-        end
-      end
-    end
-    return @interfaces.keys
+    Puppet::Interface::InterfaceCollection.interfaces
   end
 
   def self.interface?(name)
-    name = underscorize(name)
-    require "puppet/interface/#{name}" unless @interfaces.has_key? name
-    return @interfaces.has_key? name
-  rescue LoadError
-    return false
-  end
-
-  def self.interface(name, &blk)
-    interface = interface?(name) ? @interfaces[underscorize(name)] : new(name)
-    interface.instance_eval(&blk) if block_given?
-    return interface
+    Puppet::Interface::InterfaceCollection.interface?(name)
   end
 
-  def self.register_interface(name, instance)
-    @interfaces[underscorize(name)] = instance
+  def self.register(instance)
+    Puppet::Interface::InterfaceCollection.register(instance)
   end
 
-  def self.underscorize(name)
-    unless name.to_s =~ /^[-_a-z]+$/i then
-      raise ArgumentError, "#{name.inspect} (#{name.class}) is not a valid interface name"
+  def self.interface(name, &blk)
+    if interface?(name)
+      interface = Puppet::Interface::InterfaceCollection[name]
+      interface.instance_eval(&blk) if blk
+    else
+      interface = new(name, &blk)
+      Puppet::Interface::InterfaceCollection.register(interface)
+      interface.load_actions
     end
-
-    name.to_s.downcase.split(/[-_]/).join('_').to_sym
+    return interface
   end
 
   attr_accessor :default_format
@@ -71,30 +49,16 @@ class Puppet::Interface
     self.default_format = format.to_sym
   end
 
-  # Return the interface name.
-  def name
-    @name || self.to_s.sub(/.+::/, '').downcase
-  end
-
-  attr_accessor :type, :verb, :name, :arguments, :options
+  attr_accessor :type, :verb, :arguments, :options
+  attr_reader :name
 
   def initialize(name, options = {}, &block)
-    @name = self.class.underscorize(name)
+    @name = Puppet::Interface::InterfaceCollection.underscorize(name)
 
     @default_format = :pson
     options.each { |opt, val| send(opt.to_s + "=", val) }
 
-    # We have to register before loading actions,
-    # since the actions require the registration
-    # Use the full class name, so this works with
-    # subclasses.
-    Puppet::Interface.register_interface(name, self)
-
-    load_actions
-
-    if block_given?
-      instance_eval(&block)
-    end
+    instance_eval(&block) if block
   end
 
   # Try to find actions defined in other files.
@@ -102,7 +66,7 @@ class Puppet::Interface
     path = "puppet/interface/#{name}"
 
     loaded = []
-    self.class.autoloader.search_directories.each do |dir|
+    Puppet::Interface.autoloader.search_directories.each do |dir|
       fdir = ::File.join(dir, path)
       next unless FileTest.directory?(fdir)
 
diff --git a/lib/puppet/interface/catalog.rb b/lib/puppet/interface/catalog.rb
index c0af53bac..defe32127 100644
--- a/lib/puppet/interface/catalog.rb
+++ b/lib/puppet/interface/catalog.rb
@@ -1,6 +1,6 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:catalog) do
+Puppet::Interface::Indirector.interface(:catalog) do
   action(:apply) do
     invoke do |catalog|
       report = Puppet::Transaction::Report.new("apply")
diff --git a/lib/puppet/interface/certificate.rb b/lib/puppet/interface/certificate.rb
index 52ba4e3b8..09da0a6c3 100644
--- a/lib/puppet/interface/certificate.rb
+++ b/lib/puppet/interface/certificate.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:certificate) do
+Puppet::Interface::Indirector.interface(:certificate) do
 end
diff --git a/lib/puppet/interface/certificate_request.rb b/lib/puppet/interface/certificate_request.rb
index 77b485f8e..b85c15fef 100644
--- a/lib/puppet/interface/certificate_request.rb
+++ b/lib/puppet/interface/certificate_request.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:certificate_request) do
+Puppet::Interface::Indirector.interface(:certificate_request) do
 end
diff --git a/lib/puppet/interface/certificate_revocation_list.rb b/lib/puppet/interface/certificate_revocation_list.rb
index ee1e6a8c4..956fb6494 100644
--- a/lib/puppet/interface/certificate_revocation_list.rb
+++ b/lib/puppet/interface/certificate_revocation_list.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:certificate_revocation_list) do
+Puppet::Interface::Indirector.interface(:certificate_revocation_list) do
 end
diff --git a/lib/puppet/interface/config.rb b/lib/puppet/interface/config.rb
index 0aecc263f..79d2ee7c1 100644
--- a/lib/puppet/interface/config.rb
+++ b/lib/puppet/interface/config.rb
@@ -1,6 +1,6 @@
 require 'puppet/interface'
 
-Puppet::Interface.new(:config) do
+Puppet::Interface.interface(:config) do
   action(:print) do
     invoke do |*args|
       Puppet.settings[:configprint] = args.join(",")
diff --git a/lib/puppet/interface/configurer.rb b/lib/puppet/interface/configurer.rb
index 2fbde27f1..0d21c4d72 100644
--- a/lib/puppet/interface/configurer.rb
+++ b/lib/puppet/interface/configurer.rb
@@ -1,6 +1,6 @@
 require 'puppet/interface'
 
-Puppet::Interface.new(:configurer) do
+Puppet::Interface.interface(:configurer) do
   action(:synchronize) do
     invoke do |certname|
       facts = Puppet::Interface.interface(:facts).find(certname)
diff --git a/lib/puppet/interface/facts.rb b/lib/puppet/interface/facts.rb
index 8843d297d..97e22714b 100644
--- a/lib/puppet/interface/facts.rb
+++ b/lib/puppet/interface/facts.rb
@@ -1,7 +1,7 @@
 require 'puppet/interface/indirector'
 require 'puppet/node/facts'
 
-Puppet::Interface::Indirector.new(:facts) do
+Puppet::Interface::Indirector.interface(:facts) do
   set_default_format :yaml
 
   # Upload our facts to the server
diff --git a/lib/puppet/interface/file.rb b/lib/puppet/interface/file.rb
index 859f92ca4..f38af2b92 100644
--- a/lib/puppet/interface/file.rb
+++ b/lib/puppet/interface/file.rb
@@ -1,5 +1,5 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:file) do
+Puppet::Interface::Indirector.interface(:file) do
   set_indirection_name :file_bucket_file
 end
diff --git a/lib/puppet/interface/interface_collection.rb b/lib/puppet/interface/interface_collection.rb
new file mode 100644
index 000000000..47ed702aa
--- /dev/null
+++ b/lib/puppet/interface/interface_collection.rb
@@ -0,0 +1,50 @@
+require 'puppet/interface'
+
+module Puppet::Interface::InterfaceCollection
+  @interfaces = {}
+
+  def self.interfaces
+    unless @loaded
+      @loaded = true
+      $LOAD_PATH.each do |dir|
+        next unless FileTest.directory?(dir)
+        Dir.chdir(dir) do
+          Dir.glob("puppet/interface/*.rb").collect { |f| f.sub(/\.rb/, '') }.each do |file|
+            iname = file.sub(/\.rb/, '')
+            begin
+              require iname
+            rescue Exception => detail
+              puts detail.backtrace if Puppet[:trace]
+              raise "Could not load #{iname} from #{dir}/#{file}: #{detail}"
+            end
+          end
+        end
+      end
+    end
+    return @interfaces.keys
+  end
+
+  def self.[](name)
+    @interfaces[underscorize(name)] if interface?(name)
+  end
+
+  def self.interface?(name)
+    name = underscorize(name)
+    require "puppet/interface/#{name}" unless @interfaces.has_key? name
+    return @interfaces.has_key? name
+  rescue LoadError
+    return false
+  end
+
+  def self.register(interface)
+    @interfaces[underscorize(interface.name)] = interface
+  end
+
+  def self.underscorize(name)
+    unless name.to_s =~ /^[-_a-z]+$/i then
+      raise ArgumentError, "#{name.inspect} (#{name.class}) is not a valid interface name"
+    end
+
+    name.to_s.downcase.split(/[-_]/).join('_').to_sym
+  end
+end
diff --git a/lib/puppet/interface/key.rb b/lib/puppet/interface/key.rb
index 9343891d0..57519883d 100644
--- a/lib/puppet/interface/key.rb
+++ b/lib/puppet/interface/key.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:key) do
+Puppet::Interface::Indirector.interface(:key) do
 end
diff --git a/lib/puppet/interface/node.rb b/lib/puppet/interface/node.rb
index 0a0f57a1e..8940fd7dd 100644
--- a/lib/puppet/interface/node.rb
+++ b/lib/puppet/interface/node.rb
@@ -1,5 +1,5 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:node) do
+Puppet::Interface::Indirector.interface(:node) do
   set_default_format :yaml
 end
diff --git a/lib/puppet/interface/report.rb b/lib/puppet/interface/report.rb
index e785ae22d..56a58f6aa 100644
--- a/lib/puppet/interface/report.rb
+++ b/lib/puppet/interface/report.rb
@@ -1,6 +1,6 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:report) do
+Puppet::Interface::Indirector.interface(:report) do
   action(:submit) do
     invoke do |report|
       begin
diff --git a/lib/puppet/interface/resource.rb b/lib/puppet/interface/resource.rb
index 65f2dec7a..130f40fce 100644
--- a/lib/puppet/interface/resource.rb
+++ b/lib/puppet/interface/resource.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:resource) do
+Puppet::Interface::Indirector.interface(:resource) do
 end
diff --git a/lib/puppet/interface/resource_type.rb b/lib/puppet/interface/resource_type.rb
index bf16652a8..70bf3b95a 100644
--- a/lib/puppet/interface/resource_type.rb
+++ b/lib/puppet/interface/resource_type.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:resource_type) do
+Puppet::Interface::Indirector.interface(:resource_type) do
 end
diff --git a/lib/puppet/interface/status.rb b/lib/puppet/interface/status.rb
index 1a1d349d1..432d1ce54 100644
--- a/lib/puppet/interface/status.rb
+++ b/lib/puppet/interface/status.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:status) do
+Puppet::Interface::Indirector.interface(:status) do
 end
diff --git a/spec/unit/application/interface_base_spec.rb b/spec/unit/application/interface_base_spec.rb
index ba1e6abf5..3e7c04f5c 100644
--- a/spec/unit/application/interface_base_spec.rb
+++ b/spec/unit/application/interface_base_spec.rb
@@ -4,7 +4,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/application/interface_base'
 require 'puppet/application/interface_base'
 
-base_interface = Puppet::Interface.new(:basetest)
+base_interface = Puppet::Interface.interface(:basetest)
 class Puppet::Application::InterfaceBase::Basetest < Puppet::Application::InterfaceBase
 end
 
diff --git a/spec/unit/interface/action_manager_spec.rb b/spec/unit/interface/action_manager_spec.rb
index 0b12db317..bf101b344 100644
--- a/spec/unit/interface/action_manager_spec.rb
+++ b/spec/unit/interface/action_manager_spec.rb
@@ -10,65 +10,60 @@ class ActionManagerTester
 end
 
 describe Puppet::Interface::ActionManager do
-  before do
-    @tester = ActionManagerTester.new
-  end
+  subject { ActionManagerTester.new }
 
   describe "when included in a class" do
     it "should be able to define an action" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something "}
       end
     end
 
     it "should be able to list defined actions" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something" }
       end
-      @tester.action(:bar) do
+      subject.action(:bar) do
         invoke { "something" }
       end
 
-      @tester.actions.should include(:bar)
-      @tester.actions.should include(:foo)
+      subject.actions.should include(:bar)
+      subject.actions.should include(:foo)
     end
 
     it "should be able to indicate when an action is defined" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something" }
       end
 
-      @tester.should be_action(:foo)
+      subject.should be_action(:foo)
     end
   end
 
   describe "when used to extend a class" do
-    before do
-      @tester = Class.new
-      @tester.extend(Puppet::Interface::ActionManager)
-    end
+    subject { Class.new.extend(Puppet::Interface::ActionManager) }
 
     it "should be able to define an action" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something "}
       end
     end
 
     it "should be able to list defined actions" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something" }
       end
-      @tester.action(:bar) do
+      subject.action(:bar) do
         invoke { "something" }
       end
 
-      @tester.actions.should include(:bar)
-      @tester.actions.should include(:foo)
+      subject.actions.should include(:bar)
+      subject.actions.should include(:foo)
     end
 
     it "should be able to indicate when an action is defined" do
-      @tester.action(:foo) { "something" }
-      @tester.should be_action(:foo)
+      subject.action(:foo) { "something" }
+      subject.should be_action(:foo)
     end
   end
 
diff --git a/spec/unit/interface/catalog_spec.rb b/spec/unit/interface/catalog_spec.rb
index 8eb0040ff..78d62110f 100644
--- a/spec/unit/interface/catalog_spec.rb
+++ b/spec/unit/interface/catalog_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/catalog'
 
-describe Puppet::Interface.interface(:catalog) do
-  before do
-    @interface = Puppet::Interface.interface(:catalog)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'catalog' indirection" do
-    @interface.indirection.name.should == :catalog
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:catalog) do
 end
diff --git a/spec/unit/interface/certificate_request_spec.rb b/spec/unit/interface/certificate_request_spec.rb
index 8a613e5e5..a6ab8d17e 100644
--- a/spec/unit/interface/certificate_request_spec.rb
+++ b/spec/unit/interface/certificate_request_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate_request'
 
-describe Puppet::Interface.interface(:certificate_request) do
-  before do
-    @interface = Puppet::Interface.interface(:certificate_request)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'certificate_request' indirection" do
-    @interface.indirection.name.should == :certificate_request
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:certificate_request) do
 end
diff --git a/spec/unit/interface/certificate_revocation_list_spec.rb b/spec/unit/interface/certificate_revocation_list_spec.rb
index 8ee341bef..a98b48d90 100644
--- a/spec/unit/interface/certificate_revocation_list_spec.rb
+++ b/spec/unit/interface/certificate_revocation_list_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate_revocation_list'
 
-describe Puppet::Interface.interface(:certificate_revocation_list) do
-  before do
-    @interface = Puppet::Interface.interface(:certificate_revocation_list)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'certificate_revocation_list' indirection" do
-    @interface.indirection.name.should == :certificate_revocation_list
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:certificate_revocation_list) do
 end
diff --git a/spec/unit/interface/certificate_spec.rb b/spec/unit/interface/certificate_spec.rb
index 47ddcb52e..6a325343f 100644
--- a/spec/unit/interface/certificate_spec.rb
+++ b/spec/unit/interface/certificate_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate'
 
-describe Puppet::Interface.interface(:certificate) do
-  before do
-    @interface = Puppet::Interface.interface(:certificate)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'certificate' indirection" do
-    @interface.indirection.name.should == :certificate
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:certificate) do
 end
diff --git a/spec/unit/interface/config_spec.rb b/spec/unit/interface/config_spec.rb
index 79c65f2ac..e8aafd4a3 100644
--- a/spec/unit/interface/config_spec.rb
+++ b/spec/unit/interface/config_spec.rb
@@ -4,24 +4,16 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/config'
 
 describe Puppet::Interface.interface(:config) do
-  before do
-    @interface = Puppet::Interface.interface(:config)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface)
-  end
-
   it "should use Settings#print_config_options when asked to print" do
     Puppet.settings.stubs(:puts)
     Puppet.settings.expects(:print_config_options)
-    @interface.print
+    subject.print
   end
 
   it "should set 'configprint' to all desired values and call print_config_options when a specific value is provided" do
     Puppet.settings.stubs(:puts)
     Puppet.settings.expects(:print_config_options)
-    @interface.print("libdir", "ssldir")
+    subject.print("libdir", "ssldir")
     Puppet.settings[:configprint].should == "libdir,ssldir"
   end
 end
diff --git a/spec/unit/interface/facts_spec.rb b/spec/unit/interface/facts_spec.rb
index 03d6410f9..d0f87d3a0 100644
--- a/spec/unit/interface/facts_spec.rb
+++ b/spec/unit/interface/facts_spec.rb
@@ -3,17 +3,13 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/facts'
 
-describe Puppet::Interface.interface(:facts) do
-  before do
-    @interface = Puppet::Interface.interface(:facts)
-  end
-
+describe Puppet::Interface::Indirector.interface(:facts) do
   it "should define an 'upload' fact" do
-    @interface.should be_action(:upload)
+    subject.should be_action(:upload)
   end
 
   it "should set its default format to :yaml" do
-    @interface.default_format.should == :yaml
+    subject.default_format.should == :yaml
   end
 
   describe "when uploading" do
diff --git a/spec/unit/interface/file_spec.rb b/spec/unit/interface/file_spec.rb
index fc7accf0d..54427a267 100644
--- a/spec/unit/interface/file_spec.rb
+++ b/spec/unit/interface/file_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/file'
 
-describe Puppet::Interface.interface(:file) do
-  before do
-    @interface = Puppet::Interface.interface(:file)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'file' indirection" do
-    @interface.indirection.name.should == :file_bucket_file
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:file) do
 end
diff --git a/spec/unit/interface/indirector_spec.rb b/spec/unit/interface/indirector_spec.rb
index c4d93ade3..0eb7a9a4f 100644
--- a/spec/unit/interface/indirector_spec.rb
+++ b/spec/unit/interface/indirector_spec.rb
@@ -12,10 +12,6 @@ describe Puppet::Interface::Indirector do
     @instance.stubs(:indirection).returns @indirection
   end
 
-  it "should be a subclass of Interface" do
-    Puppet::Interface::Indirector.superclass.should equal(Puppet::Interface)
-  end
-
   it "should be able to return a list of indirections" do
     Puppet::Interface::Indirector.indirections.should be_include("catalog")
   end
diff --git a/spec/unit/interface/interface_collection_spec.rb b/spec/unit/interface/interface_collection_spec.rb
new file mode 100644
index 000000000..536e694fd
--- /dev/null
+++ b/spec/unit/interface/interface_collection_spec.rb
@@ -0,0 +1,97 @@
+require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
+
+require 'puppet/interface/interface_collection'
+
+describe Puppet::Interface::InterfaceCollection do
+  before :each do
+    subject.instance_variable_set("@interfaces", {})
+  end
+
+  after :all do
+    subject.instance_variable_set("@interfaces", {})
+  end
+
+  describe "::interfaces" do
+  end
+
+  describe "::[]" do
+    before :each do
+      subject.instance_variable_set("@interfaces", {:foo => 10})
+    end
+
+    it "should return the interface with the given name" do
+      subject["foo"].should == 10
+    end
+
+    it "should attempt to load the interface if it isn't found" do
+      subject.expects(:require).with('puppet/interface/bar')
+      subject["bar"]
+    end
+  end
+
+  describe "::interface?" do
+    before :each do
+      subject.instance_variable_set("@interfaces", {:foo => 10})
+    end
+
+    it "should return true if the interface specified is registered" do
+      subject.interface?("foo").should == true
+    end
+
+    it "should attempt to require the interface if it is not registered" do
+      subject.expects(:require).with('puppet/interface/bar')
+      subject.interface?("bar")
+    end
+
+    it "should return true if requiring the interface registered it" do
+      subject.stubs(:require).with do
+        subject.instance_variable_set("@interfaces", {:bar => 20})
+      end
+      subject.interface?("bar").should == true
+    end
+
+    it "should return false if the interface is not registered" do
+      subject.stubs(:require).returns(true)
+      subject.interface?("bar").should == false
+    end
+
+    it "should return false if there is a LoadError requiring the interface" do
+      subject.stubs(:require).raises(LoadError)
+      subject.interface?("bar").should == false
+    end
+  end
+
+  describe "::register" do
+    it "should store the interface by name" do
+      interface = Puppet::Interface.new(:my_interface)
+      subject.register(interface)
+      subject.instance_variable_get("@interfaces").should == {:my_interface => interface}
+    end
+  end
+
+  describe "::underscorize" do
+    faulty = [1, "#foo", "$bar", "sturm und drang", :"sturm und drang"]
+    valid  = {
+      "Foo"      => :foo,
+      :Foo       => :foo,
+      "foo_bar"  => :foo_bar,
+      :foo_bar   => :foo_bar,
+      "foo-bar"  => :foo_bar,
+      :"foo-bar" => :foo_bar,
+    }
+
+    valid.each do |input, expect|
+      it "should map #{input.inspect} to #{expect.inspect}" do
+        result = subject.underscorize(input)
+        result.should == expect
+      end
+    end
+
+    faulty.each do |input|
+      it "should fail when presented with #{input.inspect} (#{input.class})" do
+        expect { subject.underscorize(input) }.
+          should raise_error ArgumentError, /not a valid interface name/
+      end
+    end
+  end
+end
diff --git a/spec/unit/interface/key_spec.rb b/spec/unit/interface/key_spec.rb
index 93a7c937c..4b331d169 100644
--- a/spec/unit/interface/key_spec.rb
+++ b/spec/unit/interface/key_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/key'
 
-describe Puppet::Interface.interface(:key) do
-  before do
-    @interface = Puppet::Interface.interface(:key)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'key' indirection" do
-    @interface.indirection.name.should == :key
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:key) do
 end
diff --git a/spec/unit/interface/node_spec.rb b/spec/unit/interface/node_spec.rb
index 63109308d..b1b4ad421 100644
--- a/spec/unit/interface/node_spec.rb
+++ b/spec/unit/interface/node_spec.rb
@@ -3,26 +3,8 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/node'
 
-describe Puppet::Interface.interface(:node) do
-  before do
-    @interface = Puppet::Interface.interface(:node)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
+describe Puppet::Interface::Indirector.interface(:node) do
   it "should set its default format to :yaml" do
-    @interface.default_format.should == :yaml
-  end
-
-  it "should refer to the 'node' indirection" do
-    @interface.indirection.name.should == :node
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
+    subject.default_format.should == :yaml
   end
 end
diff --git a/spec/unit/interface/report_spec.rb b/spec/unit/interface/report_spec.rb
index b5bee1a62..c424880a9 100644
--- a/spec/unit/interface/report_spec.rb
+++ b/spec/unit/interface/report_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/report'
 
-describe Puppet::Interface.interface(:report) do
-  before do
-    @interface = Puppet::Interface.interface(:report)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'report' indirection" do
-    @interface.indirection.name.should == :report
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:report) do
 end
diff --git a/spec/unit/interface/resource_spec.rb b/spec/unit/interface/resource_spec.rb
index cad45b66b..aab2753b1 100644
--- a/spec/unit/interface/resource_spec.rb
+++ b/spec/unit/interface/resource_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/resource'
 
-describe Puppet::Interface.interface(:resource) do
-  before do
-    @interface = Puppet::Interface.interface(:resource)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'resource' indirection" do
-    @interface.indirection.name.should == :resource
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:resource) do
 end
diff --git a/spec/unit/interface/resource_type_spec.rb b/spec/unit/interface/resource_type_spec.rb
index 6c437c475..6e973c98b 100644
--- a/spec/unit/interface/resource_type_spec.rb
+++ b/spec/unit/interface/resource_type_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/resource_type'
 
-describe Puppet::Interface.interface(:resource_type) do
-  before do
-    @interface = Puppet::Interface.interface(:resource_type)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'resource_type' indirection" do
-    @interface.indirection.name.should == :resource_type
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:resource_type) do
 end
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index 876c7945c..e35da6b95 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -4,32 +4,32 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
 require 'puppet/interface'
 
 describe Puppet::Interface do
-  describe "at initialization" do
-    it "should require a name" do
-      Puppet::Interface.new(:me).name.should == :me
-    end
-
-    it "should register itself" do
-      Puppet::Interface.expects(:register_interface).with do |name, inst|
-        name == :me and inst.is_a?(Puppet::Interface)
-      end
-      Puppet::Interface.new(:me)
+  describe "#interface" do
+    it "should register the interface" do
+      interface = Puppet::Interface.interface(:interface_test_register)
+      interface.should == Puppet::Interface.interface(:interface_test_register)
     end
 
     it "should load actions" do
       Puppet::Interface.any_instance.expects(:load_actions)
-      Puppet::Interface.new(:me)
+      Puppet::Interface.interface(:interface_test_load_actions)
     end
 
     it "should instance-eval any provided block" do
-      face = Puppet::Interface.new(:me) do
-        action(:something) { "foo" }
+      face = Puppet::Interface.new(:interface_test_block) do
+        action(:something) do
+          invoke { "foo" }
+        end
       end
 
-      face.should be_action(:something)
+      face.something.should == "foo"
     end
   end
 
+  it "should have a name" do
+    Puppet::Interface.new(:me).name.should == :me
+  end
+
   it "should stringify with its own name" do
     Puppet::Interface.new(:me).to_s.should =~ /\bme\b/
   end
@@ -54,35 +54,9 @@ describe Puppet::Interface do
   end
 
   it "should try to require interfaces that are not known" do
-    Puppet::Interface.expects(:require).with "puppet/interface/foo"
+    Puppet::Interface::InterfaceCollection.expects(:require).with "puppet/interface/foo"
     Puppet::Interface.interface(:foo)
   end
 
   it "should be able to load all actions in all search paths"
-
-  describe "#underscorize" do
-    faulty = [1, "#foo", "$bar", "sturm und drang", :"sturm und drang"]
-    valid  = {
-      "Foo"      => :foo,
-      :Foo       => :foo,
-      "foo_bar"  => :foo_bar,
-      :foo_bar   => :foo_bar,
-      "foo-bar"  => :foo_bar,
-      :"foo-bar" => :foo_bar,
-    }
-
-    valid.each do |input, expect|
-      it "should map #{input.inspect} to #{expect.inspect}" do
-        result = Puppet::Interface.underscorize(input)
-        result.should == expect
-      end
-    end
-
-    faulty.each do |input|
-      it "should fail when presented with #{input.inspect} (#{input.class})" do
-        expect { Puppet::Interface.underscorize(input) }.
-          should raise_error ArgumentError, /not a valid interface name/
-      end
-    end
-  end
 end