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 <[email protected]>
---
lib/puppet/application/face_base.rb | 20 ++++++++++----------
spec/unit/application/face_base_spec.rb | 13 +++++++++++++
2 files changed, 23 insertions(+), 10 deletions(-)
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
--
1.7.5
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.