[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5

[Daniel Pittman]

The following commit has been merged in the experimental branch:
commit 632a0a0042ebf2e7ef209ce30005833ccee6e77b
Author: Daniel Pittman <daniel at puppetlabs.com>
Date:   Thu Apr 28 00:04:00 2011 -0700

    (#7269) Better error reporting for bad render formats.
    
    Previously we would try and send `nil` to a class to render an unsupported
    format, which was bad.  Worse, we would only discover this *after* the fact,
    when we tried to render, so the entire action had run and the result was lost
    to the world.
    
    Instead, validate the parameter early and fail during option parsing.  This
    has less nice error reporting than we can get handling it later[1], but it
    gets us a much better overall set of behaviour.
    
    [1] puppet/application.rb will print and exit, rather than raising, when the
        option handler fails; this will improve when we unify face and application
        options properly.
    
    Reviewed-By: Max Martin <max at puppetlabs.com>

diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb
index cc62257..d28a8af 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -15,8 +15,14 @@ class Puppet::Application::FaceBase < Puppet::Application
     Puppet::Util::Log.level = :info
   end
 
-  option("--render-as FORMAT") do |arg|
-    @render_as = arg.to_sym
+  option("--render-as FORMAT") do |_arg|
+    format = _arg.to_sym
+    unless @render_method = Puppet::Network::FormatHandler.format(format)
+      unless format == :for_humans or format == :json
+        raise ArgumentError, "I don't know how to render '#{format}'"
+      end
+    end
+    @render_as = format
   end
 
   option("--mode RUNMODE", "-r") do |arg|
@@ -36,18 +42,12 @@ class Puppet::Application::FaceBase < Puppet::Application
       result = hook.call(result)
     end
 
-    begin
-      render_method = Puppet::Network::FormatHandler.format(format).render_method
-    rescue
-      render_method = nil
-    end
-
     if format == :for_humans then
       render_for_humans(result)
-    elsif format == :json or render_method == "to_pson"
+    elsif format == :json or @render_method == "to_pson"
       PSON::pretty_generate(result, :allow_nan => true, :max_nesting => false)
     else
-      result.send(render_method)
+      result.send(@render_method)
     end
   end
 
diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index e84e676..0c75236 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -304,5 +304,18 @@ EOT
       json.should =~ /"two":\s*2\b/
       PSON.parse(json).should == { "one" => 1, "two" => 2 }
     end
+
+    it "should fail early if asked to render an invalid format" do
+      app.command_line.stubs(:args).returns %w{--render-as interpretive-dance help help}
+      # We shouldn't get here, thanks to the exception, and our expectation on
+      # it, but this helps us fail if that slips up and all. --daniel 2011-04-27
+      Puppet::Face[:help, :current].expects(:help).never
+
+      # ...and this is just annoying.  Thanks, puppet/application.rb.
+      $stderr.expects(:puts).
+        with "Could not parse options: I don't know how to render 'interpretive-dance'"
+
+      expect { app.run }.to exit_with 1
+    end
   end
 end

-- 
Puppet packaging for Debian