[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5
Daniel Pittman
daniel at puppetlabs.com
Tue May 10 08:18:38 UTC 2011
The following commit has been merged in the experimental branch:
commit be4d7e6eb45911793bf2370ff8a0d8d160446f54
Author: Daniel Pittman <daniel at puppetlabs.com>
Date: Thu Apr 28 14:24:46 2011 -0700
(#7269) Fix error reporting for bad render formats...
The previous change was incomplete, and could result in internal errors with
the wrong combination of inputs. Now we have more testing, and a logically
consistent model, so all works.
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 d28a8af..d027694 100644
--- a/lib/puppet/application/face_base.rb
+++ b/lib/puppet/application/face_base.rb
@@ -15,14 +15,8 @@ class Puppet::Application::FaceBase < Puppet::Application
Puppet::Util::Log.level = :info
end
- 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
+ option("--render-as FORMAT") do |format|
+ self.render_as = format.to_sym
end
option("--mode RUNMODE", "-r") do |arg|
@@ -34,20 +28,33 @@ class Puppet::Application::FaceBase < Puppet::Application
attr_accessor :face, :action, :type, :arguments, :render_as
- def render(result)
- format = render_as || action.render_as || :for_humans
+ def render_as=(format)
+ if format == :for_humans or format == :json
+ @render_as = format
+ elsif network_format = Puppet::Network::FormatHandler.format(format)
+ method = network_format.render_method
+ if method == "to_pson" then
+ @render_as = :json
+ else
+ @render_as = method.to_sym
+ end
+ else
+ raise ArgumentError, "I don't know how to render '#{format}'"
+ end
+ end
+ def render(result)
# Invoke the rendering hook supplied by the user, if appropriate.
- if hook = action.when_rendering(format) then
+ if hook = action.when_rendering(render_as) then
result = hook.call(result)
end
- if format == :for_humans then
+ if render_as == :for_humans then
render_for_humans(result)
- elsif format == :json or @render_method == "to_pson"
+ elsif render_as == :json
PSON::pretty_generate(result, :allow_nan => true, :max_nesting => false)
else
- result.send(@render_method)
+ result.send(render_as)
end
end
@@ -129,8 +136,13 @@ class Puppet::Application::FaceBase < Puppet::Application
end
if @action.nil?
- @action = @face.get_default_action()
- @is_default_action = true
+ if @action = @face.get_default_action() then
+ @is_default_action = true
+ else
+ Puppet.err "#{face.name} does not have a default action, and no action was given"
+ Puppet.err Puppet::Face[:help, :current].help(@face.name)
+ exit false
+ end
end
# Now we can interact with the default option code to build behaviour
@@ -138,7 +150,7 @@ class Puppet::Application::FaceBase < Puppet::Application
@action.options.each do |option|
option = @action.get_option(option) # make it the object.
self.class.option(*option.optparse) # ...and make the CLI parse it.
- end if @action
+ end
# ...and invoke our parent to parse all the command line options.
super
@@ -190,6 +202,9 @@ class Puppet::Application::FaceBase < Puppet::Application
# would invoke the action with options set as global state in the
# interface object. --daniel 2011-03-28
@arguments << options
+
+ # If we don't have a rendering format, set one early.
+ self.render_as ||= (@action.render_as || :for_humans)
end
@@ -207,13 +222,8 @@ class Puppet::Application::FaceBase < Puppet::Application
Puppet.err detail.to_s
end
else
- if arguments.first.is_a? Hash
- puts "#{face} does not have a default action"
- else
- puts "#{face} does not respond to action #{arguments.first}"
- end
-
- puts Puppet::Face[:help, :current].help(@face.name, *arguments)
+ puts "#{face} does not respond to action #{arguments.first}"
+ puts Puppet::Face[:help, :current].help(@face.name)
end
exit status
diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb
index 0c75236..2757020 100755
--- a/spec/unit/application/face_base_spec.rb
+++ b/spec/unit/application/face_base_spec.rb
@@ -54,7 +54,7 @@ describe Puppet::Application::FaceBase do
it "should use the default action if not given any arguments" do
app.command_line.stubs(:args).returns []
- action = stub(:options => [])
+ action = stub(:options => [], :render_as => nil)
Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action)
app.stubs(:main)
app.run
@@ -64,7 +64,7 @@ describe Puppet::Application::FaceBase do
it "should use the default action if not given a valid one" do
app.command_line.stubs(:args).returns %w{bar}
- action = stub(:options => [])
+ action = stub(:options => [], :render_as => nil)
Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action)
app.stubs(:main)
app.run
@@ -76,9 +76,8 @@ describe Puppet::Application::FaceBase do
app.command_line.stubs(:args).returns %w{bar}
Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil)
app.stubs(:main)
- app.run
- app.action.should be_nil
- app.arguments.should == [ 'bar', { } ]
+ expect { app.run }.to exit_with 1
+ @logs.first.message.should =~ /does not have a default action/
end
it "should report a sensible error when options with = fail" do
@@ -150,7 +149,7 @@ describe Puppet::Application::FaceBase do
end
it "should handle application-level options" do
- app.command_line.stubs(:args).returns %w{help --verbose help}
+ app.command_line.stubs(:args).returns %w{basetest --verbose return_true}
app.preinit
app.parse_options
app.face.name.should == :basetest
@@ -191,7 +190,7 @@ describe Puppet::Application::FaceBase do
it "should lookup help when it cannot do anything else" do
app.action = nil
- Puppet::Face[:help, :current].expects(:help).with(:basetest, *app.arguments)
+ Puppet::Face[:help, :current].expects(:help).with(:basetest)
expect { app.main }.to exit_with 1
end
@@ -205,6 +204,7 @@ describe Puppet::Application::FaceBase do
before :each do
app.stubs(:puts) # don't dump text to screen.
+ app.render_as = :json
app.face = Puppet::Face[:basetest, '0.0.1']
app.arguments = []
end
@@ -228,45 +228,42 @@ describe Puppet::Application::FaceBase do
app.action = app.face.get_action :return_raise
expect { app.main }.not_to exit_with 0
end
-
- it "should exit non-0 when the action does not exist" do
- app.action = nil
- app.arguments = ["foo"]
- expect { app.main }.to exit_with 1
- end
end
describe "#render" do
before :each do
- app.face = Puppet::Face[:basetest, '0.0.1']
- app.action = app.face.get_action(:foo)
+ app.face = Puppet::Face[:basetest, '0.0.1']
+ app.action = app.face.get_action(:foo)
end
- ["hello", 1, 1.0].each do |input|
- it "should just return a #{input.class.name}" do
- app.render(input).should == input
+ context "default rendering" do
+ before :each do app.setup end
+
+ ["hello", 1, 1.0].each do |input|
+ it "should just return a #{input.class.name}" do
+ app.render(input).should == input
+ end
end
- end
- [[1, 2], ["one"], [{ 1 => 1 }]].each do |input|
- it "should render #{input.class} using the 'pp' library" do
- app.render(input).should == input.pretty_inspect
+ [[1, 2], ["one"], [{ 1 => 1 }]].each do |input|
+ it "should render #{input.class} using the 'pp' library" do
+ app.render(input).should == input.pretty_inspect
+ end
end
- end
- it "should render a non-trivially-keyed Hash with the 'pp' library" do
- hash = { [1,2] => 3, [2,3] => 5, [3,4] => 7 }
- app.render(hash).should == hash.pretty_inspect
- end
+ it "should render a non-trivially-keyed Hash with the 'pp' library" do
+ hash = { [1,2] => 3, [2,3] => 5, [3,4] => 7 }
+ app.render(hash).should == hash.pretty_inspect
+ end
- it "should render a {String,Numeric}-keyed Hash into a table" do
- object = Object.new
- hash = { "one" => 1, "two" => [], "three" => {}, "four" => object,
- 5 => 5, 6.0 => 6 }
+ it "should render a {String,Numeric}-keyed Hash into a table" do
+ object = Object.new
+ hash = { "one" => 1, "two" => [], "three" => {}, "four" => object,
+ 5 => 5, 6.0 => 6 }
- # Gotta love ASCII-betical sort order. Hope your objects are better
- # structured for display than my test one is. --daniel 2011-04-18
- app.render(hash).should == <<EOT
+ # Gotta love ASCII-betical sort order. Hope your objects are better
+ # structured for display than my test one is. --daniel 2011-04-18
+ app.render(hash).should == <<EOT
5 5
6.0 6
four #{object.pretty_inspect.chomp}
@@ -274,14 +271,14 @@ one 1
three {}
two []
EOT
- end
+ end
- it "should render a hash nicely with a multi-line value" do
- hash = {
- "number" => { "1" => '1' * 40, "2" => '2' * 40, '3' => '3' * 40 },
- "text" => { "a" => 'a' * 40, 'b' => 'b' * 40, 'c' => 'c' * 40 }
- }
- app.render(hash).should == <<EOT
+ it "should render a hash nicely with a multi-line value" do
+ hash = {
+ "number" => { "1" => '1' * 40, "2" => '2' * 40, '3' => '3' * 40 },
+ "text" => { "a" => 'a' * 40, 'b' => 'b' * 40, 'c' => 'c' * 40 }
+ }
+ app.render(hash).should == <<EOT
number {"1"=>"1111111111111111111111111111111111111111",
"2"=>"2222222222222222222222222222222222222222",
"3"=>"3333333333333333333333333333333333333333"}
@@ -289,20 +286,20 @@ text {"a"=>"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"b"=>"bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
"c"=>"cccccccccccccccccccccccccccccccccccccccc"}
EOT
- end
+ end
- it "should invoke the action rendering hook while rendering" do
- app.action.set_rendering_method_for(:for_humans, proc { |value| "bi-winning!" })
- app.action.render_as = :for_humans
- app.render("bi-polar?").should == "bi-winning!"
- end
+ it "should invoke the action rendering hook while rendering" do
+ app.action.set_rendering_method_for(:for_humans, proc { |value| "bi-winning!" })
+ app.render("bi-polar?").should == "bi-winning!"
+ end
- it "should render JSON when asked for json" do
- app.action.render_as = :json
- json = app.render({ :one => 1, :two => 2 })
- json.should =~ /"one":\s*1\b/
- json.should =~ /"two":\s*2\b/
- PSON.parse(json).should == { "one" => 1, "two" => 2 }
+ it "should render JSON when asked for json" do
+ app.render_as = :json
+ json = app.render({ :one => 1, :two => 2 })
+ json.should =~ /"one":\s*1\b/
+ json.should =~ /"two":\s*2\b/
+ PSON.parse(json).should == { "one" => 1, "two" => 2 }
+ end
end
it "should fail early if asked to render an invalid format" do
@@ -311,11 +308,14 @@ EOT
# 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 {
+ expect { app.setup; app.run }.to exit_with 1
+ }.to print(/I don't know how to render 'interpretive-dance'/)
+ end
- expect { app.run }.to exit_with 1
+ it "should work if asked to render a NetworkHandler format" do
+ app.command_line.stubs(:args).returns %w{facts find dummy --render-as yaml}
+ expect { app.parse_options; app.setup; app.run }.to exit_with 0
end
end
end
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list