[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 f770325884ebef493cb8ca6060a65355211125b9
Author: Daniel Pittman <daniel at puppetlabs.com>
Date:   Fri Apr 15 15:06:53 2011 -0700

    (#6978) Enforce the calling convention of option hooks.
    
    We require that hooks take exactly three arguments; now we enforce that in the
    DSL, to ensure we give good, and early, errors to users who do the wrong
    thing.
    
    Paired-With: Max Martin <max at puppetlabs.com>

diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb
index ccad085..d4e59a4 100644
--- a/lib/puppet/interface/option_builder.rb
+++ b/lib/puppet/interface/option_builder.rb
@@ -31,6 +31,9 @@ class Puppet::Interface::OptionBuilder
     if @option.before_action
       raise ArgumentError, "#{@option} already has a before_action set"
     end
+    unless block.arity == 3 then
+      raise ArgumentError, "before_action takes three arguments, action, args, and options"
+    end
     @option.before_action = block
   end
 
@@ -39,6 +42,9 @@ class Puppet::Interface::OptionBuilder
     if @option.after_action
       raise ArgumentError, "#{@option} already has a after_action set"
     end
+    unless block.arity == 3 then
+      raise ArgumentError, "after_action takes three arguments, action, args, and options"
+    end
     @option.after_action = block
   end
 end
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index 51aa837..4be6a1c 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -182,7 +182,7 @@ describe Puppet::Interface::Action do
       it "should call action before decorators" do
         face.action(:baz) do
           option "--baz" do
-            before_action do
+            before_action do |action, args, options|
               report(:action_option)
             end
           end
@@ -196,7 +196,7 @@ describe Puppet::Interface::Action do
       it "should call action after decorators" do
         face.action(:baz) do
           option "--baz" do
-            after_action do
+            after_action do |action, args, options|
               report(:action_option)
             end
           end
@@ -209,7 +209,7 @@ describe Puppet::Interface::Action do
 
       it "should call local before decorators" do
         face.option "--foo FOO" do
-          before_action do
+          before_action do |action, args, options|
             report(:before)
           end
         end
@@ -218,7 +218,9 @@ describe Puppet::Interface::Action do
       end
 
       it "should call local after decorators" do
-        face.option "--foo FOO" do after_action do report(:after) end end
+        face.option "--foo FOO" do
+          after_action do |action, args, options| report(:after) end
+        end
         face.expects(:report).with(:after)
         face.bar({:foo => 12})
       end
@@ -226,7 +228,9 @@ describe Puppet::Interface::Action do
       context "with inactive decorators" do
         it "should not invoke a decorator if the options are empty" do
           face.option "--foo FOO" do
-            before_action do report :before_action end
+            before_action do |action, args, options|
+              report :before_action
+            end
           end
           face.expects(:report).never # I am testing the negative.
           face.bar
@@ -234,8 +238,12 @@ describe Puppet::Interface::Action do
 
         context "with some decorators only" do
           before :each do
-            face.option "--foo" do before_action do report :foo end end
-            face.option "--bar" do before_action do report :bar end end
+            face.option "--foo" do
+              before_action do |action, args, options| report :foo end
+            end
+            face.option "--bar" do
+              before_action do |action, args, options| report :bar end
+            end
           end
 
           it "should work with the foo option" do
@@ -274,7 +282,7 @@ describe Puppet::Interface::Action do
       context "with a child decorator" do
         subject do
           child.option "--foo FOO" do
-            before_action do
+            before_action do |action, args, options|
               report(:child_before)
             end
           end
@@ -294,7 +302,7 @@ describe Puppet::Interface::Action do
       context "with a parent decorator" do
         subject do
           parent.option "--foo FOO" do
-            before_action do
+            before_action do |action, args, options|
               report(:parent_before)
             end
           end
@@ -314,12 +322,12 @@ describe Puppet::Interface::Action do
       context "with child and parent decorators" do
         subject do
           parent.option "--foo FOO" do
-            before_action { report(:parent_before) }
-            after_action  { report(:parent_after)  }
+            before_action { |action, args, options| report(:parent_before) }
+            after_action  { |action, args, options| report(:parent_after)  }
           end
           child.option "--bar BAR" do
-            before_action { report(:child_before) }
-            after_action  { report(:child_after)  }
+            before_action { |action, args, options| report(:child_before) }
+            after_action  { |action, args, options| report(:child_after)  }
           end
 
           child.expects(:report).with(:child_before)
diff --git a/spec/unit/interface/option_builder_spec.rb b/spec/unit/interface/option_builder_spec.rb
index 0bcbed8..b32b316 100755
--- a/spec/unit/interface/option_builder_spec.rb
+++ b/spec/unit/interface/option_builder_spec.rb
@@ -25,10 +25,36 @@ describe Puppet::Interface::OptionBuilder do
     option.desc.should == text
   end
 
-  it "should support a before_action hook" do
-    option = Puppet::Interface::OptionBuilder.build(face, "--foo") do
-      before_action do :whatever end
+  context "before_action hook" do
+    it "should support a before_action hook" do
+      option = Puppet::Interface::OptionBuilder.build(face, "--foo") do
+        before_action do |a,b,c| :whatever end
+      end
+      option.before_action.should be_an_instance_of UnboundMethod
+    end
+
+    it "should fail if the hook block takes too few arguments" do
+      expect do
+        Puppet::Interface::OptionBuilder.build(face, "--foo") do
+          before_action do |one, two| true end
+        end
+      end.to raise_error ArgumentError, /takes three arguments/
+    end
+
+    it "should fail if the hook block takes too many arguments" do
+      expect do
+        Puppet::Interface::OptionBuilder.build(face, "--foo") do
+          before_action do |one, two, three, four| true end
+        end
+      end.to raise_error ArgumentError, /takes three arguments/
+    end
+
+    it "should fail if the hook block takes a variable number of arguments" do
+      expect do
+        Puppet::Interface::OptionBuilder.build(face, "--foo") do
+          before_action do |*blah| true end
+        end
+      end.to raise_error ArgumentError, /takes three arguments/
     end
-    option.before_action.should be_an_instance_of UnboundMethod
   end
 end

-- 
Puppet packaging for Debian