diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb index c284433d1..2eb2869a4 100644 --- a/lib/puppet/faces/help.rb +++ b/lib/puppet/faces/help.rb @@ -1,66 +1,93 @@ require 'puppet/faces' require 'puppet/util/command_line' Puppet::Faces.define(:help, '0.0.1') do HelpSummaryFormat = ' %-18s %s' summary "Displays help about puppet subcommands" action(:help) do 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 if options[:version] and options[:version] !~ /^current$/i then version = options[:version] else version = :current end message = [] if args.length == 0 then message << "Use: puppet [options] " message << "" message << "Available subcommands, from Puppet Faces:" Puppet::Faces.faces.sort.each do |name| face = Puppet::Faces[name, :current] message << format(HelpSummaryFormat, face.name, face.summary) end - legacy = Puppet::Util::CommandLine.available_subcommands.reject do |appname| - Puppet::Faces.face? appname.to_sym, :current - end - unless legacy.empty? then # great victory when this is true! + unless legacy_applications.empty? then # great victory when this is true! message << "" message << "Available applications, soon to be ported to Faces:" - legacy.sort.each do |appname| - message << format(HelpSummaryFormat, appname, 'REVISIT: how to summarize these?') + legacy_applications.each do |appname| + summary = horribly_extract_summary_from appname + message << format(HelpSummaryFormat, appname, summary) end end else face = Puppet::Faces[args[0].to_sym, version] if args[1] then action = face.get_action args[1].to_sym else action = nil end help = [] face.actions.each do |action| help << "Action: #{action}" end end message end 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 87ff67948..1399abfef 100644 --- a/spec/unit/faces/help_spec.rb +++ b/spec/unit/faces/help_spec.rb @@ -1,69 +1,103 @@ 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, :current).should == subject.help(:help, '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, face.version).should == subject.help(:huzzah, :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 have_matching_element %r{ #{name} } } it { should have_matching_element %r{ #{name} +#{summary}} } if summary end Puppet::Util::CommandLine.available_subcommands do |name| it { should have_matching_element %r{ #{name} } } end end + + context "when listing legacy applications" do + let :help do Puppet::Faces[:help, :current] end + + # 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 one legacy application" do + help.legacy_applications.should have_at_least(1).item + end + + # 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 list the #{name} application" do + help.legacy_applications.should_not include name + end + end + + Puppet::Faces[:help, :current].legacy_applications.each do |appname| + it "should list #{appname} in the help output" do + help.help.should have_matching_element %r{ #{appname} } + end + + summary = Puppet::Faces[:help, :current].horribly_extract_summary_from(appname) + if summary then + it "should display the summary of #{appname}" do + help.help.should have_matching_element %r{ #{summary}\b} + end + end + end + end end