diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index 29a4a62f7..1d8abe20e 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -1,96 +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 - face = facename ? Puppet::Faces[facename.to_sym, version] : nil - action = (face and actionname) ? face.get_action(actionname.to_sym) : nil + + 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/spec/unit/faces/help_spec.rb b/spec/unit/faces/help_spec.rb index f2aeb44e9..cd74a5bf1 100644 --- a/spec/unit/faces/help_spec.rb +++ b/spec/unit/faces/help_spec.rb @@ -1,90 +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