Merge branch 'bug/2.7.x/7282-fail-when-no-invocation-given-for-action' into 2.7.x

5 files changed, 71 insertions, 22 deletions

diff --git a/spec/lib/puppet/face/huzzah.rb b/spec/lib/puppet/face/huzzah.rb
index 2c2b7aa8d..6593d358a 100755
--- a/spec/lib/puppet/face/huzzah.rb
+++ b/spec/lib/puppet/face/huzzah.rb
@@ -3,5 +3,5 @@ Puppet::Face.define(:huzzah, '2.0.1') do
   copyright "Puppet Labs", 2011
   license   "Apache 2 license; see COPYING"
   summary "life is a thing for celebration"
-  action :bar do "is where beer comes from" end
+  script :bar do "is where beer comes from" end
 end
diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb
index bf7afa74e..e0d0ebe72 100755
--- a/spec/unit/interface/action_builder_spec.rb
+++ b/spec/unit/interface/action_builder_spec.rb
@@ -7,7 +7,8 @@ describe Puppet::Interface::ActionBuilder do
   let :face do Puppet::Interface.new(:puppet_interface_actionbuilder, '0.0.1') end
 
   it "should build an action" do
-    action = Puppet::Interface::ActionBuilder.build(nil, :foo) do
+    action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+      when_invoked do true end
     end
     action.should be_a(Puppet::Interface::Action)
     action.name.should == :foo
@@ -26,17 +27,25 @@ describe Puppet::Interface::ActionBuilder do
       should raise_error("Action :foo must specify a block")
   end
 
+  it "should require an invocation block" do
+    expect {
+      Puppet::Interface::ActionBuilder.build(face, :foo) {}
+    }.to raise_error(/actions need to know what to do when_invoked; please add the block/)
+  end
+
   describe "when handling options" do
     it "should have a #option DSL function" do
       method = nil
       Puppet::Interface::ActionBuilder.build(face, :foo) do
+        when_invoked do true end
         method = self.method(:option)
       end
-      method.should be
+      method.should be_an_instance_of Method
     end
 
     it "should define an option without a block" do
       action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        when_invoked do true end
         option "--bar"
       end
       action.should be_option :bar
@@ -44,6 +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
         option "--bar" do
           # This space left deliberately blank.
         end
@@ -55,6 +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
         summary "this is some text"
       end
       action.summary.should == "this is some text"
@@ -64,13 +75,16 @@ 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
         default
       end
       action.default.should be_true
     end
 
     it "should not be default by, er, default. *cough*" do
-      action = Puppet::Interface::ActionBuilder.build(face, :foo) do end
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        when_invoked do true end
+      end
       action.default.should be_false
     end
   end
@@ -79,6 +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_rendering do true end
         end
       }.to raise_error ArgumentError, /must give a rendering format to when_rendering/
@@ -87,6 +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_rendering :json
         end
       }.to raise_error ArgumentError, /must give a block to when_rendering/
@@ -95,6 +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_rendering :json do true end
         end
       }.to raise_error ArgumentError, /when_rendering methods take one argument, the result, not/
@@ -103,6 +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_rendering :json do |a, b, c| true end
         end
       }.to raise_error ArgumentError, /when_rendering methods take one argument, the result, not/
@@ -111,6 +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_rendering :json do |*args| true end
         end
       }.to raise_error(ArgumentError,
@@ -119,6 +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_rendering :json do |a| true end
       end
       action.when_rendering(:json).should be_an_instance_of Method
@@ -127,6 +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_rendering :json do |a| true end
           when_rendering :json do |a| true end
         end
@@ -135,6 +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_rendering :json do |a| true end
         when_rendering :yaml do |a| true end
       end
@@ -144,6 +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_rendering :json do |a| self end
       end
       action.when_rendering(:json).call(true).should == face
@@ -152,13 +175,16 @@ 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 end
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        when_invoked do true end
+      end
       action.render_as.should be_nil
     end
 
     it "should fail if not rendering format is given" do
       expect {
         Puppet::Interface::ActionBuilder.build(face, :foo) do
+          when_invoked do true end
           render_as
         end
       }.to raise_error ArgumentError, /must give a rendering format to render_as/
@@ -167,6 +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
           render_as name
         end
         action.render_as.should == name
@@ -175,6 +202,7 @@ describe Puppet::Interface::ActionBuilder do
 
     it "should accept :for_humans format" do
       action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        when_invoked do true end
         render_as :for_humans
       end
       action.render_as.should == :for_humans
@@ -184,6 +212,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
             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 07d517c18..5a479ad5c 100755
--- a/spec/unit/interface/action_manager_spec.rb
+++ b/spec/unit/interface/action_manager_spec.rb
@@ -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) { "something" }
+      subject.action(:foo) { when_invoked do true end }
       subject.should be_action(:foo)
     end
   end
@@ -218,25 +218,29 @@ describe Puppet::Interface::ActionManager do
 
   describe "#action" do
     it 'should add an action' do
-      subject.action(:foo) {  }
+      subject.action(:foo) { when_invoked do true end }
       subject.get_action(:foo).should be_a Puppet::Interface::Action
     end
 
     it 'should support default actions' do
-      subject.action(:foo) { default }
+      subject.action(:foo) { when_invoked do 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) { default }
-      expect { subject.action(:bar) { default } }.should raise_error
+      subject.action(:foo) { when_invoked do true end; default }
+      expect { subject.action(:bar) {
+          when_invoked do true end
+          default
+        }
+      }.should raise_error /cannot both be default/
     end
   end
 
   describe "#get_action" do
     let :parent_class do
       parent_class = Class.new(Puppet::Interface)
-      parent_class.action(:foo) {}
+      parent_class.action(:foo) { when_invoked do true end }
       parent_class
     end
 
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index ccd791575..9e539c68e 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -150,6 +150,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
           option "--bar" do
             # this line left deliberately blank
           end
@@ -162,7 +163,10 @@ 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 option "--bar" end
+        action :foo do
+          when_invoked do true end
+          option "--bar"
+        end
       end
 
       face.get_action(:foo).options.should =~ [:bar]
@@ -171,8 +175,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 option "--bar" end
-          action :baz do option "--bim" end
+          action :foo do when_invoked do true end ; option "--bar" end
+          action :baz do when_invoked do true end ; option "--bim" end
           option "--quux"
         end
       end
@@ -186,7 +190,10 @@ describe Puppet::Interface::Action do
         parent.option "--foo"
         child = parent.new(:inherited_options, '0.0.1') do
           option "--bar"
-          action :action do option "--baz" end
+          action :action do
+            when_invoked do true end
+            option "--baz"
+          end
         end
 
         action = child.get_action(:action)
@@ -215,7 +222,10 @@ describe Puppet::Interface::Action do
     it_should_behave_like "things that declare options" do
       def add_options_to(&block)
         face = Puppet::Interface.new(:with_options, '0.0.1') do
-          action(:foo, &block)
+          action(:foo) do
+            when_invoked do true end
+            self.instance_eval &block
+          end
         end
         face.get_action(:foo)
       end
@@ -498,7 +508,9 @@ describe Puppet::Interface::Action do
   it_should_behave_like "documentation on faces" do
     subject do
       face = Puppet::Interface.new(:action_documentation, '0.0.1') do
-        action :documentation do end
+        action :documentation do
+          when_invoked do true end
+        end
       end
       face.get_action(:documentation)
     end
diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb
index 27da39766..e28e55aac 100755
--- a/spec/unit/interface_spec.rb
+++ b/spec/unit/interface_spec.rb
@@ -146,6 +146,7 @@ describe Puppet::Interface do
         option "--foo"
         option "--bar"
         action :baz do
+          when_invoked { true }
           option "--quux"
         end
       end
@@ -155,7 +156,10 @@ describe Puppet::Interface do
     it "should fail when a face option duplicates an action option" do
       expect {
         subject.new(:action_level_options, '0.0.1') do
-          action :bar do option "--foo" end
+          action :bar do
+            when_invoked { true }
+            option "--foo"
+          end
           option "--foo"
         end
       }.should raise_error ArgumentError, /Option foo conflicts with existing option foo on/i
@@ -163,8 +167,8 @@ describe Puppet::Interface do
 
     it "should work when two actions have the same option" do
       face = subject.new(:with_options, '0.0.1') do
-        action :foo do option "--quux" end
-        action :bar do option "--quux" end
+        action :foo do when_invoked { true } ; option "--quux" end
+        action :bar do when_invoked { true } ; option "--quux" end
       end
 
       face.get_action(:foo).options.should =~ [:quux]
@@ -176,14 +180,14 @@ describe Puppet::Interface do
     let :parent do
       parent = Class.new(subject)
       parent.option("--inherited")
-      parent.action(:parent_action) do end
+      parent.action(:parent_action) do when_invoked { true } end
       parent
     end
 
     let :face do
       face = parent.new(:example, '0.2.1')
       face.option("--local")
-      face.action(:face_action) do end
+      face.action(:face_action) do when_invoked { true } end
       face
     end