diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index 4c5a5a967..f3a749786 100644
--- a/lib/puppet/application.rb
+++ b/lib/puppet/application.rb
@@ -1,420 +1,416 @@
 require 'optparse'
 require 'puppet/util/plugins'
 
 # This class handles all the aspects of a Puppet application/executable
 # * setting up options
 # * setting up logs
 # * choosing what to run
 # * representing execution status
 #
 # === Usage
 # An application is a subclass of Puppet::Application.
 #
 # For legacy compatibility,
 #      Puppet::Application[:example].run
 # is equivalent to
 #      Puppet::Application::Example.new.run
 #
 #
 # class Puppet::Application::Example << Puppet::Application
 #
 #     def preinit
 #         # perform some pre initialization
 #         @all = false
 #     end
 #
 #     # run_command is called to actually run the specified command
 #     def run_command
 #         send Puppet::Util::CommandLine.new.args.shift
 #     end
 #
 #     # option uses metaprogramming to create a method
 #     # and also tells the option parser how to invoke that method
 #     option("--arg ARGUMENT") do |v|
 #         @args << v
 #     end
 #
 #     option("--debug", "-d") do |v|
 #         @debug = v
 #     end
 #
 #     option("--all", "-a:) do |v|
 #         @all = v
 #     end
 #
 #     def handle_unknown(opt,arg)
 #         # last chance to manage an option
 #         ...
 #         # let's say to the framework we finally handle this option
 #         true
 #     end
 #
 #     def read
 #         # read action
 #     end
 #
 #     def write
 #         # writeaction
 #     end
 #
 # end
 #
 # === Preinit
 # The preinit block is the first code to be called in your application, before option parsing,
 # setup or command execution.
 #
 # === Options
 # Puppet::Application uses +OptionParser+ to manage the application options.
 # Options are defined with the +option+ method to which are passed various
 # arguments, including the long option, the short option, a description...
 # Refer to +OptionParser+ documentation for the exact format.
 # * If the option method is given a block, this one will be called whenever
 # the option is encountered in the command-line argument.
 # * If the option method has no block, a default functionnality will be used, that
 # stores the argument (or true/false if the option doesn't require an argument) in
 # the global (to the application) options array.
 # * If a given option was not defined by a the +option+ method, but it exists as a Puppet settings:
 #  * if +unknown+ was used with a block, it will be called with the option name and argument
 #  * if +unknown+ wasn't used, then the option/argument is handed to Puppet.settings.handlearg for
 #    a default behavior
 #
 # --help is managed directly by the Puppet::Application class, but can be overriden.
 #
 # === Setup
 # Applications can use the setup block to perform any initialization.
 # The defaul +setup+ behaviour is to: read Puppet configuration and manage log level and destination
 #
 # === What and how to run
 # If the +dispatch+ block is defined it is called. This block should return the name of the registered command
 # to be run.
 # If it doesn't exist, it defaults to execute the +main+ command if defined.
 #
 # === Execution state
 # The class attributes/methods of Puppet::Application serve as a global place to set and query the execution
 # status of the application: stopping, restarting, etc.  The setting of the application status does not directly
 # aftect its running status; it's assumed that the various components within the application will consult these
 # settings appropriately and affect their own processing accordingly.  Control operations (signal handlers and
 # the like) should set the status appropriately to indicate to the overall system that it's the process of
 # stopping or restarting (or just running as usual).
 #
 # So, if something in your application needs to stop the process, for some reason, you might consider:
 #
 #  def stop_me!
 #      # indicate that we're stopping
 #      Puppet::Application.stop!
 #      # ...do stuff...
 #  end
 #
 # And, if you have some component that involves a long-running process, you might want to consider:
 #
 #  def my_long_process(giant_list_to_munge)
 #      giant_list_to_munge.collect do |member|
 #          # bail if we're stopping
 #          return if Puppet::Application.stop_requested?
 #          process_member(member)
 #      end
 #  end
 module Puppet
 class Application
   require 'puppet/util'
   include Puppet::Util
 
   DOCPATTERN = File.expand_path(File.dirname(__FILE__) + "/util/command_line/*" )
 
   class << self
     include Puppet::Util
 
     attr_accessor :run_status
 
     def clear!
       self.run_status = nil
     end
 
     def stop!
       self.run_status = :stop_requested
     end
 
     def restart!
       self.run_status = :restart_requested
     end
 
     # Indicates that Puppet::Application.restart! has been invoked and components should
     # do what is necessary to facilitate a restart.
     def restart_requested?
       :restart_requested == run_status
     end
 
     # Indicates that Puppet::Application.stop! has been invoked and components should do what is necessary
     # for a clean stop.
     def stop_requested?
       :stop_requested == run_status
     end
 
     # Indicates that one of stop! or start! was invoked on Puppet::Application, and some kind of process
     # shutdown/short-circuit may be necessary.
     def interrupted?
       [:restart_requested, :stop_requested].include? run_status
     end
 
     # Indicates that Puppet::Application believes that it's in usual running run_mode (no stop/restart request
     # currently active).
     def clear?
       run_status.nil?
     end
 
     # Only executes the given block if the run status of Puppet::Application is clear (no restarts, stops,
     # etc. requested).
     # Upon block execution, checks the run status again; if a restart has been requested during the block's
     # execution, then controlled_run will send a new HUP signal to the current process.
     # Thus, long-running background processes can potentially finish their work before a restart.
     def controlled_run(&block)
       return unless clear?
       result = block.call
       Process.kill(:HUP, $PID) if restart_requested?
       result
     end
 
     def should_parse_config
       @parse_config = true
     end
 
     def should_not_parse_config
       @parse_config = false
     end
 
     def should_parse_config?
       @parse_config = true if ! defined?(@parse_config)
       @parse_config
     end
 
     # used to declare code that handle an option
     def option(*options, &block)
       long = options.find { |opt| opt =~ /^--/ }.gsub(/^--(?:\[no-\])?([^ =]+).*$/, '\1' ).gsub('-','_')
       fname = symbolize("handle_#{long}")
       if (block_given?)
         define_method(fname, &block)
       else
         define_method(fname) do |value|
           self.options["#{long}".to_sym] = value
         end
       end
       self.option_parser_commands << [options, fname]
     end
 
     def banner(banner = nil)
       @banner ||= banner
     end
 
     def option_parser_commands
       @option_parser_commands ||= (
         superclass.respond_to?(:option_parser_commands) ? superclass.option_parser_commands.dup : []
       )
       @option_parser_commands
     end
 
     def find(name)
       klass = name.to_s.capitalize
 
       # const_defined? is used before const_get since const_defined? will only
       # check within our namespace, whereas const_get will check ancestor
       # trees as well, resulting in unexpected behaviour.
       if !self.const_defined?(klass)
         puts "Unable to find application '#{name.to_s}'."
         Kernel::exit(1)
       end
 
       self.const_get(klass)
     end
 
     def [](name)
       find(name).new
     end
 
     # Sets or gets the run_mode name. Sets the run_mode name if a mode_name is
     # passed. Otherwise, gets the run_mode or a default run_mode
     #
     def run_mode( mode_name = nil)
       return @run_mode if @run_mode and not mode_name
 
       require 'puppet/util/run_mode'
       @run_mode = Puppet::Util::RunMode[ mode_name || :user ]
     end
   end
 
   attr_reader :options, :command_line
 
   # Every app responds to --version
   option("--version", "-V") do |arg|
     puts "#{Puppet.version}"
     exit
   end
 
   # Every app responds to --help
   option("--help", "-h") do |v|
     puts help
     exit
   end
 
   def should_parse_config?
     self.class.should_parse_config?
   end
 
   # override to execute code before running anything else
   def preinit
   end
 
   def initialize(command_line = nil)
     require 'puppet/util/command_line'
     @command_line = command_line || Puppet::Util::CommandLine.new
     set_run_mode self.class.run_mode
     @options = {}
 
     require 'puppet'
   end
 
   # WARNING: This is a totally scary, frightening, and nasty internal API.  We
   # strongly advise that you do not use this, and if you insist, we will
   # politely allow you to keep both pieces of your broken code.
   #
   # We plan to provide a supported, long-term API to deliver this in a way
   # that you can use.  Please make sure that you let us know if you do require
   # this, and this message is still present in the code. --daniel 2011-02-03
   def set_run_mode(mode)
     @run_mode = mode
     $puppet_application_mode = @run_mode
     $puppet_application_name = name
 
     if Puppet.respond_to? :settings
       # This is to reduce the amount of confusion in rspec
       # because it might have loaded defaults.rb before the globals were set
       # and thus have the wrong defaults for the current application
       Puppet.settings.set_value(:confdir, Puppet.run_mode.conf_dir, :mutable_defaults)
       Puppet.settings.set_value(:vardir, Puppet.run_mode.var_dir, :mutable_defaults)
       Puppet.settings.set_value(:name, Puppet.application_name.to_s, :mutable_defaults)
       Puppet.settings.set_value(:logdir, Puppet.run_mode.logopts, :mutable_defaults)
       Puppet.settings.set_value(:rundir, Puppet.run_mode.run_dir, :mutable_defaults)
       Puppet.settings.set_value(:run_mode, Puppet.run_mode.name.to_s, :mutable_defaults)
     end
   end
 
   # This is the main application entry point
   def run
     exit_on_fail("initialize")                                   { hook('preinit')       { preinit } }
     exit_on_fail("parse options")                                { hook('parse_options') { parse_options } }
     exit_on_fail("parse configuration file")                     { Puppet.settings.parse } if should_parse_config?
     exit_on_fail("prepare for execution")                        { hook('setup')         { setup } }
     exit_on_fail("configure routes from #{Puppet[:route_file]}") { configure_indirector_routes }
     exit_on_fail("run")                                          { hook('run_command')   { run_command } }
   end
 
   def main
     raise NotImplementedError, "No valid command or main"
   end
 
   def run_command
     main
   end
 
   def setup
     # Handle the logging settings
     if options[:debug] or options[:verbose]
       Puppet::Util::Log.newdestination(:console)
       if options[:debug]
         Puppet::Util::Log.level = :debug
       else
         Puppet::Util::Log.level = :info
       end
     end
 
     Puppet::Util::Log.newdestination(:syslog) unless options[:setdest]
   end
 
   def configure_indirector_routes
     route_file = Puppet[:route_file]
     if File.exists?(route_file)
       routes = YAML.load_file(route_file)
       application_routes = routes[name.to_s]
       Puppet::Indirector.configure_routes(application_routes) if application_routes
     end
   end
 
   def parse_options
     # Create an option parser
     option_parser = OptionParser.new(self.class.banner)
 
     # Add all global options to it.
     Puppet.settings.optparse_addargs([]).each do |option|
       option_parser.on(*option) do |arg|
         handlearg(option[0], arg)
       end
     end
 
     # Add options that are local to this application, which were
     # created using the "option()" metaprogramming method.  If there
     # are any conflicts, this application's options will be favored.
     self.class.option_parser_commands.each do |options, fname|
       option_parser.on(*options) do |value|
         # Call the method that "option()" created.
         self.send(fname, value)
       end
     end
 
-    # scan command line.
-    begin
-      option_parser.parse!(self.command_line.args)
-    rescue OptionParser::ParseError => detail
-      $stderr.puts detail
-      $stderr.puts "Try 'puppet #{command_line.subcommand_name} --help'"
-      exit(1)
-    end
+    # Scan command line.  We just hand any exceptions to our upper levels,
+    # rather than printing help and exiting, so that we can meaningfully
+    # respond with context-sensitive help if we want to. --daniel 2011-04-12
+    option_parser.parse!(self.command_line.args)
   end
 
   def handlearg(opt, arg)
     # rewrite --[no-]option to --no-option if that's what was given
     if opt =~ /\[no-\]/ and !arg
       opt = opt.gsub(/\[no-\]/,'no-')
     end
     # otherwise remove the [no-] prefix to not confuse everybody
     opt = opt.gsub(/\[no-\]/, '')
     unless respond_to?(:handle_unknown) and send(:handle_unknown, opt, arg)
       # Puppet.settings.handlearg doesn't handle direct true/false :-)
       if arg.is_a?(FalseClass)
         arg = "false"
       elsif arg.is_a?(TrueClass)
         arg = "true"
       end
       Puppet.settings.handlearg(opt, arg)
     end
   end
 
   # this is used for testing
   def self.exit(code)
     exit(code)
   end
 
   def name
     self.class.to_s.sub(/.*::/,"").downcase.to_sym
   end
 
   def help
     "No help available for puppet #{name}"
   end
 
   private
 
   def exit_on_fail(message, code = 1)
     yield
   rescue ArgumentError, RuntimeError, NotImplementedError => detail
     puts detail.backtrace if Puppet[:trace]
     $stderr.puts "Could not #{message}: #{detail}"
     exit(code)
   end
 
   def hook(step,&block)
     Puppet::Plugins.send("before_application_#{step}",:application_object => self)
     x = yield
     Puppet::Plugins.send("after_application_#{step}",:application_object => self, :return_value => x)
     x
   end
 end
 end
diff --git a/lib/puppet/application/cert.rb b/lib/puppet/application/cert.rb
index cbd6fd610..c08775380 100644
--- a/lib/puppet/application/cert.rb
+++ b/lib/puppet/application/cert.rb
@@ -1,223 +1,222 @@
 require 'puppet/application'
 
 class Puppet::Application::Cert < Puppet::Application
 
   should_parse_config
   run_mode :master
 
   attr_accessor :all, :ca, :digest, :signed
 
   def subcommand
     @subcommand
   end
   def subcommand=(name)
     # Handle the nasty, legacy mapping of "clean" to "destroy".
     sub = name.to_sym
     @subcommand = (sub == :clean ? :destroy : sub)
   end
 
   option("--clean", "-c") do
     self.subcommand = "destroy"
   end
 
   option("--all", "-a") do
     @all = true
   end
 
   option("--digest DIGEST") do |arg|
     @digest = arg
   end
 
   option("--signed", "-s") do
     @signed = true
   end
 
   option("--debug", "-d") do |arg|
     Puppet::Util::Log.level = :debug
   end
 
   require 'puppet/ssl/certificate_authority/interface'
   Puppet::SSL::CertificateAuthority::Interface::INTERFACE_METHODS.reject {|m| m == :destroy }.each do |method|
     option("--#{method}", "-#{method.to_s[0,1]}") do
       self.subcommand = method
     end
   end
 
   option("--verbose", "-v") do
     Puppet::Util::Log.level = :info
   end
 
   def help
-    puts <<-HELP
+    <<-HELP
 
 puppet-cert(8) -- Manage certificates and requests
 ========
 
 SYNOPSIS
 --------
 Standalone certificate authority. Capable of generating certificates,
 but mostly used for signing certificate requests from puppet clients.
 
 
 USAGE
 -----
 puppet cert [-h|--help] [-V|--version] [-d|--debug] [-v|--verbose]
   [-g|--generate] [-l|--list] [-s|--sign] [-r|--revoke] [-p|--print]
   [-c|--clean] [--verify] [--digest <digest>] [--fingerprint] [host]
 
 
 DESCRIPTION
 -----------
 Because the puppet master service defaults to not signing client
 certificate requests, this script is available for signing outstanding
 requests. It can be used to list outstanding requests and then either
 sign them individually or sign all of them.
 
 
 OPTIONS
 -------
 Note that any configuration parameter that's valid in the configuration
 file is also a valid long argument. For example, 'ssldir' is a valid
 configuration parameter, so you can specify '--ssldir <directory>' as an
 argument.
 
 See the configuration file documentation at
 http://docs.puppetlabs.com/references/stable/configuration.html for the
 full list of acceptable parameters. A commented list of all
 configuration options can also be generated by running puppet cert with
 '--genconfig'.
 
 * --all:
   Operate on all items. Currently only makes sense with '--sign',
   '--clean', or '--list'.
 
 * --digest:
   Set the digest for fingerprinting (defaults to md5). Valid values
   depends on your openssl and openssl ruby extension version, but should
   contain at least md5, sha1, md2, sha256.
 
 * --clean:
   Remove all files related to a host from puppet cert's storage. This is
   useful when rebuilding hosts, since new certificate signing requests
   will only be honored if puppet cert does not have a copy of a signed
   certificate for that host. The certificate of the host is also
   revoked. If '--all' is specified then all host certificates, both
   signed and unsigned, will be removed.
 
 * --debug:
   Enable full debugging.
 
 * --generate:
   Generate a certificate for a named client. A certificate/keypair will
   be generated for each client named on the command line.
 
 * --help:
   Print this help message
 
 * --list:
   List outstanding certificate requests. If '--all' is specified, signed
   certificates are also listed, prefixed by '+', and revoked or invalid
   certificates are prefixed by '-' (the verification outcome is printed
   in parenthesis).
 
 * --print:
   Print the full-text version of a host's certificate.
 
 * --fingerprint:
   Print the DIGEST (defaults to md5) fingerprint of a host's
   certificate.
 
 * --revoke:
   Revoke the certificate of a client. The certificate can be specified
   either by its serial number, given as a decimal number or a
   hexadecimal number prefixed by '0x', or by its hostname. The
   certificate is revoked by adding it to the Certificate Revocation List
   given by the 'cacrl' config parameter. Note that the puppetmasterd
   needs to be restarted after revoking certificates.
 
 * --sign:
   Sign an outstanding certificate request. Unless '--all' is specified,
   hosts must be listed after all flags.
 
 * --verbose:
   Enable verbosity.
 
 * --version:
   Print the puppet version number and exit.
 
 * --verify:
   Verify the named certificate against the local CA certificate.
 
 
 EXAMPLE
 -------
     $ puppet cert -l
     culain.madstop.com
     $ puppet cert -s culain.madstop.com
 
 
 AUTHOR
 ------
 Luke Kanies
 
 
 COPYRIGHT
 ---------
 Copyright (c) 2011 Puppet Labs, LLC Licensed under the Apache 2.0 License
 
     HELP
-    exit
   end
 
   def main
     if @all
       hosts = :all
     elsif @signed
       hosts = :signed
     else
       hosts = command_line.args.collect { |h| h.downcase }
     end
     begin
       @ca.apply(:revoke, :to => hosts) if subcommand == :destroy
       @ca.apply(subcommand, :to => hosts, :digest => @digest)
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       puts detail.to_s
       exit(24)
     end
   end
 
   def setup
     require 'puppet/ssl/certificate_authority'
     exit(Puppet.settings.print_configs ? 0 : 1) if Puppet.settings.print_configs?
 
     Puppet::Util::Log.newdestination :console
 
     if [:generate, :destroy].include? subcommand
       Puppet::SSL::Host.ca_location = :local
     else
       Puppet::SSL::Host.ca_location = :only
     end
 
     begin
       @ca = Puppet::SSL::CertificateAuthority.new
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       puts detail.to_s
       exit(23)
     end
   end
 
   def parse_options
     # handle the bareword subcommand pattern.
     result = super
     unless self.subcommand then
       if sub = self.command_line.args.shift then
         self.subcommand = sub
       else
         help
       end
     end
     result
   end
 end
diff --git a/lib/puppet/application/faces_base.rb b/lib/puppet/application/faces_base.rb
index 288b50048..f1b77f285 100644
--- a/lib/puppet/application/faces_base.rb
+++ b/lib/puppet/application/faces_base.rb
@@ -1,150 +1,158 @@
 require 'puppet/application'
 require 'puppet/faces'
+require 'optparse'
 
 class Puppet::Application::FacesBase < Puppet::Application
   should_parse_config
   run_mode :agent
 
   option("--debug", "-d") do |arg|
     Puppet::Util::Log.level = :debug
   end
 
   option("--verbose", "-v") do
     Puppet::Util::Log.level = :info
   end
 
   option("--format FORMAT") do |arg|
     @format = arg.to_sym
   end
 
   option("--mode RUNMODE", "-r") do |arg|
     raise "Invalid run mode #{arg}; supported modes are user, agent, master" unless %w{user agent master}.include?(arg)
     self.class.run_mode(arg.to_sym)
     set_run_mode self.class.run_mode
   end
 
 
   attr_accessor :face, :action, :type, :arguments, :format
   attr_writer :exit_code
 
   # This allows you to set the exit code if you don't want to just exit
   # immediately but you need to indicate a failure.
   def exit_code
     @exit_code || 0
   end
 
   # Override this if you need custom rendering.
   def render(result)
     render_method = Puppet::Network::FormatHandler.format(format).render_method
     if render_method == "to_pson"
       jj result
       exit(0)
     else
       result.send(render_method)
     end
   end
 
   def preinit
     super
     Signal.trap(:INT) do
       $stderr.puts "Cancelling Face"
       exit(0)
     end
+  end
 
+  def parse_options
     # We need to parse enough of the command line out early, to identify what
     # the action is, so that we can obtain the full set of options to parse.
 
-    # TODO: These should be configurable versions, through a global
+    # REVISIT: These should be configurable versions, through a global
     # '--version' option, but we don't implement that yet... --daniel 2011-03-29
     @type   = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
     @face   = Puppet::Faces[@type, :current]
     @format = @face.default_format
 
     # Now, walk the command line and identify the action.  We skip over
     # arguments based on introspecting the action and all, and find the first
     # non-option word to use as the action.
     action = nil
     index  = -1
     until @action or (index += 1) >= command_line.args.length do
       item = command_line.args[index]
       if item =~ /^-/ then
         option = @face.options.find do |name|
           item =~ /^-+#{name.to_s.gsub(/[-_]/, '[-_]')}(?:[ =].*)?$/
         end
         if option then
           option = @face.get_option(option)
           # If we have an inline argument, just carry on.  We don't need to
           # care about optional vs mandatory in that case because we do a real
           # parse later, and that will totally take care of raising the error
           # when we get there. --daniel 2011-04-04
           if option.takes_argument? and !item.index('=') then
             index += 1 unless
               (option.optional_argument? and command_line.args[index + 1] =~ /^-/)
           end
         elsif option = find_global_settings_argument(item) then
           unless Puppet.settings.boolean? option.name then
             # As far as I can tell, we treat non-bool options as always having
             # a mandatory argument. --daniel 2011-04-05
             index += 1          # ...so skip the argument.
           end
         else
-          raise ArgumentError, "Unknown option #{item.sub(/=.*$/, '').inspect}"
+          raise OptionParser::InvalidOption.new(item.sub(/=.*$/, ''))
         end
       else
         action = @face.get_action(item.to_sym)
         if action.nil? then
-          raise ArgumentError, "#{@face} does not have an #{item.inspect} action!"
+          raise OptionParser::InvalidArgument.new("#{@face} does not have an #{item} action")
         end
         @action = action
       end
     end
 
-    @action or raise ArgumentError, "No action given on the command line!"
+    unless @action
+      raise OptionParser::MissingArgument.new("No action given on the command line")
+    end
 
-    # Finally, we can interact with the default option code to build behaviour
+    # Now we can interact with the default option code to build behaviour
     # around the full set of options we now know we support.
     @action.options.each do |option|
       option = @action.get_option(option) # make it the object.
       self.class.option(*option.optparse) # ...and make the CLI parse it.
     end
+
+    # ...and invoke our parent to parse all the command line options.
+    super
   end
 
   def find_global_settings_argument(item)
     Puppet.settings.each do |name, object|
       object.optparse_args.each do |arg|
         next unless arg =~ /^-/
         # sadly, we have to emulate some of optparse here...
         pattern = /^#{arg.sub('[no-]', '').sub(/[ =].*$/, '')}(?:[ =].*)?$/
         pattern.match item and return object
       end
     end
     return nil                  # nothing found.
   end
 
   def setup
     Puppet::Util::Log.newdestination :console
 
     @arguments = command_line.args
 
     # Note: because of our definition of where the action is set, we end up
     # with it *always* being the first word of the remaining set of command
     # line arguments.  So, strip that off when we construct the arguments to
     # pass down to the face action. --daniel 2011-04-04
     @arguments.delete_at(0)
 
     # We copy all of the app options to the end of the call; This allows each
     # action to read in the options.  This replaces the older model where we
     # would invoke the action with options set as global state in the
     # interface object.  --daniel 2011-03-28
     @arguments << options
   end
 
 
   def main
     # Call the method associated with the provided action (e.g., 'find').
     if result = @face.send(@action.name, *arguments)
       puts render(result)
     end
     exit(exit_code)
   end
 end
diff --git a/lib/puppet/application/help.rb b/lib/puppet/application/help.rb
new file mode 100644
index 000000000..fd8818db0
--- /dev/null
+++ b/lib/puppet/application/help.rb
@@ -0,0 +1,8 @@
+# -*- coding: utf-8 -*-
+require 'puppet/application/faces_base'
+
+class Puppet::Application::Help < Puppet::Application::FacesBase
+  # Meh.  Disable the default behaviour, which is to inspect the
+  # string and return that – not so helpful. --daniel 2011-04-11
+  def render(result) result end
+end
diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb
new file mode 100644
index 000000000..1d8abe20e
--- /dev/null
+++ b/lib/puppet/faces/help.rb
@@ -0,0 +1,104 @@
+require 'puppet/faces'
+require 'puppet/util/command_line'
+require 'pathname'
+require 'erb'
+
+Puppet::Faces.define(:help, '0.0.1') do
+  summary "Displays help about puppet subcommands"
+
+  action(:help) do
+    summary "Display help about faces and their actions."
+
+    option "--version VERSION" do
+      desc "Which version of the interface to show help for"
+    end
+
+    when_invoked do |*args|
+      # Check our invocation, because we want varargs and can't do defaults
+      # yet.  REVISIT: when we do option defaults, and positional options, we
+      # should rewrite this to use those. --daniel 2011-04-04
+      options = args.pop
+      if options.nil? or args.length > 2 then
+        raise ArgumentError, "help only takes two (optional) arguments, a face name, and an action"
+      end
+
+      version = :current
+      if options.has_key? :version then
+        if options[:version].to_s !~ /^current$/i then
+          version = options[:version]
+        else
+          if args.length == 0 then
+            raise ArgumentError, "version only makes sense when a face is given"
+          end
+        end
+      end
+
+      # Name those parameters...
+      facename, actionname = args
+
+      if facename then
+        if legacy_applications.include? facename then
+          actionname and raise ArgumentError, "Legacy subcommands don't take actions"
+          return Puppet::Application[facename].help
+        else
+          face = Puppet::Faces[facename.to_sym, version]
+          actionname and action = face.get_action(actionname.to_sym)
+        end
+      end
+
+      case args.length
+      when 0 then
+        template = erb 'global.erb'
+      when 1 then
+        face or fail ArgumentError, "Unable to load face #{facename}"
+        template = erb 'face.erb'
+      when 2 then
+        face or fail ArgumentError, "Unable to load face #{facename}"
+        action or fail ArgumentError, "Unable to load action #{actionname} from #{face}"
+        template = erb 'action.erb'
+      else
+        fail ArgumentError, "Too many arguments to help action"
+      end
+
+      # Run the ERB template in our current binding, including all the local
+      # variables we established just above. --daniel 2011-04-11
+      return template.result(binding)
+    end
+  end
+
+  def erb(name)
+    template = (Pathname(__FILE__).dirname + "help" + name)
+    erb = ERB.new(template.read, nil, '%')
+    erb.filename = template.to_s
+    return erb
+  end
+
+  def legacy_applications
+    # The list of applications, less those that are duplicated as a face.
+    Puppet::Util::CommandLine.available_subcommands.reject do |appname|
+      Puppet::Faces.face? appname.to_sym, :current or
+        # ...this is a nasty way to exclude non-applications. :(
+        %w{faces_base indirection_base}.include? appname
+    end.sort
+  end
+
+  def horribly_extract_summary_from(appname)
+    begin
+      require "puppet/application/#{appname}"
+      help = Puppet::Application[appname].help.split("\n")
+      # Now we find the line with our summary, extract it, and return it.  This
+      # depends on the implementation coincidence of how our pages are
+      # formatted.  If we can't match the pattern we expect we return the empty
+      # string to ensure we don't blow up in the summary. --daniel 2011-04-11
+      while line = help.shift do
+        if md = /^puppet-#{appname}\([^\)]+\) -- (.*)$/.match(line) then
+          return md[1]
+        end
+      end
+    rescue Exception
+      # Damn, but I hate this: we just ignore errors here, no matter what
+      # class they are.  Meh.
+    end
+    return ''
+  end
+end
diff --git a/lib/puppet/faces/help/action.erb b/lib/puppet/faces/help/action.erb
new file mode 100644
index 000000000..eaf131464
--- /dev/null
+++ b/lib/puppet/faces/help/action.erb
@@ -0,0 +1,3 @@
+Use: puppet <%= face.name %> [options] <%= action.name %> [options]
+
+Summary: <%= action.summary %>
diff --git a/lib/puppet/faces/help/face.erb b/lib/puppet/faces/help/face.erb
new file mode 100644
index 000000000..efe5fd809
--- /dev/null
+++ b/lib/puppet/faces/help/face.erb
@@ -0,0 +1,7 @@
+Use: puppet <%= face.name %> [options] <action> [options]
+
+Available actions:
+% face.actions.each do |actionname|
+%   action = face.get_action(actionname)
+  <%= action.name.to_s.ljust(16) %>  <%= action.summary %>
+% end
diff --git a/lib/puppet/faces/help/global.erb b/lib/puppet/faces/help/global.erb
new file mode 100644
index 000000000..e123367a2
--- /dev/null
+++ b/lib/puppet/faces/help/global.erb
@@ -0,0 +1,20 @@
+puppet <subcommand> [options] <action> [options]
+
+Available subcommands, from Puppet Faces:
+% Puppet::Faces.faces.sort.each do |name|
+%   face = Puppet::Faces[name, :current]
+  <%= face.name.to_s.ljust(16) %>  <%= face.summary %>
+% end
+
+% unless legacy_applications.empty? then # great victory when this is true!
+Available applications, soon to be ported to Faces:
+%   legacy_applications.each do |appname|
+%     summary = horribly_extract_summary_from appname
+  <%= appname.to_s.ljust(16) %>  <%= summary %>
+%   end
+% end
+
+See 'puppet help <subcommand> <action>' for help on a specific subcommand action.
+See 'puppet help <subcommand>' for help on a specific subcommand.
+See 'puppet man  <subcommand>' for the full man page.
+Puppet v<%= Puppet::PUPPETVERSION %>
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 07e27efa8..27b3584b9 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -1,115 +1,120 @@
 require 'puppet'
 require 'puppet/util/autoload'
 
 class Puppet::Interface
   require 'puppet/interface/face_collection'
 
   require 'puppet/interface/action_manager'
   include Puppet::Interface::ActionManager
   extend Puppet::Interface::ActionManager
 
   require 'puppet/interface/option_manager'
   include Puppet::Interface::OptionManager
   extend Puppet::Interface::OptionManager
 
   include Puppet::Util
 
   class << self
     # This is just so we can search for actions.  We only use its
     # list of directories to search.
     # Can't we utilize an external autoloader, or simply use the $LOAD_PATH? -pvb
     def autoloader
       @autoloader ||= Puppet::Util::Autoload.new(:application, "puppet/faces")
     end
 
     def faces
       Puppet::Interface::FaceCollection.faces
     end
 
     def face?(name, version)
       Puppet::Interface::FaceCollection.face?(name, version)
     end
 
     def register(instance)
       Puppet::Interface::FaceCollection.register(instance)
     end
 
     def define(name, version, &block)
       if face?(name, version)
         face = Puppet::Interface::FaceCollection[name, version]
       else
         face = self.new(name, version)
         Puppet::Interface::FaceCollection.register(face)
         # REVISIT: Shouldn't this be delayed until *after* we evaluate the
         # current block, not done before? --daniel 2011-04-07
         face.load_actions
       end
 
       face.instance_eval(&block) if block_given?
 
       return face
     end
 
     def [](name, version)
       unless face = Puppet::Interface::FaceCollection[name, version]
         if current = Puppet::Interface::FaceCollection[name, :current]
           raise Puppet::Error, "Could not find version #{version} of #{current}"
         else
           raise Puppet::Error, "Could not find Puppet Face #{name.inspect}"
         end
       end
       face
     end
   end
 
   attr_accessor :default_format
 
   def set_default_format(format)
     self.default_format = format.to_sym
   end
 
-  attr_accessor :type, :verb, :version, :arguments
-  attr_reader :name
+  attr_accessor :summary
+  def summary(value = nil)
+    @summary = value unless value.nil?
+    @summary
+  end
+
+  attr_reader :name, :version
 
   def initialize(name, version, &block)
     unless Puppet::Interface::FaceCollection.validate_version(version)
       raise ArgumentError, "Cannot create face #{name.inspect} with invalid version number '#{version}'!"
     end
 
     @name = Puppet::Interface::FaceCollection.underscorize(name)
     @version = version
     @default_format = :pson
 
     instance_eval(&block) if block_given?
   end
 
   # Try to find actions defined in other files.
   def load_actions
     path = "puppet/faces/#{name}"
 
     loaded = []
     [path, "#{name}@#{version}/#{path}"].each do |path|
       Puppet::Interface.autoloader.search_directories.each do |dir|
         fdir = ::File.join(dir, path)
         next unless FileTest.directory?(fdir)
 
         Dir.chdir(fdir) do
           Dir.glob("*.rb").each do |file|
             aname = file.sub(/\.rb/, '')
             if loaded.include?(aname)
               Puppet.debug "Not loading duplicate action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
               next
             end
             loaded << aname
             Puppet.debug "Loading action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
             require "#{Dir.pwd}/#{aname}"
           end
         end
       end
     end
   end
 
   def to_s
     "Puppet::Faces[#{name.inspect}, #{version.inspect}]"
   end
 end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index e4a37a1f7..302e61901 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -1,119 +1,120 @@
 # -*- coding: utf-8 -*-
 require 'puppet/interface'
 require 'puppet/interface/option'
 
 class Puppet::Interface::Action
   def initialize(face, name, attrs = {})
     raise "#{name.inspect} is an invalid action name" unless name.to_s =~ /^[a-z]\w*$/
     @face    = face
     @name    = name.to_sym
     @options = {}
     attrs.each do |k, v| send("#{k}=", v) end
   end
 
   attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
+  attr_accessor :summary
 
   # Initially, this was defined to allow the @action.invoke pattern, which is
   # a very natural way to invoke behaviour given our introspection
   # capabilities.   Heck, our initial plan was to have the faces delegate to
   # the action object for invocation and all.
   #
   # It turns out that we have a binding problem to solve: @face was bound to
   # the parent class, not the subclass instance, and we don't pass the
   # appropriate context or change the binding enough to make this work.
   #
   # We could hack around it, by either mandating that you pass the context in
   # to invoke, or try to get the binding right, but that has probably got
   # subtleties that we don't instantly think of – especially around threads.
   #
   # So, we are pulling this method for now, and will return it to life when we
   # have the time to resolve the problem.  For now, you should replace...
   #
   #     @action = @face.get_action(name)
   #     @action.invoke(arg1, arg2, arg3)
   #
   # ...with...
   #
   #     @action = @face.get_action(name)
   #     @face.send(@action.name, arg1, arg2, arg3)
   #
   # I understand that is somewhat cumbersome, but it functions as desired.
   # --daniel 2011-03-31
   #
   # PS: This code is left present, but commented, to support this chunk of
   # documentation, for the benefit of the reader.
   #
   # def invoke(*args, &block)
   #   @face.send(name, *args, &block)
   # end
 
   def when_invoked=(block)
     # We need to build an instance method as a wrapper, using normal code, to
     # be able to expose argument defaulting between the caller and definer in
     # the Ruby API.  An extra method is, sadly, required for Ruby 1.8 to work.
     #
     # In future this also gives us a place to hook in additional behaviour
     # such as calling out to the action instance to validate and coerce
     # parameters, which avoids any exciting context switching and all.
     #
     # Hopefully we can improve this when we finally shuffle off the last of
     # Ruby 1.8 support, but that looks to be a few "enterprise" release eras
     # away, so we are pretty stuck with this for now.
     #
     # Patches to make this work more nicely with Ruby 1.9 using runtime
     # version checking and all are welcome, but they can't actually help if
     # the results are not totally hidden away in here.
     #
     # Incidentally, we though about vendoring evil-ruby and actually adjusting
     # the internal C structure implementation details under the hood to make
     # this stuff work, because it would have been cleaner.  Which gives you an
     # idea how motivated we were to make this cleaner.  Sorry. --daniel 2011-03-31
 
     internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym
     file = __FILE__ + "+eval"
     line = __LINE__ + 1
     wrapper = "def #{@name}(*args, &block)
                  args << {} unless args.last.is_a? Hash
                  args << block if block_given?
                  self.__send__(#{internal_name.inspect}, *args)
                end"
 
     if @face.is_a?(Class)
       @face.class_eval do eval wrapper, nil, file, line end
       @face.define_method(internal_name, &block)
     else
       @face.instance_eval do eval wrapper, nil, file, line end
       @face.meta_def(internal_name, &block)
     end
   end
 
   def add_option(option)
     option.aliases.each do |name|
       if conflict = get_option(name) then
         raise ArgumentError, "Option #{option} conflicts with existing option #{conflict}"
       elsif conflict = @face.get_option(name) then
         raise ArgumentError, "Option #{option} conflicts with existing option #{conflict} on #{@face}"
       end
     end
 
     option.aliases.each do |name|
       @options[name] = option
     end
 
     option
   end
 
   def option?(name)
     @options.include? name.to_sym
   end
 
   def options
     (@options.keys + @face.options).sort
   end
 
   def get_option(name)
     @options[name.to_sym] || @face.get_option(name)
   end
 end
diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index b08c3d023..34bb3fa44 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -1,31 +1,35 @@
 require 'puppet/interface'
 require 'puppet/interface/action'
 
 class Puppet::Interface::ActionBuilder
   attr_reader :action
 
   def self.build(face, name, &block)
     raise "Action #{name.inspect} must specify a block" unless block
     new(face, name, &block).action
   end
 
   private
   def initialize(face, name, &block)
     @face   = face
     @action = Puppet::Interface::Action.new(face, name)
     instance_eval(&block)
   end
 
   # Ideally the method we're defining here would be added to the action, and a
   # method on the face would defer to it, but we can't get scope correct, so
   # we stick with this. --daniel 2011-03-24
   def when_invoked(&block)
     raise "when_invoked on an ActionBuilder with no corresponding Action" unless @action
     @action.when_invoked = block
   end
 
   def option(*declaration, &block)
     option = Puppet::Interface::OptionBuilder.build(@action, *declaration, &block)
     @action.add_option(option)
   end
+
+  def summary(text)
+    @action.summary = text
+  end
 end
diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb
index 84296582c..e4eb22fa3 100644
--- a/lib/puppet/interface/face_collection.rb
+++ b/lib/puppet/interface/face_collection.rb
@@ -1,123 +1,131 @@
 # -*- coding: utf-8 -*-
 require 'puppet/interface'
 
 module Puppet::Interface::FaceCollection
   SEMVER_VERSION = /^(\d+)\.(\d+)\.(\d+)([A-Za-z][0-9A-Za-z-]*|)$/
 
   @faces = Hash.new { |hash, key| hash[key] = {} }
 
   def self.faces
     unless @loaded
       @loaded = true
       $LOAD_PATH.each do |dir|
         next unless FileTest.directory?(dir)
         Dir.chdir(dir) do
           Dir.glob("puppet/faces/*.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 @faces.keys
   end
 
   def self.validate_version(version)
     !!(SEMVER_VERSION =~ version.to_s)
   end
 
   def self.cmp_semver(a, b)
     a, b = [a, b].map do |x|
       parts = SEMVER_VERSION.match(x).to_a[1..4]
       parts[0..2] = parts[0..2].map { |e| e.to_i }
       parts
     end
 
     cmp = a[0..2] <=> b[0..2]
     if cmp == 0
       cmp = a[3] <=> b[3]
       cmp = +1 if a[3].empty? && !b[3].empty?
       cmp = -1 if b[3].empty? && !a[3].empty?
     end
     cmp
   end
 
   def self.[](name, version)
     @faces[underscorize(name)][version] if face?(name, version)
   end
 
   def self.face?(name, version)
     name = underscorize(name)
-    return true if @faces[name].has_key?(version)
+
+    # Note: be careful not to accidentally create the top level key, either,
+    # because it will result in confusion when people try to enumerate the
+    # list of valid faces later. --daniel 2011-04-11
+    return true if @faces.has_key?(name) and @faces[name].has_key?(version)
 
     # We always load the current version file; the common case is that we have
     # the expected version and any compatibility versions in the same file,
     # the default.  Which means that this is almost always the case.
     #
     # We use require to avoid executing the code multiple times, like any
     # other Ruby library that we might want to use.  --daniel 2011-04-06
     begin
       require "puppet/faces/#{name}"
 
       # If we wanted :current, we need to index to find that; direct version
       # requests just work™ as they go. --daniel 2011-04-06
       if version == :current then
         # We need to find current out of this.  This is the largest version
         # number that doesn't have a dedicated on-disk file present; those
         # represent "experimental" versions of faces, which we don't fully
         # support yet.
         #
         # We walk the versions from highest to lowest and take the first version
         # that is not defined in an explicitly versioned file on disk as the
         # current version.
         #
         # This constrains us to only ship experimental versions with *one*
         # version in the file, not multiple, but given you can't reliably load
         # them except by side-effect when you ignore that rule this seems safe
         # enough...
         #
         # Given those constraints, and that we are not going to ship a versioned
         # interface that is not :current in this release, we are going to leave
         # these thoughts in place, and just punt on the actual versioning.
         #
         # When we upgrade the core to support multiple versions we can solve the
         # problems then; as lazy as possible.
         #
         # We do support multiple versions in the same file, though, so we sort
         # versions here and return the last item in that set.
         #
         # --daniel 2011-04-06
         latest_ver = @faces[name].keys.sort {|a, b| cmp_semver(a, b) }.last
         @faces[name][:current] = @faces[name][latest_ver]
       end
     rescue LoadError => e
       raise unless e.message =~ %r{-- puppet/faces/#{name}$}
       # ...guess we didn't find the file; return a much better problem.
     end
 
     # Now, either we have the version in our set of faces, or we didn't find
     # the version they were looking for.  In the future we will support
     # loading versioned stuff from some look-aside part of the Ruby load path,
     # but we don't need that right now.
     #
     # So, this comment is a place-holder for that.  --daniel 2011-04-06
-    return !! @faces[name].has_key?(version)
+    #
+    # Note: be careful not to accidentally create the top level key, either,
+    # because it will result in confusion when people try to enumerate the
+    # list of valid faces later. --daniel 2011-04-11
+    return !! (@faces.has_key?(name) and @faces[name].has_key?(version))
   end
 
   def self.register(face)
     @faces[underscorize(face.name)][face.version] = face
   end
 
   def self.underscorize(name)
     unless name.to_s =~ /^[-_a-z]+$/i then
       raise ArgumentError, "#{name.inspect} (#{name.class}) is not a valid face name"
     end
 
     name.to_s.downcase.split(/[-_]/).join('_').to_sym
   end
 end
diff --git a/lib/puppet/util/command_line.rb b/lib/puppet/util/command_line.rb
index 52b5f81ef..fa462ee2d 100644
--- a/lib/puppet/util/command_line.rb
+++ b/lib/puppet/util/command_line.rb
@@ -1,102 +1,112 @@
 require "puppet/util/plugins"
 
 module Puppet
   module Util
     class CommandLine
 
       LegacyName = Hash.new{|h,k| k}.update(
         'agent'      => 'puppetd',
         'cert'       => 'puppetca',
         'doc'        => 'puppetdoc',
         'filebucket' => 'filebucket',
         'apply'      => 'puppet',
         'describe'   => 'pi',
         'queue'      => 'puppetqd',
         'resource'   => 'ralsh',
         'kick'       => 'puppetrun',
         'master'     => 'puppetmasterd'
       )
 
-      def initialize( zero = $0, argv = ARGV, stdin = STDIN )
+      def initialize(zero = $0, argv = ARGV, stdin = STDIN)
         @zero  = zero
         @argv  = argv.dup
         @stdin = stdin
 
-        @subcommand_name, @args = subcommand_and_args( @zero, @argv, @stdin )
+        @subcommand_name, @args = subcommand_and_args(@zero, @argv, @stdin)
         Puppet::Plugins.on_commandline_initialization(:command_line_object => self)
       end
 
       attr :subcommand_name
       attr :args
 
       def appdir
         File.join('puppet', 'application')
       end
 
-      def available_subcommands
-        absolute_appdirs = $LOAD_PATH.collect do |x| 
+      def self.available_subcommands
+        absolute_appdirs = $LOAD_PATH.collect do |x|
           File.join(x,'puppet','application')
         end.select{ |x| File.directory?(x) }
         absolute_appdirs.inject([]) do |commands, dir|
           commands + Dir[File.join(dir, '*.rb')].map{|fn| File.basename(fn, '.rb')}
         end.uniq
       end
-
-      def usage_message
-        usage = "Usage: puppet command <space separated arguments>"
-        available = "Available commands are: #{available_subcommands.sort.join(', ')}"
-        [usage, available].join("\n")
+      # available_subcommands was previously an instance method, not a class
+      # method, and we have an unknown number of user-implemented applications
+      # that depend on that behaviour.  Forwarding allows us to preserve a
+      # backward compatible API. --daniel 2011-04-11
+      def available_subcommands
+        self.class.available_subcommands
       end
 
       def require_application(application)
         require File.join(appdir, application)
       end
 
       def execute
-        if subcommand_name.nil?
-          puts usage_message
-        elsif available_subcommands.include?(subcommand_name) #subcommand
+        if subcommand_name and available_subcommands.include?(subcommand_name) then
           require_application subcommand_name
           app = Puppet::Application.find(subcommand_name).new(self)
           Puppet::Plugins.on_application_initialization(:appliation_object => self)
           app.run
+        elsif execute_external_subcommand then
+          # Logically, we shouldn't get here, but we do, so whatever.  We just
+          # return to the caller.  How strange we are. --daniel 2011-04-11
         else
-          abort "Error: Unknown command #{subcommand_name}.\n#{usage_message}" unless execute_external_subcommand
+          unless subcommand_name.nil? then
+            puts "Error: Unknown Puppet subcommand #{subcommand_name}.\n"
+          end
+
+          # Doing this at the top of the file is natural, but causes puppet.rb
+          # to load too early, which causes things to break.  This is a nasty
+          # thing, found in #7065. --daniel 2011-04-11
+          require 'puppet/faces/help'
+          puts Puppet::Faces[:help, :current].help
         end
       end
 
       def execute_external_subcommand
         external_command = "puppet-#{subcommand_name}"
 
         require 'puppet/util'
-        path_to_subcommand = Puppet::Util.which( external_command )
+        path_to_subcommand = Puppet::Util.which(external_command)
         return false unless path_to_subcommand
 
-        system( path_to_subcommand, *args )
+        system(path_to_subcommand, *args)
         true
       end
 
       def legacy_executable_name
         LegacyName[ subcommand_name ]
       end
 
       private
 
-      def subcommand_and_args( zero, argv, stdin )
+      def subcommand_and_args(zero, argv, stdin)
         zero = File.basename(zero, '.rb')
 
         if zero == 'puppet'
           case argv.first
           when nil;              [ stdin.tty? ? nil : "apply", argv] # ttys get usage info
           when "--help", "-h";         [nil,     argv] # help should give you usage, not the help for `puppet apply`
           when /^-|\.pp$|\.rb$/; ["apply", argv]
           else [ argv.first, argv[1..-1] ]
           end
         else
           [ zero, argv ]
         end
       end
 
     end
   end
 end
diff --git a/spec/lib/puppet/faces/basetest.rb b/spec/lib/puppet/faces/basetest.rb
new file mode 100644
index 000000000..d20c52b97
--- /dev/null
+++ b/spec/lib/puppet/faces/basetest.rb
@@ -0,0 +1 @@
+Puppet::Faces.define(:basetest, '0.0.1')
diff --git a/spec/lib/puppet/faces/huzzah.rb b/spec/lib/puppet/faces/huzzah.rb
index 735004475..e86730250 100644
--- a/spec/lib/puppet/faces/huzzah.rb
+++ b/spec/lib/puppet/faces/huzzah.rb
@@ -1,4 +1,5 @@
 require 'puppet/faces'
 Puppet::Faces.define(:huzzah, '2.0.1') do
+  summary "life is a thing for celebration"
   action :bar do "is where beer comes from" end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index d28cb2504..1187c1caf 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -1,67 +1,74 @@
 dir = File.expand_path(File.dirname(__FILE__))
 $LOAD_PATH.unshift File.join(dir, 'lib')
 
 # Don't want puppet getting the command line arguments for rake or autotest
 ARGV.clear
 
 require 'puppet'
 require 'mocha'
 gem 'rspec', '>=2.0.0'
+require 'rspec/expectations'
 
 # So everyone else doesn't have to include this base constant.
 module PuppetSpec
   FIXTURE_DIR = File.join(dir = File.expand_path(File.dirname(__FILE__)), "fixtures") unless defined?(FIXTURE_DIR)
 end
 
 require 'pathname'
 require 'tmpdir'
 
 require 'lib/puppet_spec/verbose'
 require 'lib/puppet_spec/files'
 require 'lib/puppet_spec/fixtures'
 require 'monkey_patches/alias_should_to_must'
 require 'monkey_patches/publicize_methods'
 
 Pathname.glob("#{dir}/shared_behaviours/**/*.rb") do |behaviour|
   require behaviour.relative_path_from(Pathname.new(dir))
 end
 
 RSpec.configure do |config|
   include PuppetSpec::Fixtures
 
   config.mock_with :mocha
 
   config.before :each do
     GC.disable
 
     # these globals are set by Application
     $puppet_application_mode = nil
     $puppet_application_name = nil
     Signal.stubs(:trap)
 
     # Set the confdir and vardir to gibberish so that tests
     # have to be correctly mocked.
     Puppet[:confdir] = "/dev/null"
     Puppet[:vardir] = "/dev/null"
 
     # Avoid opening ports to the outside world
     Puppet.settings[:bindaddress] = "127.0.0.1"
 
     @logs = []
     Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(@logs))
   end
 
   config.after :each do
     Puppet.settings.clear
     Puppet::Node::Environment.clear
     Puppet::Util::Storage.clear
     Puppet::Util::ExecutionStub.reset
 
     PuppetSpec::Files.cleanup
 
     @logs.clear
     Puppet::Util::Log.close_all
 
     GC.enable
   end
 end
+
+RSpec::Matchers.define :have_matching_element do |expected|
+  match do |actual|
+    actual.any? { |item| item =~ expected }
+  end
+end
diff --git a/spec/unit/application/cert_spec.rb b/spec/unit/application/cert_spec.rb
index 5b25ab7b8..a1b5eb19a 100755
--- a/spec/unit/application/cert_spec.rb
+++ b/spec/unit/application/cert_spec.rb
@@ -1,234 +1,222 @@
-#!/usr/bin/env ruby
-
+#!/usr/bin/env rspec
 require 'spec_helper'
-
 require 'puppet/application/cert'
 
 describe Puppet::Application::Cert do
   before :each do
     @cert_app = Puppet::Application[:cert]
     Puppet::Util::Log.stubs(:newdestination)
     Puppet::Util::Log.stubs(:level=)
   end
 
   it "should operate in master run_mode" do
     @cert_app.class.run_mode.name.should equal(:master)
   end
 
   it "should ask Puppet::Application to parse Puppet configuration file" do
     @cert_app.should_parse_config?.should be_true
   end
 
   it "should declare a main command" do
     @cert_app.should respond_to(:main)
   end
 
   Puppet::SSL::CertificateAuthority::Interface::INTERFACE_METHODS.reject{ |m| m == :destroy }.each do |method|
     it "should declare option --#{method}" do
       @cert_app.should respond_to("handle_#{method}".to_sym)
     end
   end
 
   it "should set log level to info with the --verbose option" do
 
     Puppet::Log.expects(:level=).with(:info)
 
     @cert_app.handle_verbose(0)
   end
 
   it "should set log level to debug with the --debug option" do
 
     Puppet::Log.expects(:level=).with(:debug)
 
     @cert_app.handle_debug(0)
   end
 
   it "should set the fingerprint digest with the --digest option" do
     @cert_app.handle_digest(:digest)
 
     @cert_app.digest.should == :digest
   end
 
   it "should set cert_mode to :destroy for --clean" do
     @cert_app.handle_clean(0)
     @cert_app.subcommand.should == :destroy
   end
 
   it "should set all to true for --all" do
     @cert_app.handle_all(0)
     @cert_app.all.should be_true
   end
 
   it "should set signed to true for --signed" do
     @cert_app.handle_signed(0)
     @cert_app.signed.should be_true
   end
 
   Puppet::SSL::CertificateAuthority::Interface::INTERFACE_METHODS.reject { |m| m == :destroy }.each do |method|
     it "should set cert_mode to #{method} with option --#{method}" do
       @cert_app.send("handle_#{method}".to_sym, nil)
 
       @cert_app.subcommand.should == method
     end
   end
 
   describe "during setup" do
 
     before :each do
       Puppet::Log.stubs(:newdestination)
       Puppet::SSL::Host.stubs(:ca_location=)
       Puppet::SSL::CertificateAuthority.stubs(:new)
     end
 
     it "should set console as the log destination" do
       Puppet::Log.expects(:newdestination).with(:console)
 
       @cert_app.setup
     end
 
     it "should print puppet config if asked to in Puppet config" do
       @cert_app.stubs(:exit)
       Puppet.settings.stubs(:print_configs?).returns(true)
 
       Puppet.settings.expects(:print_configs)
 
       @cert_app.setup
     end
 
     it "should exit after printing puppet config if asked to in Puppet config" do
       Puppet.settings.stubs(:print_configs?).returns(true)
 
       lambda { @cert_app.setup }.should raise_error(SystemExit)
     end
 
     it "should set the CA location to 'only'" do
       Puppet::SSL::Host.expects(:ca_location=).with(:only)
 
       @cert_app.setup
     end
 
     it "should create a new certificate authority" do
       Puppet::SSL::CertificateAuthority.expects(:new)
 
       @cert_app.setup
     end
 
     it "should set the ca_location to :local if the cert_mode is generate" do
       @cert_app.subcommand = 'generate'
       Puppet::SSL::Host.expects(:ca_location=).with(:local)
       @cert_app.setup
     end
 
     it "should set the ca_location to :local if the cert_mode is destroy" do
       @cert_app.subcommand = 'destroy'
       Puppet::SSL::Host.expects(:ca_location=).with(:local)
       @cert_app.setup
     end
 
     it "should set the ca_location to :only if the cert_mode is print" do
       @cert_app.subcommand = 'print'
       Puppet::SSL::Host.expects(:ca_location=).with(:only)
       @cert_app.setup
     end
   end
 
   describe "when running" do
     before :each do
       @cert_app.all = false
       @ca = stub_everything 'ca'
       @cert_app.ca = @ca
       @cert_app.command_line.stubs(:args).returns([])
     end
 
     it "should delegate to the CertificateAuthority" do
       @ca.expects(:apply)
 
       @cert_app.main
     end
 
     it "should delegate with :all if option --all was given" do
       @cert_app.handle_all(0)
 
       @ca.expects(:apply).with { |cert_mode,to| to[:to] == :all }
 
       @cert_app.main
     end
 
     it "should delegate to ca.apply with the hosts given on command line" do
       @cert_app.command_line.stubs(:args).returns(["host"])
 
       @ca.expects(:apply).with { |cert_mode,to| to[:to] == ["host"]}
 
       @cert_app.main
     end
 
     it "should send the currently set digest" do
       @cert_app.command_line.stubs(:args).returns(["host"])
       @cert_app.handle_digest(:digest)
 
       @ca.expects(:apply).with { |cert_mode,to| to[:digest] == :digest}
 
       @cert_app.main
     end
 
     it "should revoke cert if cert_mode is clean" do
       @cert_app.subcommand = :destroy
       @cert_app.command_line.stubs(:args).returns(["host"])
 
       @ca.expects(:apply).with { |cert_mode,to| cert_mode == :revoke }
       @ca.expects(:apply).with { |cert_mode,to| cert_mode == :destroy }
 
       @cert_app.main
     end
   end
 
   describe "when identifying subcommands" do
     before :each do
       @cert_app.all = false
       @ca = stub_everything 'ca'
       @cert_app.ca = @ca
     end
 
-    it "should SystemExit after printing help message" do
-      # Make the help method silent for testing; this is a bit nasty, but we
-      # can't identify a cleaner method.  Help welcome. --daniel 2011-02-22
-      Puppet.features.stubs(:usage?).returns(false)
-      @cert_app.stubs(:puts)
-
-      @cert_app.command_line.stubs(:args).returns([])
-      expect { @cert_app.parse_options }.should raise_error SystemExit
-    end
-
     %w{list revoke generate sign print verify fingerprint}.each do |cmd|
       short = cmd[0,1]
       [cmd, "--#{cmd}", "-#{short}"].each do |option|
         # In our command line '-v' was eaten by 'verbose', so we can't consume
         # it here; this is a special case from our otherwise standard
         # processing. --daniel 2011-02-22
         next if option == "-v"
 
         it "should recognise '#{option}'" do
           args = [option, "fun.example.com"]
 
           @cert_app.command_line.stubs(:args).returns(args)
           @cert_app.parse_options
           @cert_app.subcommand.should == cmd.to_sym
 
           args.should == ["fun.example.com"]
         end
       end
     end
 
     %w{clean --clean -c}.each do |ugly|
       it "should recognise the '#{ugly}' option as destroy" do
         args = [ugly, "fun.example.com"]
 
         @cert_app.command_line.stubs(:args).returns(args)
         @cert_app.parse_options
         @cert_app.subcommand.should == :destroy
 
         args.should == ["fun.example.com"]
       end
     end
   end
 end
diff --git a/spec/unit/application/certificate_spec.rb b/spec/unit/application/certificate_spec.rb
index 6153d9538..27d6ac81b 100755
--- a/spec/unit/application/certificate_spec.rb
+++ b/spec/unit/application/certificate_spec.rb
@@ -1,17 +1,20 @@
 require 'puppet/application/certificate'
 
 describe Puppet::Application::Certificate do
   it "should have a 'ca-location' option" do
     # REVISIT: This is delegated from the face, and we will have a test there,
     # so is this actually a valuable test? --daniel 2011-04-07
     subject.command_line.stubs(:args).returns %w{list}
     subject.preinit
+    subject.parse_options
     subject.should respond_to(:handle_ca_location)
   end
 
   it "should accept the ca-location option" do
     subject.command_line.stubs(:args).returns %w{--ca-location local list}
-    subject.preinit and subject.parse_options and subject.setup
+    subject.preinit
+    subject.parse_options
+    subject.setup
     subject.arguments.should == [{ :ca_location => "local" }]
   end
 end
diff --git a/spec/unit/application/faces_base_spec.rb b/spec/unit/application/faces_base_spec.rb
index b7a11ad56..18bd30295 100755
--- a/spec/unit/application/faces_base_spec.rb
+++ b/spec/unit/application/faces_base_spec.rb
@@ -1,192 +1,188 @@
-#!/usr/bin/env ruby
+#!/usr/bin/env rspec
 
 require 'spec_helper'
 require 'puppet/application/faces_base'
 require 'tmpdir'
 
 class Puppet::Application::FacesBase::Basetest < Puppet::Application::FacesBase
 end
 
 describe Puppet::Application::FacesBase do
   before :all do
-    @dir = Dir.mktmpdir
-    $LOAD_PATH.push(@dir)
-    FileUtils.mkdir_p(File.join @dir, 'puppet', 'faces')
-    File.open(File.join(@dir, 'puppet', 'faces', 'basetest.rb'), 'w') do |f|
-      f.puts "Puppet::Faces.define(:basetest, '0.0.1')"
-    end
-
     Puppet::Faces.define(:basetest, '0.0.1') do
       option("--[no-]boolean")
       option("--mandatory MANDATORY")
       option("--optional [OPTIONAL]")
 
       action :foo do
         option("--action")
         when_invoked { |*args| args.length }
       end
     end
   end
 
-  after :all do
-    FileUtils.remove_entry_secure @dir
-    $LOAD_PATH.pop
-  end
-
   let :app do
     app = Puppet::Application::FacesBase::Basetest.new
-    app.stubs(:exit)
-    app.stubs(:puts)
-    app.command_line.stubs(:subcommand_name).returns 'subcommand'
+    app.command_line.stubs(:subcommand_name).returns('subcommand')
     Puppet::Util::Log.stubs(:newdestination)
     app
   end
 
   describe "#find_global_settings_argument" do
     it "should not match --ca to --ca-location" do
       option = mock('ca option', :optparse_args => ["--ca"])
       Puppet.settings.expects(:each).yields(:ca, option)
 
       app.find_global_settings_argument("--ca-location").should be_nil
     end
   end
 
-  describe "#preinit" do
+  describe "#parse_options" do
     before :each do
       app.command_line.stubs(:args).returns %w{}
     end
 
     describe "parsing the command line" do
       context "with just an action" do
         before :all do
           # We have to stub Signal.trap to avoid a crazy mess where we take
           # over signal handling and make it impossible to cancel the test
           # suite run.
           #
           # It would be nice to fix this elsewhere, but it is actually hard to
           # capture this in rspec 2.5 and all. :(  --daniel 2011-04-08
           Signal.stubs(:trap)
           app.command_line.stubs(:args).returns %w{foo}
           app.preinit
+          app.parse_options
         end
 
         it "should set the faces based on the type" do
           app.face.name.should == :basetest
         end
 
         it "should set the format based on the faces default" do
           app.format.should == :pson
         end
 
         it "should find the action" do
           app.action.should be
           app.action.name.should == :foo
         end
       end
 
       it "should fail if no action is given" do
-        expect { app.preinit }.
-          should raise_error ArgumentError, /No action given/
+        expect { app.preinit; app.parse_options }.
+          to raise_error OptionParser::MissingArgument, /No action given/
       end
 
       it "should report a sensible error when options with = fail" do
         app.command_line.stubs(:args).returns %w{--action=bar foo}
-        expect { app.preinit }.
-          should raise_error ArgumentError, /Unknown option "--action"/
+        expect { app.preinit; app.parse_options }.
+          to raise_error OptionParser::InvalidOption, /invalid option: --action/
       end
 
       it "should fail if an action option is before the action" do
         app.command_line.stubs(:args).returns %w{--action foo}
-        expect { app.preinit }.
-          should raise_error ArgumentError, /Unknown option "--action"/
+        expect { app.preinit; app.parse_options }.
+          to raise_error OptionParser::InvalidOption, /invalid option: --action/
       end
 
       it "should fail if an unknown option is before the action" do
         app.command_line.stubs(:args).returns %w{--bar foo}
-        expect { app.preinit }.
-          should raise_error ArgumentError, /Unknown option "--bar"/
+        expect { app.preinit; app.parse_options }.
+          to raise_error OptionParser::InvalidOption, /invalid option: --bar/
       end
 
-      it "should not fail if an unknown option is after the action" do
+      it "should fail if an unknown option is after the action" do
         app.command_line.stubs(:args).returns %w{foo --bar}
-        app.preinit
-        app.action.name.should == :foo
-        app.face.should_not be_option :bar
-        app.action.should_not be_option :bar
+        expect { app.preinit; app.parse_options }.
+          to raise_error OptionParser::InvalidOption, /invalid option: --bar/
       end
 
       it "should accept --bar as an argument to a mandatory option after action" do
         app.command_line.stubs(:args).returns %w{foo --mandatory --bar}
-        app.preinit and app.parse_options
+        app.preinit
+        app.parse_options
         app.action.name.should == :foo
         app.options.should == { :mandatory => "--bar" }
       end
 
       it "should accept --bar as an argument to a mandatory option before action" do
         app.command_line.stubs(:args).returns %w{--mandatory --bar foo}
-        app.preinit and app.parse_options
+        app.preinit
+        app.parse_options
         app.action.name.should == :foo
         app.options.should == { :mandatory => "--bar" }
       end
 
       it "should not skip when --foo=bar is given" do
         app.command_line.stubs(:args).returns %w{--mandatory=bar --bar foo}
-        expect { app.preinit }.
-          should raise_error ArgumentError, /Unknown option "--bar"/
+        expect { app.preinit; app.parse_options }.
+          to raise_error OptionParser::InvalidOption, /invalid option: --bar/
       end
 
       { "boolean options before" => %w{--trace foo},
         "boolean options after"  => %w{foo --trace}
       }.each do |name, args|
         it "should accept global boolean settings #{name} the action" do
           app.command_line.stubs(:args).returns args
-          app.preinit && app.parse_options
+          app.preinit
+          app.parse_options
           Puppet[:trace].should be_true
         end
       end
 
       { "before" => %w{--syslogfacility user1 foo},
         " after" => %w{foo --syslogfacility user1}
       }.each do |name, args|
         it "should accept global settings with arguments #{name} the action" do
           app.command_line.stubs(:args).returns args
-          app.preinit && app.parse_options
+          app.preinit
+          app.parse_options
           Puppet[:syslogfacility].should == "user1"
         end
       end
     end
   end
 
   describe "#setup" do
     it "should remove the action name from the arguments" do
       app.command_line.stubs(:args).returns %w{--mandatory --bar foo}
-      app.preinit and app.parse_options and app.setup
+      app.preinit
+      app.parse_options
+      app.setup
       app.arguments.should == [{ :mandatory => "--bar" }]
     end
 
     it "should pass positional arguments" do
       app.command_line.stubs(:args).returns %w{--mandatory --bar foo bar baz quux}
-      app.preinit and app.parse_options and app.setup
+      app.preinit
+      app.parse_options
+      app.setup
       app.arguments.should == ['bar', 'baz', 'quux', { :mandatory => "--bar" }]
     end
   end
 
   describe "#main" do
-    before do
+    before :each do
+      app.expects(:exit).with(0)
+
       app.face      = Puppet::Faces[:basetest, '0.0.1']
       app.action    = app.face.get_action(:foo)
       app.format    = :pson
       app.arguments = ["myname", "myarg"]
     end
 
     it "should send the specified verb and name to the faces" do
       app.face.expects(:foo).with(*app.arguments)
       app.main
     end
 
     it "should use its render method to render any result" do
       app.expects(:render).with(app.arguments.length + 1)
+      app.stubs(:puts)          # meh.  Don't print nil, thanks. --daniel 2011-04-12
       app.main
     end
   end
 end
diff --git a/spec/unit/application/indirection_base_spec.rb b/spec/unit/application/indirection_base_spec.rb
index 10ebe8e3d..98eb3a118 100755
--- a/spec/unit/application/indirection_base_spec.rb
+++ b/spec/unit/application/indirection_base_spec.rb
@@ -1,39 +1,39 @@
 #!/usr/bin/env ruby
 
 require 'spec_helper'
 require 'puppet/application/indirection_base'
 require 'puppet/faces/indirector'
 
 ########################################################################
 # Stub for testing; the names are critical, sadly. --daniel 2011-03-30
 class Puppet::Application::TestIndirection < Puppet::Application::IndirectionBase
 end
 
 face = Puppet::Faces::Indirector.define(:testindirection, '0.0.1') do
 end
 # REVISIT: This horror is required because we don't allow anything to be
 # :current except for if it lives on, and is loaded from, disk. --daniel 2011-03-29
-face.version = :current
+face.instance_variable_set('@version', :current)
 Puppet::Faces.register(face)
 ########################################################################
 
 
 describe Puppet::Application::IndirectionBase do
   subject { Puppet::Application::TestIndirection.new }
 
   it "should accept a terminus command line option" do
     # It would be nice not to have to stub this, but whatever... writing an
     # entire indirection stack would cause us more grief. --daniel 2011-03-31
     terminus = mock("test indirection terminus")
     Puppet::Indirector::Indirection.expects(:instance).
       with(:testindirection).twice.returns()
 
     subject.command_line.
       instance_variable_set('@args', %w{--terminus foo save})
 
     # Not a very nice thing. :(
     $stderr.stubs(:puts)
 
     expect { subject.run }.should raise_error SystemExit
   end
 end
diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb
index 740b76f62..a1a46c814 100755
--- a/spec/unit/application_spec.rb
+++ b/spec/unit/application_spec.rb
@@ -1,620 +1,610 @@
 #!/usr/bin/env ruby
 
 require 'spec_helper'
 
 require 'puppet/application'
 require 'puppet'
 require 'getoptlong'
 
 describe Puppet::Application do
 
   before do
     @app = Class.new(Puppet::Application).new
     @appclass = @app.class
 
     @app.stubs(:name).returns("test_app")
     # avoid actually trying to parse any settings
     Puppet.settings.stubs(:parse)
   end
 
   describe "finding" do
     before do
       @klass = Puppet::Application
       @klass.stubs(:puts)
     end
 
     it "should find classes in the namespace" do
       @klass.find("Agent").should == @klass::Agent
     end
 
     it "should not find classes outside the namespace" do
       lambda { @klass.find("String") }.should raise_error(SystemExit)
     end
 
     it "should exit if it can't find a class" do
       lambda { @klass.find("ThisShallNeverEverEverExistAsdf") }.should raise_error(SystemExit)
     end
   end
 
   describe ".run_mode" do
     it "should default to user" do
       @appclass.run_mode.name.should == :user
     end
 
     it "should set and get a value" do
       @appclass.run_mode :agent
       @appclass.run_mode.name.should == :agent
     end
   end
 
   it "should sadly and frighteningly allow run_mode to change at runtime" do
     class TestApp < Puppet::Application
       run_mode :master
       def run_command
         # This is equivalent to calling these methods externally to the
         # instance, but since this is what "real world" code is likely to do
         # (and we need the class anyway) we may as well test that. --daniel 2011-02-03
         set_run_mode self.class.run_mode "agent"
       end
     end
 
     Puppet[:run_mode].should == "user"
 
     expect {
       app = TestApp.new
 
       Puppet[:run_mode].should == "master"
 
       app.run
 
       app.class.run_mode.name.should == :agent
       $puppet_application_mode.name.should == :agent
     }.should_not raise_error
 
     Puppet[:run_mode].should == "agent"
   end
 
   it "it should not allow run mode to be set multiple times" do
     pending "great floods of tears, you can do this right now" # --daniel 2011-02-03
     app = Puppet::Application.new
     expect {
       app.set_run_mode app.class.run_mode "master"
       $puppet_application_mode.name.should == :master
       app.set_run_mode app.class.run_mode "agent"
       $puppet_application_mode.name.should == :agent
     }.should raise_error
   end
 
   it "should explode when an invalid run mode is set at runtime, for great victory"
   # ...but you can, and while it will explode, that only happens too late for
   # us to easily test. --daniel 2011-02-03
 
   it "should have a run entry-point" do
     @app.should respond_to(:run)
   end
 
   it "should have a read accessor to options" do
     @app.should respond_to(:options)
   end
 
   it "should include a default setup method" do
     @app.should respond_to(:setup)
   end
 
   it "should include a default preinit method" do
     @app.should respond_to(:preinit)
   end
 
   it "should include a default run_command method" do
     @app.should respond_to(:run_command)
   end
 
   it "should invoke main as the default" do
     @app.expects( :main )
     @app.run_command
   end
 
   describe 'when invoking clear!' do
     before :each do
       Puppet::Application.run_status = :stop_requested
       Puppet::Application.clear!
     end
 
     it 'should have nil run_status' do
       Puppet::Application.run_status.should be_nil
     end
 
     it 'should return false for restart_requested?' do
       Puppet::Application.restart_requested?.should be_false
     end
 
     it 'should return false for stop_requested?' do
       Puppet::Application.stop_requested?.should be_false
     end
 
     it 'should return false for interrupted?' do
       Puppet::Application.interrupted?.should be_false
     end
 
     it 'should return true for clear?' do
       Puppet::Application.clear?.should be_true
     end
   end
 
   describe 'after invoking stop!' do
     before :each do
       Puppet::Application.run_status = nil
       Puppet::Application.stop!
     end
 
     after :each do
       Puppet::Application.run_status = nil
     end
 
     it 'should have run_status of :stop_requested' do
       Puppet::Application.run_status.should == :stop_requested
     end
 
     it 'should return true for stop_requested?' do
       Puppet::Application.stop_requested?.should be_true
     end
 
     it 'should return false for restart_requested?' do
       Puppet::Application.restart_requested?.should be_false
     end
 
     it 'should return true for interrupted?' do
       Puppet::Application.interrupted?.should be_true
     end
 
     it 'should return false for clear?' do
       Puppet::Application.clear?.should be_false
     end
   end
 
   describe 'when invoking restart!' do
     before :each do
       Puppet::Application.run_status = nil
       Puppet::Application.restart!
     end
 
     after :each do
       Puppet::Application.run_status = nil
     end
 
     it 'should have run_status of :restart_requested' do
       Puppet::Application.run_status.should == :restart_requested
     end
 
     it 'should return true for restart_requested?' do
       Puppet::Application.restart_requested?.should be_true
     end
 
     it 'should return false for stop_requested?' do
       Puppet::Application.stop_requested?.should be_false
     end
 
     it 'should return true for interrupted?' do
       Puppet::Application.interrupted?.should be_true
     end
 
     it 'should return false for clear?' do
       Puppet::Application.clear?.should be_false
     end
   end
 
   describe 'when performing a controlled_run' do
     it 'should not execute block if not :clear?' do
       Puppet::Application.run_status = :stop_requested
       target = mock 'target'
       target.expects(:some_method).never
       Puppet::Application.controlled_run do
         target.some_method
       end
     end
 
     it 'should execute block if :clear?' do
       Puppet::Application.run_status = nil
       target = mock 'target'
       target.expects(:some_method).once
       Puppet::Application.controlled_run do
         target.some_method
       end
     end
 
     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'
         target.expects(:some_method).once
         old_handler = trap('HUP') { target.some_method }
         begin
           Puppet::Application.controlled_run do
             Puppet::Application.run_status = :restart_requested
           end
         ensure
           trap('HUP', old_handler)
         end
       end
     end
 
     after :each do
       Puppet::Application.run_status = nil
     end
   end
 
   describe "when parsing command-line options" do
 
     before :each do
       @app.command_line.stubs(:args).returns([])
 
       Puppet.settings.stubs(:optparse_addargs).returns([])
     end
 
     it "should pass the banner to the option parser" do
       option_parser = stub "option parser"
       option_parser.stubs(:on)
       option_parser.stubs(:parse!)
       @app.class.instance_eval do
         banner "banner"
       end
 
       OptionParser.expects(:new).with("banner").returns(option_parser)
 
       @app.parse_options
     end
 
     it "should get options from Puppet.settings.optparse_addargs" do
       Puppet.settings.expects(:optparse_addargs).returns([])
 
       @app.parse_options
     end
 
     it "should add Puppet.settings options to OptionParser" do
       Puppet.settings.stubs(:optparse_addargs).returns( [["--option","-o", "Funny Option"]])
       Puppet.settings.expects(:handlearg).with("--option", 'true')
       @app.command_line.stubs(:args).returns(["--option"])
       @app.parse_options
     end
 
     it "should ask OptionParser to parse the command-line argument" do
       @app.command_line.stubs(:args).returns(%w{ fake args })
       OptionParser.any_instance.expects(:parse!).with(%w{ fake args })
 
       @app.parse_options
     end
 
     describe "when using --help" do
 
       it "should call exit" do
         @app.expects(:exit)
         @app.stubs(:puts)
 
         @app.handle_help(nil)
       end
 
     end
 
     describe "when using --version" do
       it "should declare a version option" do
         @app.should respond_to(:handle_version)
       end
 
       it "should exit after printing the version" do
         @app.stubs(:puts)
 
         lambda { @app.handle_version(nil) }.should raise_error(SystemExit)
       end
     end
 
     describe "when dealing with an argument not declared directly by the application" do
       it "should pass it to handle_unknown if this method exists" do
         Puppet.settings.stubs(:optparse_addargs).returns([["--not-handled", :REQUIRED]])
 
         @app.expects(:handle_unknown).with("--not-handled", "value").returns(true)
         @app.command_line.stubs(:args).returns(["--not-handled", "value"])
         @app.parse_options
       end
 
       it "should pass it to Puppet.settings if handle_unknown says so" do
         Puppet.settings.stubs(:optparse_addargs).returns([["--topuppet", :REQUIRED]])
 
         @app.stubs(:handle_unknown).with("--topuppet", "value").returns(false)
 
         Puppet.settings.expects(:handlearg).with("--topuppet", "value")
         @app.command_line.stubs(:args).returns(["--topuppet", "value"])
         @app.parse_options
       end
 
       it "should pass it to Puppet.settings if there is no handle_unknown method" do
         Puppet.settings.stubs(:optparse_addargs).returns([["--topuppet", :REQUIRED]])
 
         @app.stubs(:respond_to?).returns(false)
 
         Puppet.settings.expects(:handlearg).with("--topuppet", "value")
         @app.command_line.stubs(:args).returns(["--topuppet", "value"])
         @app.parse_options
       end
 
       it "should transform boolean false value to string for Puppet.settings" do
         Puppet.settings.expects(:handlearg).with("--option", "false")
         @app.handlearg("--option", false)
       end
 
       it "should transform boolean true value to string for Puppet.settings" do
         Puppet.settings.expects(:handlearg).with("--option", "true")
         @app.handlearg("--option", true)
       end
 
       it "should transform boolean option to normal form for Puppet.settings" do
         Puppet.settings.expects(:handlearg).with("--option", "true")
         @app.handlearg("--[no-]option", true)
       end
 
       it "should transform boolean option to no- form for Puppet.settings" do
         Puppet.settings.expects(:handlearg).with("--no-option", "false")
         @app.handlearg("--[no-]option", false)
       end
 
     end
-
-    it "should exit if OptionParser raises an error" do
-      $stderr.stubs(:puts)
-      OptionParser.any_instance.stubs(:parse!).raises(OptionParser::ParseError.new("blah blah"))
-
-      @app.expects(:exit)
-
-      lambda { @app.parse_options }.should_not raise_error
-    end
-
   end
 
   describe "when calling default setup" do
 
     before :each do
       @app.stubs(:should_parse_config?).returns(false)
       @app.options.stubs(:[])
     end
 
     [ :debug, :verbose ].each do |level|
       it "should honor option #{level}" do
         @app.options.stubs(:[]).with(level).returns(true)
         Puppet::Util::Log.stubs(:newdestination)
 
         Puppet::Util::Log.expects(:level=).with(level == :verbose ? :info : :debug)
 
         @app.setup
       end
     end
 
     it "should honor setdest option" do
       @app.options.stubs(:[]).with(:setdest).returns(false)
 
       Puppet::Util::Log.expects(:newdestination).with(:syslog)
 
       @app.setup
     end
 
   end
 
   describe "when configuring routes" do
     include PuppetSpec::Files
 
     before :each do
       Puppet::Node.indirection.reset_terminus_class
     end
 
     after :each do
       Puppet::Node.indirection.reset_terminus_class
     end
 
     it "should use the routes specified for only the active application" do
       Puppet[:route_file] = tmpfile('routes')
       File.open(Puppet[:route_file], 'w') do |f|
         f.print <<-ROUTES
           test_app:
             node:
               terminus: exec
           other_app:
             node:
               terminus: plain
             catalog:
               terminus: invalid
         ROUTES
       end
 
       @app.configure_indirector_routes
 
       Puppet::Node.indirection.terminus_class.should == 'exec'
     end
 
     it "should not fail if the route file doesn't exist" do
       Puppet[:route_file] = "/dev/null/non-existent"
 
       expect { @app.configure_indirector_routes }.should_not raise_error
     end
 
     it "should raise an error if the routes file is invalid" do
       Puppet[:route_file] = tmpfile('routes')
       File.open(Puppet[:route_file], 'w') do |f|
         f.print <<-ROUTES
          invalid : : yaml
         ROUTES
       end
 
       expect { @app.configure_indirector_routes }.should raise_error
     end
   end
 
   describe "when running" do
 
     before :each do
       @app.stubs(:preinit)
       @app.stubs(:setup)
       @app.stubs(:parse_options)
     end
 
     it "should call preinit" do
       @app.stubs(:run_command)
 
       @app.expects(:preinit)
 
       @app.run
     end
 
     it "should call parse_options" do
       @app.stubs(:run_command)
 
       @app.expects(:parse_options)
 
       @app.run
     end
 
     it "should call run_command" do
 
       @app.expects(:run_command)
 
       @app.run
     end
 
     it "should parse Puppet configuration if should_parse_config is called" do
       @app.stubs(:run_command)
       @app.class.should_parse_config
 
       Puppet.settings.expects(:parse)
 
       @app.run
     end
 
     it "should not parse_option if should_not_parse_config is called" do
       @app.stubs(:run_command)
       @app.class.should_not_parse_config
 
       Puppet.settings.expects(:parse).never
 
       @app.run
     end
 
     it "should parse Puppet configuration if needed" do
       @app.stubs(:run_command)
       @app.stubs(:should_parse_config?).returns(true)
 
       Puppet.settings.expects(:parse)
 
       @app.run
     end
 
     it "should call run_command" do
       @app.expects(:run_command)
 
       @app.run
     end
 
     it "should call main as the default command" do
       @app.expects(:main)
 
       @app.run
     end
 
     it "should warn and exit if no command can be called" do
       $stderr.expects(:puts)
       @app.expects(:exit).with(1)
       @app.run
     end
 
     it "should raise an error if dispatch returns no command" do
       @app.stubs(:get_command).returns(nil)
       $stderr.expects(:puts)
       @app.expects(:exit).with(1)
       @app.run
     end
 
     it "should raise an error if dispatch returns an invalid command" do
       @app.stubs(:get_command).returns(:this_function_doesnt_exist)
       $stderr.expects(:puts)
       @app.expects(:exit).with(1)
       @app.run
     end
   end
 
   describe "when metaprogramming" do
 
     describe "when calling option" do
       it "should create a new method named after the option" do
         @app.class.option("--test1","-t") do
         end
 
         @app.should respond_to(:handle_test1)
       end
 
       it "should transpose in option name any '-' into '_'" do
         @app.class.option("--test-dashes-again","-t") do
         end
 
         @app.should respond_to(:handle_test_dashes_again)
       end
 
       it "should create a new method called handle_test2 with option(\"--[no-]test2\")" do
         @app.class.option("--[no-]test2","-t") do
         end
 
         @app.should respond_to(:handle_test2)
       end
 
       describe "when a block is passed" do
         it "should create a new method with it" do
           @app.class.option("--[no-]test2","-t") do
             raise "I can't believe it, it works!"
           end
 
           lambda { @app.handle_test2 }.should raise_error
         end
 
         it "should declare the option to OptionParser" do
           OptionParser.any_instance.stubs(:on)
           OptionParser.any_instance.expects(:on).with { |*arg| arg[0] == "--[no-]test3" }
 
           @app.class.option("--[no-]test3","-t") do
           end
 
           @app.parse_options
         end
 
         it "should pass a block that calls our defined method" do
           OptionParser.any_instance.stubs(:on)
           OptionParser.any_instance.stubs(:on).with('--test4','-t').yields(nil)
 
           @app.expects(:send).with(:handle_test4, nil)
 
           @app.class.option("--test4","-t") do
           end
 
           @app.parse_options
         end
       end
 
       describe "when no block is given" do
         it "should declare the option to OptionParser" do
           OptionParser.any_instance.stubs(:on)
           OptionParser.any_instance.expects(:on).with("--test4","-t")
 
           @app.class.option("--test4","-t")
 
           @app.parse_options
         end
 
         it "should give to OptionParser a block that adds the the value to the options array" do
           OptionParser.any_instance.stubs(:on)
           OptionParser.any_instance.stubs(:on).with("--test4","-t").yields(nil)
 
           @app.options.expects(:[]=).with(:test4,nil)
 
           @app.class.option("--test4","-t")
 
           @app.parse_options
         end
       end
     end
 
   end
 end
diff --git a/spec/unit/faces/help_spec.rb b/spec/unit/faces/help_spec.rb
new file mode 100644
index 000000000..cd74a5bf1
--- /dev/null
+++ b/spec/unit/faces/help_spec.rb
@@ -0,0 +1,112 @@
+require 'spec_helper'
+require 'puppet/faces/help'
+
+describe Puppet::Faces[:help, '0.0.1'] do
+  it "should have a help action" do
+    subject.should be_action :help
+  end
+
+  it "should have a default action of help" do
+    pending "REVISIT: we don't support default actions yet"
+  end
+
+  it "should accept a call with no arguments" do
+    expect { subject.help() }.should_not raise_error
+  end
+
+  it "should accept a face name" do
+    expect { subject.help(:help) }.should_not raise_error
+  end
+
+  it "should accept a face and action name" do
+    expect { subject.help(:help, :help) }.should_not raise_error
+  end
+
+  it "should fail if more than a face and action are given" do
+    expect { subject.help(:help, :help, :for_the_love_of_god) }.
+      should raise_error ArgumentError
+  end
+
+  it "should treat :current and 'current' identically" do
+    subject.help(:help, :version => :current).should ==
+      subject.help(:help, :version => 'current')
+  end
+
+  it "should complain when the request version of a face is missing" do
+    expect { subject.help(:huzzah, :bar, :version => '17.0.0') }.
+      should raise_error Puppet::Error
+  end
+
+  it "should find a face by version" do
+    face = Puppet::Faces[:huzzah, :current]
+    subject.help(:huzzah, :version => face.version).
+      should == subject.help(:huzzah, :version => :current)
+  end
+
+  context "when listing subcommands" do
+    subject { Puppet::Faces[:help, :current].help }
+
+    # Check a precondition for the next block; if this fails you have
+    # something odd in your set of faces, and we skip testing things that
+    # matter. --daniel 2011-04-10
+    it "should have at least one face with a summary" do
+      Puppet::Faces.faces.should be_any do |name|
+        Puppet::Faces[name, :current].summary
+      end
+    end
+
+    Puppet::Faces.faces.each do |name|
+      face = Puppet::Faces[name, :current]
+      summary = face.summary
+
+      it { should =~ %r{ #{name} } }
+      it { should =~ %r{ #{name} +#{summary}} } if summary
+    end
+
+    Puppet::Faces[:help, :current].legacy_applications.each do |appname|
+      it { should =~ %r{ #{appname} } }
+
+      summary = Puppet::Faces[:help, :current].horribly_extract_summary_from(appname)
+      summary and it { should =~ %r{ #{summary}\b} }
+    end
+  end
+
+  context "#legacy_applications" do
+    subject { Puppet::Faces[:help, :current].legacy_applications }
+
+    # If we don't, these tests are ... less than useful, because they assume
+    # it.  When this breaks you should consider ditching the entire feature
+    # and tests, but if not work out how to fake one. --daniel 2011-04-11
+    it { should have_at_least(1).item }
+
+    # Meh.  This is nasty, but we can't control the other list; the specific
+    # bug that caused these to be listed is annoyingly subtle and has a nasty
+    # fix, so better to have a "fail if you do something daft" trigger in
+    # place here, I think. --daniel 2011-04-11
+    %w{faces_base indirection_base}.each do |name|
+      it { should_not include name }
+    end
+  end
+
+  context "help for legacy applications" do
+    subject { Puppet::Faces[:help, :current] }
+    let :appname do subject.legacy_applications.first end
+
+    # This test is purposely generic, so that as we eliminate legacy commands
+    # we don't get into a loop where we either test a face-based replacement
+    # and fail to notice breakage, or where we have to constantly rewrite this
+    # test and all. --daniel 2011-04-11
+    it "should return the legacy help when given the subcommand" do
+      help = subject.help(appname)
+      help.should =~ /puppet-#{appname}/
+      %w{SYNOPSIS USAGE DESCRIPTION OPTIONS COPYRIGHT}.each do |heading|
+        help.should =~ /^#{heading}$/
+      end
+    end
+
+    it "should fail when asked for an action on a legacy command" do
+      expect { subject.help(appname, :whatever) }.
+        to raise_error ArgumentError, /Legacy subcommands don't take actions/
+    end
+  end
+end
diff --git a/spec/unit/faces_spec.rb b/spec/unit/faces_spec.rb
new file mode 100644
index 000000000..b6c49d917
--- /dev/null
+++ b/spec/unit/faces_spec.rb
@@ -0,0 +1 @@
+# You should look at interface_spec.rb
diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb
index 7d2710942..666575605 100755
--- a/spec/unit/interface/action_builder_spec.rb
+++ b/spec/unit/interface/action_builder_spec.rb
@@ -1,59 +1,70 @@
 #!/usr/bin/env ruby
 
 require 'spec_helper'
 require 'puppet/interface/action_builder'
 
 describe Puppet::Interface::ActionBuilder do
   describe "::build" do
     it "should build an action" do
       action = Puppet::Interface::ActionBuilder.build(nil, :foo) do
       end
       action.should be_a(Puppet::Interface::Action)
       action.name.should == :foo
     end
 
     it "should define a method on the face which invokes the action" do
       face = Puppet::Interface.new(:action_builder_test_interface, '0.0.1')
       action = Puppet::Interface::ActionBuilder.build(face, :foo) do
         when_invoked do
           "invoked the method"
         end
       end
 
       face.foo.should == "invoked the method"
     end
 
     it "should require a block" do
       expect { Puppet::Interface::ActionBuilder.build(nil, :foo) }.
         should raise_error("Action :foo must specify a block")
     end
 
     describe "when handling options" do
       let :face do Puppet::Interface.new(:option_handling, '0.0.1') end
 
       it "should have a #option DSL function" do
         method = nil
         Puppet::Interface::ActionBuilder.build(face, :foo) do
           method = self.method(:option)
         end
         method.should be
       end
 
       it "should define an option without a block" do
         action = Puppet::Interface::ActionBuilder.build(face, :foo) do
           option "--bar"
         end
         action.should be_option :bar
       end
 
       it "should accept an empty block" do
         action = Puppet::Interface::ActionBuilder.build(face, :foo) do
           option "--bar" do
             # This space left deliberately blank.
           end
         end
         action.should be_option :bar
       end
     end
+
+    context "inline documentation" do
+      let :face do Puppet::Interface.new(:inline_action_docs, '0.0.1') end
+
+      it "should set the summary" do
+        action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+          summary "this is some text"
+        end
+        action.summary.should == "this is some text"
+      end
+    end
   end
 end
diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb
index b83bd50d3..752871035 100755
--- a/spec/unit/interface/face_collection_spec.rb
+++ b/spec/unit/interface/face_collection_spec.rb
@@ -1,175 +1,181 @@
 #!/usr/bin/env ruby
 require 'spec_helper'
 
 require 'tmpdir'
 require 'puppet/interface/face_collection'
 
 describe Puppet::Interface::FaceCollection do
   # To avoid cross-pollution we have to save and restore both the hash
   # containing all the interface data, and the array used by require.  Restoring
   # both means that we don't leak side-effects across the code. --daniel 2011-04-06
+  #
+  # Worse luck, we *also* need to flush $" of anything defining a face,
+  # because otherwise we can cross-pollute from other test files and end up
+  # with no faces loaded, but the require value set true. --daniel 2011-04-10
   before :each do
     @original_faces    = subject.instance_variable_get("@faces").dup
     @original_required = $".dup
+    $".delete_if do |path| path =~ %r{/faces/.*\.rb$} end
     subject.instance_variable_get("@faces").clear
   end
 
   after :each do
     subject.instance_variable_set("@faces", @original_faces)
     $".clear ; @original_required.each do |item| $" << item end
   end
 
   describe "::faces" do
     it "REVISIT: should have some tests here, if we describe it"
   end
 
   describe "::validate_version" do
     it 'should permit three number versions' do
       subject.validate_version('10.10.10').should == true
     end
 
     it 'should permit versions with appended descriptions' do
       subject.validate_version('10.10.10beta').should == true
     end
 
     it 'should not permit versions with more than three numbers' do
       subject.validate_version('1.2.3.4').should == false
     end
 
     it 'should not permit versions with only two numbers' do
       subject.validate_version('10.10').should == false
     end
 
     it 'should not permit versions with only one number' do
       subject.validate_version('123').should == false
     end
 
     it 'should not permit versions with text in any position but at the end' do
       subject.validate_version('v1.1.1').should == false
     end
   end
 
   describe "::[]" do
     before :each do
       subject.instance_variable_get("@faces")[:foo]['0.0.1'] = 10
     end
 
     before :each do
       @dir = Dir.mktmpdir
       @lib = FileUtils.mkdir_p(File.join @dir, 'puppet', 'faces')
       $LOAD_PATH.push(@dir)
     end
 
     after :each do
       FileUtils.remove_entry_secure @dir
       $LOAD_PATH.pop
     end
 
     it "should return the faces with the given name" do
       subject["foo", '0.0.1'].should == 10
     end
 
     it "should attempt to load the faces if it isn't found" do
       subject.expects(:require).with('puppet/faces/bar')
       subject["bar", '0.0.1']
     end
 
     it "should attempt to load the default faces for the specified version :current" do
-      subject.expects(:require).never # except...
       subject.expects(:require).with('puppet/faces/fozzie')
       subject['fozzie', :current]
     end
   end
 
   describe "::face?" do
-    before :each do
-      subject.instance_variable_get("@faces")[:foo]['0.0.1'] = 10
-    end
-
     it "should return true if the faces specified is registered" do
+      subject.instance_variable_get("@faces")[:foo]['0.0.1'] = 10
       subject.face?("foo", '0.0.1').should == true
     end
 
     it "should attempt to require the faces if it is not registered" do
       subject.expects(:require).with do |file|
         subject.instance_variable_get("@faces")[:bar]['0.0.1'] = true
         file == 'puppet/faces/bar'
       end
       subject.face?("bar", '0.0.1').should == true
     end
 
     it "should return true if requiring the faces registered it" do
       subject.stubs(:require).with do
         subject.instance_variable_get("@faces")[:bar]['0.0.1'] = 20
       end
     end
 
     it "should return false if the faces is not registered" do
       subject.stubs(:require).returns(true)
       subject.face?("bar", '0.0.1').should be_false
     end
 
     it "should return false if the faces file itself is missing" do
       subject.stubs(:require).
         raises(LoadError, 'no such file to load -- puppet/faces/bar')
       subject.face?("bar", '0.0.1').should be_false
     end
 
     it "should register the version loaded by `:current` as `:current`" do
       subject.expects(:require).with do |file|
         subject.instance_variable_get("@faces")[:huzzah]['2.0.1'] = :huzzah_faces
         file == 'puppet/faces/huzzah'
       end
       subject.face?("huzzah", :current)
       subject.instance_variable_get("@faces")[:huzzah][:current].should == :huzzah_faces
     end
 
     context "with something on disk" do
       it "should register the version loaded from `puppet/faces/{name}` as `:current`" do
         subject.should be_face "huzzah", '2.0.1'
         subject.should be_face "huzzah", :current
         Puppet::Faces[:huzzah, '2.0.1'].should == Puppet::Faces[:huzzah, :current]
       end
 
       it "should index :current when the code was pre-required" do
         subject.instance_variable_get("@faces")[:huzzah].should_not be_key :current
         require 'puppet/faces/huzzah'
         subject.face?(:huzzah, :current).should be_true
       end
     end
+
+    it "should not cause an invalid face to be enumerated later" do
+      subject.face?(:there_is_no_face, :current).should be_false
+      subject.faces.should_not include :there_is_no_face
+    end
   end
 
   describe "::register" do
     it "should store the faces by name" do
       faces = Puppet::Faces.new(:my_faces, '0.0.1')
       subject.register(faces)
       subject.instance_variable_get("@faces").should == {:my_faces => {'0.0.1' => faces}}
     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 face name/
       end
     end
   end
 end
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index ea11b21ba..7e6b7de77 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -1,147 +1,185 @@
+require 'spec_helper'
 require 'puppet/faces'
 require 'puppet/interface'
 
 describe Puppet::Interface do
   subject { Puppet::Interface }
 
   before :all do
     @faces = Puppet::Interface::FaceCollection.instance_variable_get("@faces").dup
   end
 
   before :each do
     Puppet::Interface::FaceCollection.instance_variable_get("@faces").clear
   end
 
   after :all do
     Puppet::Interface::FaceCollection.instance_variable_set("@faces", @faces)
   end
 
+  describe "#[]" do
+    it "should fail when no version is requested" do
+      expect { subject[:huzzah] }.should raise_error ArgumentError
+    end
+
+    it "should raise an exception when the requested version is unavailable" do
+      expect { subject[:huzzah, '17.0.0'] }.should raise_error, Puppet::Error
+    end
+
+    it "should raise an exception when the requested face doesn't exist" do
+      expect { subject[:burrble_toot, :current] }.should raise_error, Puppet::Error
+    end
+  end
+
   describe "#define" do
     it "should register the face" do
       face = subject.define(:face_test_register, '0.0.1')
       face.should == subject[:face_test_register, '0.0.1']
     end
 
     it "should load actions" do
       subject.any_instance.expects(:load_actions)
       subject.define(:face_test_load_actions, '0.0.1')
     end
 
     it "should require a version number" do
-      expect { subject.define(:no_version) }.should raise_error ArgumentError
+      expect { subject.define(:no_version) }.to raise_error ArgumentError
+    end
+
+    it "should support summary builder and accessor methods" do
+      subject.new(:foo, '1.0.0').should respond_to(:summary).with(0).arguments
+      subject.new(:foo, '1.0.0').should respond_to(:summary=).with(1).arguments
+    end
+
+    it "should set the summary text" do
+      text = "hello, freddy, my little pal"
+      subject.define(:face_test_summary, '1.0.0') do
+        summary text
+      end
+      subject[:face_test_summary, '1.0.0'].summary.should == text
+    end
+
+    it "should support mutating the summary" do
+      text = "hello, freddy, my little pal"
+      subject.define(:face_test_summary, '1.0.0') do
+        summary text
+      end
+      subject[:face_test_summary, '1.0.0'].summary.should == text
+      subject[:face_test_summary, '1.0.0'].summary = text + text
+      subject[:face_test_summary, '1.0.0'].summary.should == text + text
     end
   end
 
   describe "#initialize" do
     it "should require a version number" do
-      expect { subject.new(:no_version) }.should raise_error ArgumentError
+      expect { subject.new(:no_version) }.to raise_error ArgumentError
     end
 
     it "should require a valid version number" do
       expect { subject.new(:bad_version, 'Rasins') }.
         should raise_error ArgumentError
     end
 
     it "should instance-eval any provided block" do
       face = subject.new(:face_test_block, '0.0.1') do
         action(:something) do
           when_invoked { "foo" }
         end
       end
 
       face.something.should == "foo"
     end
   end
 
   it "should have a name" do
     subject.new(:me, '0.0.1').name.should == :me
   end
 
   it "should stringify with its own name" do
     subject.new(:me, '0.0.1').to_s.should =~ /\bme\b/
   end
 
   it "should allow overriding of the default format" do
     face = subject.new(:me, '0.0.1')
     face.set_default_format :foo
     face.default_format.should == :foo
   end
 
   it "should default to :pson for its format" do
     subject.new(:me, '0.0.1').default_format.should == :pson
   end
 
   # Why?
   it "should create a class-level autoloader" do
     subject.autoloader.should be_instance_of(Puppet::Util::Autoload)
   end
 
   it "should try to require faces that are not known" do
     pending "mocking require causes random stack overflow"
     subject::FaceCollection.expects(:require).with "puppet/faces/foo"
     subject[:foo, '0.0.1']
   end
 
   it "should be able to load all actions in all search paths"
 
 
   it_should_behave_like "things that declare options" do
     def add_options_to(&block)
       subject.new(:with_options, '0.0.1', &block)
     end
   end
 
   describe "with face-level options" do
     it "should not return any action-level options" do
       face = subject.new(:with_options, '0.0.1') do
         option "--foo"
         option "--bar"
         action :baz do
           option "--quux"
         end
       end
       face.options.should =~ [:foo, :bar]
     end
 
     it "should fail when a face option duplicates an action option" do
       expect {
         subject.new(:action_level_options, '0.0.1') do
           action :bar do option "--foo" end
           option "--foo"
         end
       }.should raise_error ArgumentError, /Option foo conflicts with existing option foo on/i
     end
 
     it "should work when two actions have the same option" do
       face = subject.new(:with_options, '0.0.1') do
         action :foo do option "--quux" end
         action :bar do option "--quux" end
       end
 
       face.get_action(:foo).options.should =~ [:quux]
       face.get_action(:bar).options.should =~ [:quux]
     end
   end
 
   describe "with inherited options" do
     let :face do
       parent = Class.new(subject)
       parent.option("--inherited")
       face = parent.new(:example, '0.2.1')
       face.option("--local")
       face
     end
 
     describe "#options" do
       it "should list inherited options" do
         face.options.should =~ [:inherited, :local]
       end
     end
 
     describe "#get_option" do
       it "should return an inherited option object" do
         face.get_option(:inherited).should be_an_instance_of subject::Option
       end
     end
   end
 end
diff --git a/spec/unit/util/command_line_spec.rb b/spec/unit/util/command_line_spec.rb
index ca5f0540f..6cf90475b 100755
--- a/spec/unit/util/command_line_spec.rb
+++ b/spec/unit/util/command_line_spec.rb
@@ -1,137 +1,143 @@
 #!/usr/bin/env ruby
 
 require 'spec_helper'
 
 
 require 'puppet/util/command_line'
 
 describe Puppet::Util::CommandLine do
   include PuppetSpec::Files
   before do
     @tty  = stub("tty",  :tty? => true )
     @pipe = stub("pipe", :tty? => false)
   end
 
   it "should pull off the first argument if it looks like a subcommand" do
     command_line = Puppet::Util::CommandLine.new("puppet", %w{ client --help whatever.pp }, @tty )
 
     command_line.subcommand_name.should == "client"
     command_line.args.should            == %w{ --help whatever.pp }
   end
 
   it "should use 'apply' if the first argument looks like a .pp file" do
     command_line = Puppet::Util::CommandLine.new("puppet", %w{ whatever.pp }, @tty )
 
     command_line.subcommand_name.should == "apply"
     command_line.args.should            == %w{ whatever.pp }
   end
 
   it "should use 'apply' if the first argument looks like a .rb file" do
     command_line = Puppet::Util::CommandLine.new("puppet", %w{ whatever.rb }, @tty )
 
     command_line.subcommand_name.should == "apply"
     command_line.args.should            == %w{ whatever.rb }
   end
 
   it "should use 'apply' if the first argument looks like a flag" do
     command_line = Puppet::Util::CommandLine.new("puppet", %w{ --debug }, @tty )
 
     command_line.subcommand_name.should == "apply"
     command_line.args.should            == %w{ --debug }
   end
 
   it "should use 'apply' if the first argument is -" do
     command_line = Puppet::Util::CommandLine.new("puppet", %w{ - }, @tty )
 
     command_line.subcommand_name.should == "apply"
     command_line.args.should            == %w{ - }
   end
 
   it "should return nil if the first argument is --help" do
     command_line = Puppet::Util::CommandLine.new("puppet", %w{ --help }, @tty )
 
     command_line.subcommand_name.should == nil
   end
 
 
   it "should return nil if there are no arguments on a tty" do
     command_line = Puppet::Util::CommandLine.new("puppet", [], @tty )
 
     command_line.subcommand_name.should == nil
     command_line.args.should            == []
   end
 
   it "should use 'apply' if there are no arguments on a pipe" do
     command_line = Puppet::Util::CommandLine.new("puppet", [], @pipe )
 
     command_line.subcommand_name.should == "apply"
     command_line.args.should            == []
   end
 
   it "should return the executable name if it is not puppet" do
     command_line = Puppet::Util::CommandLine.new("puppetmasterd", [], @tty )
 
     command_line.subcommand_name.should == "puppetmasterd"
   end
 
   it "should translate subcommand names into their legacy equivalent" do
     command_line = Puppet::Util::CommandLine.new("puppet", ["master"], @tty)
     command_line.legacy_executable_name.should == "puppetmasterd"
   end
 
   it "should leave legacy command names alone" do
     command_line = Puppet::Util::CommandLine.new("puppetmasterd", [], @tty)
     command_line.legacy_executable_name.should == "puppetmasterd"
   end
 
   describe "when the subcommand is not implemented" do
     it "should find and invoke an executable with a hyphenated name" do
       commandline = Puppet::Util::CommandLine.new("puppet", ['whatever', 'argument'], @tty)
       Puppet::Util.expects(:which).with('puppet-whatever').returns('/dev/null/puppet-whatever')
       commandline.expects(:system).with('/dev/null/puppet-whatever', 'argument')
 
       commandline.execute
     end
 
     describe "and an external implementation cannot be found" do
       it "should abort and show the usage message" do
         commandline = Puppet::Util::CommandLine.new("puppet", ['whatever', 'argument'], @tty)
         Puppet::Util.expects(:which).with('puppet-whatever').returns(nil)
         commandline.expects(:system).never
 
-        commandline.expects(:usage_message).returns("the usage message")
-        commandline.expects(:abort).with{|x| x =~ /the usage message/}.raises("stubbed abort")
+        text = Puppet::Faces[:help, :current].help
+        commandline.expects(:puts).with { |x| x =~ /Unknown Puppet subcommand/ }
+        commandline.expects(:puts).with text
 
-        lambda{ commandline.execute }.should raise_error('stubbed abort')
+        commandline.execute
       end
     end
   end
   describe 'when loading commands' do
     before do
       @core_apps = %w{describe filebucket kick queue resource agent cert apply doc master}
       @command_line = Puppet::Util::CommandLine.new("foo", %w{ client --help whatever.pp }, @tty )
     end
+    it "should expose available_subcommands as a class method" do
+      @core_apps.each do |command|
+        @command_line.available_subcommands.should include command
+      end
+    end
     it 'should be able to find all existing commands' do
       @core_apps.each do |command|
         @command_line.available_subcommands.should include command
       end
     end
     describe 'when multiple paths have applications' do
       before do
         @dir=tmpdir('command_line_plugin_test')
         @appdir="#{@dir}/puppet/application"
         FileUtils.mkdir_p(@appdir)
         FileUtils.touch("#{@appdir}/foo.rb")
         $LOAD_PATH.unshift(@dir) # WARNING: MUST MATCH THE AFTER ACTIONS!
       end
       it 'should be able to find commands from both paths' do
         found = @command_line.available_subcommands
         found.should include 'foo'
         @core_apps.each { |cmd| found.should include cmd }
       end
       after do
         $LOAD_PATH.shift        # WARNING: MUST MATCH THE BEFORE ACTIONS!
       end
     end
   end
 end
diff --git a/spec/watchr.rb b/spec/watchr.rb
index bad89b088..26919d1a1 100644
--- a/spec/watchr.rb
+++ b/spec/watchr.rb
@@ -1,138 +1,142 @@
 ENV["WATCHR"] = "1"
 ENV['AUTOTEST'] = 'true'
 
 def run_comp(cmd)
   puts cmd
   results = []
   old_sync = $stdout.sync
   $stdout.sync = true
   line = []
   begin
     open("| #{cmd}", "r") do |f|
       until f.eof? do
         c = f.getc
         putc c
         line << c
         if c == ?\n
           results << if RUBY_VERSION >= "1.9" then
-            line.join
-              else
-                line.pack "c*"
-                  end
+                       line.join
+                     else
+                       line.pack "c*"
+                     end
           line.clear
         end
       end
     end
   ensure
     $stdout.sync = old_sync
   end
   results.join
 end
 
 def clear
   #system("clear")
 end
 
 def growl(message, status)
   # Strip the color codes
   message.gsub!(/\[\d+m/, '')
 
   growlnotify = `which growlnotify`.chomp
   return if growlnotify.empty?
   title = "Watchr Test Results"
   image = status == :pass ? "autotest/images/pass.png" : "autotest/images/fail.png"
   options = "-w -n Watchr --image '#{File.expand_path(image)}' -m '#{message}' '#{title}'"
   system %(#{growlnotify} #{options} &)
 end
 
 def file2specs(file)
   %w{spec/unit spec/integration}.collect { |d|
     file.sub('lib/puppet', d).sub(".rb", "_spec.rb")
   }.find_all { |f|
     File.exist?(f)
   }
 end
 
 def file2test(file)
   result = file.sub('lib/puppet', 'test')
   return nil unless File.exist?(result)
   result
 end
 
 def run_spec(command)
   clear
   result = run_comp(command).split("\n").last
   status = result.include?('0 failures') ? :pass : :fail
   growl result, status
 end
 
 def run_test(command)
   clear
   result = run_comp(command).split("\n").last
   status = result.include?('0 failures, 0 errors') ? :pass : :fail
   growl result.split("\n").last rescue nil
 end
 
 def run_test_file(file)
   run_test(%Q(#{file}))
 end
 
 def run_spec_files(files)
   files = Array(files)
   return if files.empty?
-  opts = File.readlines('spec/spec.opts').collect { |l| l.chomp }.join(" ")
+  if File.exist?(File.expand_path("~/.rspec")) then
+    opts = ''                   # use the user defaults
+  else
+    opts = File.readlines('spec/spec.opts').collect { |l| l.chomp }.join(" ")
+  end
   run_spec("rspec #{opts} --tty #{files.join(' ')}")
 end
 
 def run_all_tests
   run_test("rake unit")
 end
 
 def run_all_specs
-  run_test("rake spec")
+  run_spec_files "spec"
 end
 
 def run_suite
-  run_all_tests
   run_all_specs
+  run_all_tests
 end
 
 watch('spec/spec_helper.rb') { run_all_specs }
 watch(%r{^spec/(unit|integration)/.*\.rb$}) { |md| run_spec_files(md[0]) }
 watch(%r{^lib/puppet/(.*)\.rb$}) { |md|
   run_spec_files(file2specs(md[0]))
   if t = file2test(md[0])
     run_test_file(t)
   end
 }
 watch(%r{^spec/lib/spec.*}) { |md| run_all_specs }
 watch(%r{^spec/lib/monkey_patches/.*}) { |md| run_all_specs }
 watch(%r{test/.+\.rb}) { |md|
   if md[0] =~ /\/lib\//
     run_all_tests
   else
     run_test_file(md[0])
   end
 }
 
 # Ctrl-\
 Signal.trap 'QUIT' do
   puts " --- Running all tests ---\n\n"
   run_suite
 end
 
 @interrupted = false
 
 # Ctrl-C
 Signal.trap 'INT' do
   if @interrupted
     @wants_to_quit = true
     abort("\n")
   else
     puts "Interrupt a second time to quit; wait for rerun of tests"
     @interrupted = true
     Kernel.sleep 1.5
     # raise Interrupt, nil # let the run loop catch it
     run_suite
   end
 end