Merge branch 'feature/2.7.x/6978-face-and-action-options-should-have-hooks-for-various-actions' into 2.7.x

6 files changed, 252 insertions, 20 deletions

diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb
index 8f29c8a7b..aecaf198f 100755
--- a/spec/unit/interface/action_builder_spec.rb
+++ b/spec/unit/interface/action_builder_spec.rb
@@ -12,10 +12,11 @@ describe Puppet::Interface::ActionBuilder do
     end
 
     it "should define a method on the face which invokes the action" do
-      face = Puppet::Interface.new(:action_builder_test_interface, '0.0.1')
-      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
-        when_invoked do
-          "invoked the method"
+      face = Puppet::Interface.new(:action_builder_test_interface, '0.0.1') do
+        action :foo do
+          when_invoked do
+            "invoked the method"
+          end
         end
       end
 
diff --git a/spec/unit/interface/action_manager_spec.rb b/spec/unit/interface/action_manager_spec.rb
index 387faa043..6a6c254d4 100755
--- a/spec/unit/interface/action_manager_spec.rb
+++ b/spec/unit/interface/action_manager_spec.rb
@@ -104,6 +104,7 @@ describe Puppet::Interface::ActionManager do
       @klass = Class.new do
         include Puppet::Interface::ActionManager
         extend Puppet::Interface::ActionManager
+        def __invoke_decorations(*args) true end
       end
       @instance = @klass.new
     end
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index 8c6782976..4be6a1c55 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -79,7 +79,7 @@ describe Puppet::Interface::Action do
       end
 
       it "should work when options are supplied" do
-        options = face.bar :bar => "beer"
+        options = face.bar(:bar => "beer")
         options.should == { :bar => "beer" }
       end
     end
@@ -169,4 +169,183 @@ describe Puppet::Interface::Action do
       }.should raise_error ArgumentError, /Option foo conflicts with existing option foo/i
     end
   end
+
+  context "with action decorators" do
+    context "local only" do
+      let :face do
+        Puppet::Interface.new(:action_decorators, '0.0.1') do
+          action :bar do when_invoked do true end end
+          def report(arg) end
+        end
+      end
+
+      it "should call action before decorators" do
+        face.action(:baz) do
+          option "--baz" do
+            before_action do |action, args, options|
+              report(:action_option)
+            end
+          end
+          when_invoked do true end
+        end
+
+        face.expects(:report).with(:action_option)
+        face.baz :baz => true
+      end
+
+      it "should call action after decorators" do
+        face.action(:baz) do
+          option "--baz" do
+            after_action do |action, args, options|
+              report(:action_option)
+            end
+          end
+          when_invoked do true end
+        end
+
+        face.expects(:report).with(:action_option)
+        face.baz :baz => true
+      end
+
+      it "should call local before decorators" do
+        face.option "--foo FOO" do
+          before_action do |action, args, options|
+            report(:before)
+          end
+        end
+        face.expects(:report).with(:before)
+        face.bar({:foo => 12})
+      end
+
+      it "should call local after decorators" do
+        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
+
+      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 |action, args, options|
+              report :before_action
+            end
+          end
+          face.expects(:report).never # I am testing the negative.
+          face.bar
+        end
+
+        context "with some decorators only" do
+          before :each do
+            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
+            face.expects(:report).with(:foo)
+            face.bar(:foo => true)
+          end
+
+          it "should work with the bar option" do
+            face.expects(:report).with(:bar)
+            face.bar(:bar => true)
+          end
+
+          it "should work with both options" do
+            face.expects(:report).with(:foo)
+            face.expects(:report).with(:bar)
+            face.bar(:foo => true, :bar => true)
+          end
+        end
+      end
+    end
+
+    context "with inherited decorators" do
+      let :parent do
+        parent = Class.new(Puppet::Interface)
+        parent.script :on_parent do :on_parent end
+        parent.define_method :report do |arg| arg end
+        parent
+      end
+
+      let :child do
+        child = parent.new(:inherited_decorators, '0.0.1') do
+          script :on_child do :on_child end
+        end
+      end
+
+      context "with a child decorator" do
+        subject do
+          child.option "--foo FOO" do
+            before_action do |action, args, options|
+              report(:child_before)
+            end
+          end
+          child.expects(:report).with(:child_before)
+          child
+        end
+
+        it "child actions should invoke the decorator" do
+          subject.on_child({:foo => true, :bar => true}).should == :on_child
+        end
+
+        it "parent actions should invoke the decorator" do
+          subject.on_parent({:foo => true, :bar => true}).should == :on_parent
+        end
+      end
+
+      context "with a parent decorator" do
+        subject do
+          parent.option "--foo FOO" do
+            before_action do |action, args, options|
+              report(:parent_before)
+            end
+          end
+          child.expects(:report).with(:parent_before)
+          child
+        end
+
+        it "child actions should invoke the decorator" do
+          subject.on_child({:foo => true, :bar => true}).should == :on_child
+        end
+
+        it "parent actions should invoke the decorator" do
+          subject.on_parent({:foo => true, :bar => true}).should == :on_parent
+        end
+      end
+
+      context "with child and parent decorators" do
+        subject do
+          parent.option "--foo FOO" do
+            before_action { |action, args, options| report(:parent_before) }
+            after_action  { |action, args, options| report(:parent_after)  }
+          end
+          child.option "--bar BAR" do
+            before_action { |action, args, options| report(:child_before) }
+            after_action  { |action, args, options| report(:child_after)  }
+          end
+
+          child.expects(:report).with(:child_before)
+          child.expects(:report).with(:parent_before)
+          child.expects(:report).with(:parent_after)
+          child.expects(:report).with(:child_after)
+
+          child
+        end
+
+        it "child actions should invoke all the decorator" do
+          subject.on_child({:foo => true, :bar => true}).should == :on_child
+        end
+
+        it "parent actions should invoke all the decorator" do
+          subject.on_parent({:foo => true, :bar => true}).should == :on_parent
+        end
+      end
+    end
+  end
 end
diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb
index d1114dde7..f9498cbb8 100755
--- a/spec/unit/interface/face_collection_spec.rb
+++ b/spec/unit/interface/face_collection_spec.rb
@@ -24,10 +24,6 @@ describe Puppet::Interface::FaceCollection do
     $".clear ; @original_required.each do |item| $" << item end
   end
 
-  describe "::faces" do
-    it "REVISIT: should have some tests here, if we describe it"
-  end
-
   describe "::validate_version" do
     it 'should permit three number versions' do
       subject.validate_version('10.10.10').should == true
diff --git a/spec/unit/interface/option_builder_spec.rb b/spec/unit/interface/option_builder_spec.rb
index fae48324e..b32b316f6 100755
--- a/spec/unit/interface/option_builder_spec.rb
+++ b/spec/unit/interface/option_builder_spec.rb
@@ -8,22 +8,53 @@ describe Puppet::Interface::OptionBuilder do
       should be_an_instance_of Puppet::Interface::Option
   end
 
-  describe "when using the DSL block" do
-    it "should work with an empty block" do
-      option = Puppet::Interface::OptionBuilder.build(face, "--foo") do
-        # This block deliberately left blank.
-      end
+  it "should work with an empty block" do
+    option = Puppet::Interface::OptionBuilder.build(face, "--foo") do
+      # This block deliberately left blank.
+    end
 
-      option.should be_an_instance_of Puppet::Interface::Option
+    option.should be_an_instance_of Puppet::Interface::Option
+  end
+
+  it "should support documentation declarations" do
+    text = "this is the description"
+    option = Puppet::Interface::OptionBuilder.build(face, "--foo") do
+      desc text
     end
+    option.should be_an_instance_of Puppet::Interface::Option
+    option.desc.should == text
+  end
 
-    it "should support documentation declarations" do
-      text = "this is the description"
+  context "before_action hook" do
+    it "should support a before_action hook" do
       option = Puppet::Interface::OptionBuilder.build(face, "--foo") do
-        desc text
+        before_action do |a,b,c| :whatever end
       end
-      option.should be_an_instance_of Puppet::Interface::Option
-      option.desc.should == text
+      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
   end
 end
diff --git a/spec/unit/interface/option_spec.rb b/spec/unit/interface/option_spec.rb
index 3bcd121e2..4c24dc940 100755
--- a/spec/unit/interface/option_spec.rb
+++ b/spec/unit/interface/option_spec.rb
@@ -72,4 +72,28 @@ describe Puppet::Interface::Option do
       option.to_s.should == "foo-bar"
     end
   end
+
+  %w{before after}.each do |side|
+    describe "#{side} hooks" do
+      subject { Puppet::Interface::Option.new(face, "--foo") }
+      let :proc do Proc.new do :from_proc end end
+
+      it { should respond_to "#{side}_action" }
+      it { should respond_to "#{side}_action=" }
+
+      it "should set the #{side}_action hook" do
+        subject.send("#{side}_action").should be_nil
+        subject.send("#{side}_action=", proc)
+        subject.send("#{side}_action").should be_an_instance_of UnboundMethod
+      end
+
+      data = [1, "foo", :foo, Object.new, method(:hash), method(:hash).unbind]
+      data.each do |input|
+        it "should fail if a #{input.class} is added to the #{side} hooks" do
+          expect { subject.send("#{side}_action=", input) }.
+            to raise_error ArgumentError, /not a proc/
+        end
+      end
+    end
+  end
 end