(#6962) Better argument checking for help.

We used to blink and miss the fact that we failed to load an action or face in
the past; now we test for that, and fail with a clear error message when the
user asks for something we can't deliver.

Additionally, fix a couple of tests that were silently broken because they
passed the wrong arguments, but still got some output.

Paired-With: Matt Robinson <matt@puppetlabs.com>

2 files changed, 17 insertions, 10 deletions

diff --git a/lib/puppet/faces/help.rb b/lib/puppet/faces/help.rb
index 26f839735..29a4a62f7 100644
--- a/lib/puppet/faces/help.rb
+++ b/lib/puppet/faces/help.rb
@@ -38,13 +38,19 @@ Puppet::Faces.define(:help, '0.0.1') do
       face   = facename ? Puppet::Faces[facename.to_sym, version] : nil
       action = (face and actionname) ? face.get_action(actionname.to_sym) : nil
 
-      template = case args.length
-                 when 0 then erb 'global.erb'
-                 when 1 then erb 'face.erb'
-                 when 2 then erb 'action.erb'
-                 else
-                   fail ArgumentError, "Too many arguments to help action"
-                 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
diff --git a/spec/unit/faces/help_spec.rb b/spec/unit/faces/help_spec.rb
index 1399abfef..aa811c4b3 100644
--- a/spec/unit/faces/help_spec.rb
+++ b/spec/unit/faces/help_spec.rb
@@ -28,8 +28,8 @@ describe Puppet::Faces[:help, '0.0.1'] do
   end
 
   it "should treat :current and 'current' identically" do
-    subject.help(:help, :current).should ==
-      subject.help(:help, 'current')
+    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
@@ -39,7 +39,8 @@ describe Puppet::Faces[:help, '0.0.1'] do
 
   it "should find a face by version" do
     face = Puppet::Faces[:huzzah, :current]
-    subject.help(:huzzah, face.version).should == subject.help(:huzzah, :current)
+    subject.help(:huzzah, :version => face.version).
+      should == subject.help(:huzzah, :version => :current)
   end
 
   context "when listing subcommands" do