Ruby 1.9 is strict about argument arity for methods that are metaprogrammatically defined. A ton of specs that were setting up when_invoked didn't pass options even though they should have been.
Reviewed-by: Daniel Pittman <[email protected]> Signed-off-by: Matt Robinson <[email protected]> --- Local-branch: ticket/2.7.x/7507-filter_19_failures lib/puppet/interface/action.rb | 2 +- spec/unit/interface/action_builder_spec.rb | 42 ++++++++-------- spec/unit/interface/action_manager_spec.rb | 70 ++++++++++++++-------------- spec/unit/interface/action_spec.rb | 59 +++++++++++------------ 4 files changed, 86 insertions(+), 87 deletions(-) diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index 622371a..3c377a4 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -192,7 +192,7 @@ class Puppet::Interface::Action # but will on 1.9.2, which treats it as "no arguments". Which bites, # because this just begs for us to wind up in the horrible situation # where a 1.8 vs 1.9 error bites our end users. --daniel 2011-04-19 - raise ArgumentError, "action when_invoked requires at least one argument (options)" + raise ArgumentError, "when_invoked requires at least one argument (options) for action #{@name}" elsif arity > 0 then range = Range.new(1, arity - 1) decl = range.map { |x| "arg#{x}" } << "options = {}" diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb index c398605..602daaa 100755 --- a/spec/unit/interface/action_builder_spec.rb +++ b/spec/unit/interface/action_builder_spec.rb @@ -8,7 +8,7 @@ describe Puppet::Interface::ActionBuilder do it "should build an action" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end end action.should be_a(Puppet::Interface::Action) action.name.should == :foo @@ -16,7 +16,7 @@ describe Puppet::Interface::ActionBuilder do it "should define a method on the face which invokes the action" do face = Puppet::Interface.new(:action_builder_test_interface, '0.0.1') do - action(:foo) { when_invoked { "invoked the method" } } + action(:foo) { when_invoked { |options| "invoked the method" } } end face.foo.should == "invoked the method" @@ -37,7 +37,7 @@ describe Puppet::Interface::ActionBuilder do it "should have a #option DSL function" do method = nil Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end method = self.method(:option) end method.should be_an_instance_of Method @@ -45,7 +45,7 @@ describe Puppet::Interface::ActionBuilder do it "should define an option without a block" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end option "--bar" end action.should be_option :bar @@ -53,7 +53,7 @@ describe Puppet::Interface::ActionBuilder do it "should accept an empty block" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end option "--bar" do # This space left deliberately blank. end @@ -65,7 +65,7 @@ describe Puppet::Interface::ActionBuilder do context "inline documentation" do it "should set the summary" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end summary "this is some text" end action.summary.should == "this is some text" @@ -75,7 +75,7 @@ describe Puppet::Interface::ActionBuilder do context "action defaulting" do it "should set the default to true" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end default end action.default.should be_true @@ -83,7 +83,7 @@ describe Puppet::Interface::ActionBuilder do it "should not be default by, er, default. *cough*" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end end action.default.should be_false end @@ -93,7 +93,7 @@ describe Puppet::Interface::ActionBuilder do it "should fail if no rendering format is given" do expect { Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering do true end end }.to raise_error ArgumentError, /must give a rendering format to when_rendering/ @@ -102,7 +102,7 @@ describe Puppet::Interface::ActionBuilder do it "should fail if no block is given" do expect { Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering :json end }.to raise_error ArgumentError, /must give a block to when_rendering/ @@ -111,7 +111,7 @@ describe Puppet::Interface::ActionBuilder do it "should fail if the block takes no arguments" do expect { Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering :json do true end end }.to raise_error ArgumentError, /when_rendering methods take one argument, the result, not/ @@ -120,7 +120,7 @@ describe Puppet::Interface::ActionBuilder do it "should fail if the block takes more than one argument" do expect { Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering :json do |a, b, c| true end end }.to raise_error ArgumentError, /when_rendering methods take one argument, the result, not/ @@ -129,7 +129,7 @@ describe Puppet::Interface::ActionBuilder do it "should fail if the block takes a variable number of arguments" do expect { Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering :json do |*args| true end end }.to raise_error(ArgumentError, @@ -138,7 +138,7 @@ describe Puppet::Interface::ActionBuilder do it "should stash a rendering block" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering :json do |a| true end end action.when_rendering(:json).should be_an_instance_of Method @@ -147,7 +147,7 @@ describe Puppet::Interface::ActionBuilder do it "should fail if you try to set the same rendering twice" do expect { Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering :json do |a| true end when_rendering :json do |a| true end end @@ -156,7 +156,7 @@ describe Puppet::Interface::ActionBuilder do it "should work if you set two different renderings" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering :json do |a| true end when_rendering :yaml do |a| true end end @@ -166,7 +166,7 @@ describe Puppet::Interface::ActionBuilder do it "should be bound to the face when called" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end when_rendering :json do |a| self end end action.when_rendering(:json).call(true).should == face @@ -176,7 +176,7 @@ describe Puppet::Interface::ActionBuilder do context "#render_as" do it "should default to nil (eg: based on context)" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end end action.render_as.should be_nil end @@ -184,7 +184,7 @@ describe Puppet::Interface::ActionBuilder do it "should fail if not rendering format is given" do expect { Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end render_as end }.to raise_error ArgumentError, /must give a rendering format to render_as/ @@ -193,7 +193,7 @@ describe Puppet::Interface::ActionBuilder do Puppet::Network::FormatHandler.formats.each do |name| it "should accept #{name.inspect} format" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end render_as name end action.render_as.should == name @@ -204,7 +204,7 @@ describe Puppet::Interface::ActionBuilder do it "should fail if given #{input.inspect}" do expect { Puppet::Interface::ActionBuilder.build(face, :foo) do - when_invoked do true end + when_invoked do |options| true end render_as input end }.to raise_error ArgumentError, /#{input.inspect} is not a valid rendering format/ diff --git a/spec/unit/interface/action_manager_spec.rb b/spec/unit/interface/action_manager_spec.rb index 5a479ad..e357a5f 100755 --- a/spec/unit/interface/action_manager_spec.rb +++ b/spec/unit/interface/action_manager_spec.rb @@ -15,57 +15,57 @@ describe Puppet::Interface::ActionManager do describe "when included in a class" do it "should be able to define an action" do subject.action(:foo) do - when_invoked { "something "} + when_invoked { |options| "something "} end end it "should be able to define a 'script' style action" do - subject.script :bar do + subject.script :bar do |options| "a bar is where beer is found" end end it "should be able to list defined actions" do subject.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end subject.action(:bar) do - when_invoked { "something" } + when_invoked { |options| "something" } end subject.actions.should =~ [:foo, :bar] end it "should list 'script' actions" do - subject.script :foo do "foo" end + subject.script :foo do |options| "foo" end subject.actions.should =~ [:foo] end it "should list both script and normal actions" do subject.action :foo do - when_invoked do "foo" end + when_invoked do |options| "foo" end end - subject.script :bar do "a bar is where beer is found" end + subject.script :bar do |options| "a bar is where beer is found" end subject.actions.should =~ [:foo, :bar] end it "should be able to indicate when an action is defined" do subject.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end subject.should be_action(:foo) end it "should indicate an action is defined for script actions" do - subject.script :foo do "foo" end + subject.script :foo do |options| "foo" end subject.should be_action :foo end it "should correctly treat action names specified as strings" do subject.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end subject.should be_action("foo") @@ -77,16 +77,16 @@ describe Puppet::Interface::ActionManager do it "should be able to define an action" do subject.action(:foo) do - when_invoked { "something "} + when_invoked { |options| "something "} end end it "should be able to list defined actions" do subject.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end subject.action(:bar) do - when_invoked { "something" } + when_invoked { |options| "something" } end subject.actions.should include(:bar) @@ -94,7 +94,7 @@ describe Puppet::Interface::ActionManager do end it "should be able to indicate when an action is defined" do - subject.action(:foo) { when_invoked do true end } + subject.action(:foo) { when_invoked do |options| true end } subject.should be_action(:foo) end end @@ -112,36 +112,36 @@ describe Puppet::Interface::ActionManager do it "should be able to define an action at the class level" do @klass.action(:foo) do - when_invoked { "something "} + when_invoked { |options| "something "} end end it "should create an instance method when an action is defined at the class level" do @klass.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.foo.should == "something" end it "should be able to define an action at the instance level" do @instance.action(:foo) do - when_invoked { "something "} + when_invoked { |options| "something "} end end it "should create an instance method when an action is defined at the instance level" do @instance.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.foo.should == "something" end it "should be able to list actions defined at the class level" do @klass.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end @klass.action(:bar) do - when_invoked { "something" } + when_invoked { |options| "something" } end @klass.actions.should include(:bar) @@ -150,10 +150,10 @@ describe Puppet::Interface::ActionManager do it "should be able to list actions defined at the instance level" do @instance.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.action(:bar) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.actions.should include(:bar) @@ -162,10 +162,10 @@ describe Puppet::Interface::ActionManager do it "should be able to list actions defined at both instance and class level" do @klass.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.action(:bar) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.actions.should include(:bar) @@ -174,14 +174,14 @@ describe Puppet::Interface::ActionManager do it "should be able to indicate when an action is defined at the class level" do @klass.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.should be_action(:foo) end it "should be able to indicate when an action is defined at the instance level" do @klass.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.should be_action(:foo) end @@ -191,13 +191,13 @@ describe Puppet::Interface::ActionManager do @instance = @subclass.new @klass.action(:parent) do - when_invoked { "a" } + when_invoked { |options| "a" } end @subclass.action(:sub) do - when_invoked { "a" } + when_invoked { |options| "a" } end @instance.action(:instance) do - when_invoked { "a" } + when_invoked { |options| "a" } end @instance.should be_action(:parent) @@ -210,7 +210,7 @@ describe Puppet::Interface::ActionManager do @instance = @subclass.new @klass.action(:foo) do - when_invoked { "something" } + when_invoked { |options| "something" } end @instance.foo.should == "something" end @@ -218,19 +218,19 @@ describe Puppet::Interface::ActionManager do describe "#action" do it 'should add an action' do - subject.action(:foo) { when_invoked do true end } + subject.action(:foo) { when_invoked do |options| true end } subject.get_action(:foo).should be_a Puppet::Interface::Action end it 'should support default actions' do - subject.action(:foo) { when_invoked do true end; default } + subject.action(:foo) { when_invoked do |options| true end; default } subject.get_default_action.should == subject.get_action(:foo) end it 'should not support more than one default action' do - subject.action(:foo) { when_invoked do true end; default } + subject.action(:foo) { when_invoked do |options| true end; default } expect { subject.action(:bar) { - when_invoked do true end + when_invoked do |options| true end default } }.should raise_error /cannot both be default/ @@ -240,7 +240,7 @@ describe Puppet::Interface::ActionManager do describe "#get_action" do let :parent_class do parent_class = Class.new(Puppet::Interface) - parent_class.action(:foo) { when_invoked do true end } + parent_class.action(:foo) { when_invoked do |options| true end } parent_class end diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb index 9e539c6..cf8d61d 100755 --- a/spec/unit/interface/action_spec.rb +++ b/spec/unit/interface/action_spec.rb @@ -20,11 +20,10 @@ describe Puppet::Interface::Action do expect { Puppet::Interface.new(:action_when_invoked, '1.0.0') do action :foo do - when_invoked do - end + when_invoked { } end end - }.to raise_error ArgumentError, /foobra/ + }.to raise_error ArgumentError, /foo/ end it "should work with arity 1 blocks" do @@ -72,11 +71,11 @@ describe Puppet::Interface::Action do it "should be able to call other actions on the same object" do face = Puppet::Interface.new(:my_face, '0.0.1') do action(:foo) do - when_invoked { 25 } + when_invoked { |options| 25 } end action(:bar) do - when_invoked { "the value of foo is '#{foo}'" } + when_invoked { |options| "the value of foo is '#{foo}'" } end end face.foo.should == 25 @@ -90,25 +89,25 @@ describe Puppet::Interface::Action do it "should be able to call other actions on the same object when defined on a class" do class Puppet::Interface::MyInterfaceBaseClass < Puppet::Interface action(:foo) do - when_invoked { 25 } + when_invoked { |options| 25 } end action(:bar) do - when_invoked { "the value of foo is '#{foo}'" } + when_invoked { |options| "the value of foo is '#{foo}'" } end action(:quux) do - when_invoked { "qux told me #{qux}" } + when_invoked { |options| "qux told me #{qux}" } end end face = Puppet::Interface::MyInterfaceBaseClass.new(:my_inherited_face, '0.0.1') do action(:baz) do - when_invoked { "the value of foo in baz is '#{foo}'" } + when_invoked { |options| "the value of foo in baz is '#{foo}'" } end action(:qux) do - when_invoked { baz } + when_invoked { |options| baz } end end face.foo.should == 25 @@ -150,7 +149,7 @@ describe Puppet::Interface::Action do it "should support options with an empty block" do face = Puppet::Interface.new(:action_level_options, '0.0.1') do action :foo do - when_invoked do true end + when_invoked do |options| true end option "--bar" do # this line left deliberately blank end @@ -164,7 +163,7 @@ describe Puppet::Interface::Action do it "should return only action level options when there are no face options" do face = Puppet::Interface.new(:action_level_options, '0.0.1') do action :foo do - when_invoked do true end + when_invoked do |options| true end option "--bar" end end @@ -175,8 +174,8 @@ describe Puppet::Interface::Action do describe "with both face and action options" do let :face do Puppet::Interface.new(:action_level_options, '0.0.1') do - action :foo do when_invoked do true end ; option "--bar" end - action :baz do when_invoked do true end ; option "--bim" end + action :foo do when_invoked do |options| true end ; option "--bar" end + action :baz do when_invoked do |options| true end ; option "--bim" end option "--quux" end end @@ -191,7 +190,7 @@ describe Puppet::Interface::Action do child = parent.new(:inherited_options, '0.0.1') do option "--bar" action :action do - when_invoked do true end + when_invoked do |options| true end option "--baz" end end @@ -223,7 +222,7 @@ describe Puppet::Interface::Action do def add_options_to(&block) face = Puppet::Interface.new(:with_options, '0.0.1') do action(:foo) do - when_invoked do true end + when_invoked do |options| true end self.instance_eval &block end end @@ -244,7 +243,7 @@ describe Puppet::Interface::Action do face = Puppet::Interface.new(:required_action_option, '0.0.1') do action(:bar) do option('--foo') { required } - when_invoked { } + when_invoked {|options| } end end expect { face.bar }.to raise_error ArgumentError, /The following options are required: foo/ @@ -253,7 +252,7 @@ describe Puppet::Interface::Action do it "should fail when a required face option is not provided" do face = Puppet::Interface.new(:required_face_option, '0.0.1') do option('--foo') { required } - action(:bar) { when_invoked { } } + action(:bar) { when_invoked {|options| } } end expect { face.bar }.to raise_error ArgumentError, /The following options are required: foo/ end @@ -263,7 +262,7 @@ describe Puppet::Interface::Action do context "declared locally" do let :face do Puppet::Interface.new(:action_decorators, '0.0.1') do - action :bar do when_invoked do true end end + action :bar do when_invoked do |options| true end end def reported; @reported; end def report(arg) (@reported ||= []) << arg @@ -278,7 +277,7 @@ describe Puppet::Interface::Action do option("-q", "--quux") { before_action { |_,_,_| report :quux } } option("-f") { before_action { |_,_,_| report :f } } option("--baz") { before_action { |_,_,_| report :baz } } - when_invoked { } + when_invoked {|options| } end face.boo :foo => 1, :bar => 1, :quux => 1, :f => 1, :baz => 1 @@ -292,7 +291,7 @@ describe Puppet::Interface::Action do option("-q", "--quux") { after_action { |_,_,_| report :quux } } option("-f") { after_action { |_,_,_| report :f } } option("--baz") { after_action { |_,_,_| report :baz } } - when_invoked { } + when_invoked {|options| } end face.boo :foo => 1, :bar => 1, :quux => 1, :f => 1, :baz => 1 @@ -307,7 +306,7 @@ describe Puppet::Interface::Action do option("-f") { before_action { |_,_,_| report :f } } option("--baz") { before_action { |_,_,_| report :baz } } end - face.script(:boo) { } + face.script(:boo) {|options| } face.boo :foo => 1, :bar => 1, :quux => 1, :f => 1, :baz => 1 face.reported.should == [ :foo, :bar, :quux, :f, :baz ] @@ -321,7 +320,7 @@ describe Puppet::Interface::Action do option("-f") { after_action { |_,_,_| report :f } } option("--baz") { after_action { |_,_,_| report :baz } } end - face.script(:boo) { } + face.script(:boo) {|options| } face.boo :foo => 1, :bar => 1, :quux => 1, :f => 1, :baz => 1 face.reported.should == [ :foo, :bar, :quux, :f, :baz ].reverse @@ -337,7 +336,7 @@ describe Puppet::Interface::Action do option("-q", "--action-quux") { before_action { |_,_,_| report :action_quux } } option("-a") { before_action { |_,_,_| report :a } } option("--action-baz") { before_action { |_,_,_| report :action_baz } } - when_invoked { } + when_invoked {|options| } end option("-u", "--face-quux") { before_action { |_,_,_| report :face_quux } } option("-f") { before_action { |_,_,_| report :f } } @@ -360,7 +359,7 @@ describe Puppet::Interface::Action do option("-q", "--action-quux") { after_action { |_,_,_| report :action_quux } } option("-a") { after_action { |_,_,_| report :a } } option("--action-baz") { after_action { |_,_,_| report :action_baz } } - when_invoked { } + when_invoked {|options| } end option("-u", "--face-quux") { after_action { |_,_,_| report :face_quux } } option("-f") { after_action { |_,_,_| report :f } } @@ -405,7 +404,7 @@ describe Puppet::Interface::Action do context "and inheritance" do let :parent do Class.new(Puppet::Interface) do - script(:on_parent) { :on_parent } + script(:on_parent) {|options| :on_parent } def reported; @reported; end def report(arg) @@ -416,7 +415,7 @@ describe Puppet::Interface::Action do let :child do parent.new(:inherited_decorators, '0.0.1') do - script(:on_child) { :on_child } + script(:on_child) {|options| :on_child } end end @@ -479,7 +478,7 @@ describe Puppet::Interface::Action do after_action { |action, args, options| report :b_after } end - child.script(:decorations) { report :invoked } + child.script(:decorations) { |options| report :invoked } child end @@ -509,7 +508,7 @@ describe Puppet::Interface::Action do subject do face = Puppet::Interface.new(:action_documentation, '0.0.1') do action :documentation do - when_invoked do true end + when_invoked do |options| true end end end face.get_action(:documentation) @@ -528,7 +527,7 @@ describe Puppet::Interface::Action do context "#validate_args" do subject do Puppet::Interface.new(:validate_args, '1.0.0') do - script :test do true end + script :test do |options| true end end end -- 1.7.3.1 -- 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.
