[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5
Pieter van de Bruggen
pieter at puppetlabs.com
Tue May 10 08:18:45 UTC 2011
The following commit has been merged in the experimental branch:
commit 207deae2dc06ca439e3b5ee9b044221a1c2899bb
Author: Pieter van de Bruggen <pieter at puppetlabs.com>
Date: Fri Apr 29 12:18:12 2011 -0700
(#7289) Specify order for option decorations.
`before_action` decorations should always resolve in resolution order
from most general (inherited from furthest away) to most specific
(declared on the instance), and should always execute Face-level
option decorations before action-level option decorations.
`after_action` decorations should execute in the opposite order.
Reviewed-By: Daniel Pittman
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index ba68ac6..eb376c4 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -139,6 +139,8 @@ class Puppet::Interface
action.get_option(name).__decoration_name(type)
end
+ methods.reverse! if type == :after
+
# Exceptions here should propagate up; this implements a hook we can use
# reasonably for option validation.
methods.each do |hook|
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index 464b2a7..96bb5b4 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -9,7 +9,10 @@ class Puppet::Interface::Action
@name = name.to_sym
attrs.each do |k, v| send("#{k}=", v) end
- @options = {}
+ # @options collects the added options in the order they're declared.
+ # @options_hash collects the options keyed by alias for quick lookups.
+ @options = []
+ @options_hash = {}
@when_rendering = {}
end
@@ -211,22 +214,23 @@ WRAPPER
end
option.aliases.each do |name|
- @options[name] = option
+ @options << name
+ @options_hash[name] = option
end
option
end
def option?(name)
- @options.include? name.to_sym
+ @options_hash.include? name.to_sym
end
def options
- (@options.keys + @face.options).sort
+ @face.options + @options
end
def get_option(name, with_inherited_options = true)
- option = @options[name.to_sym]
+ option = @options_hash[name.to_sym]
if option.nil? and with_inherited_options
option = @face.get_option(name)
end
diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb
index d42359c..326a91d 100644
--- a/lib/puppet/interface/option_manager.rb
+++ b/lib/puppet/interface/option_manager.rb
@@ -8,6 +8,11 @@ module Puppet::Interface::OptionManager
end
def add_option(option)
+ # @options collects the added options in the order they're declared.
+ # @options_hash collects the options keyed by alias for quick lookups.
+ @options ||= []
+ @options_hash ||= {}
+
option.aliases.each do |name|
if conflict = get_option(name) then
raise ArgumentError, "Option #{option} conflicts with existing option #{conflict}"
@@ -21,25 +26,30 @@ module Puppet::Interface::OptionManager
end
end
- option.aliases.each { |name| @options[name] = option }
- option
+ option.aliases.each do |name|
+ @options << name
+ @options_hash[name] = option
+ end
+
+ return option
end
def options
- @options ||= {}
- result = @options.keys
+ result = (@options ||= [])
if self.is_a?(Class) and superclass.respond_to?(:options)
- result += superclass.options
+ result = superclass.options + result
elsif self.class.respond_to?(:options)
- result += self.class.options
+ result = self.class.options + result
end
- result.sort
+
+ return result
end
def get_option(name, with_inherited_options = true)
- @options ||= {}
- result = @options[name.to_sym]
+ @options_hash ||= {}
+
+ result = @options_hash[name.to_sym]
if result.nil? and with_inherited_options then
if self.is_a?(Class) and superclass.respond_to?(:get_option)
result = superclass.get_option(name)
@@ -47,6 +57,7 @@ module Puppet::Interface::OptionManager
result = self.class.get_option(name)
end
end
+
return result
end
diff --git a/spec/shared_behaviours/things_that_declare_options.rb b/spec/shared_behaviours/things_that_declare_options.rb
index 5300a15..4ffe55d 100755
--- a/spec/shared_behaviours/things_that_declare_options.rb
+++ b/spec/shared_behaviours/things_that_declare_options.rb
@@ -37,9 +37,12 @@ shared_examples_for "things that declare options" do
it "should list all the options" do
thing = add_options_to do
option "--foo"
- option "--bar"
+ option "--bar", '-b'
+ option "-q", "--quux"
+ option "-f"
+ option "--baz"
end
- thing.options.should =~ [:foo, :bar]
+ thing.options.should == [:foo, :bar, :b, :q, :quux, :f, :baz]
end
it "should detect conflicts in long options" do
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index 23d0de4..8d0606d 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -249,180 +249,247 @@ describe Puppet::Interface::Action do
end
end
- context "with action decorators" do
- context "local only" do
+ context "with decorators" 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
- def report(arg) end
+ def reported; @reported; end
+ def report(arg)
+ (@reported ||= []) << 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
+ it "should execute before advice on action options in declaration order" do
+ face.action(:boo) do
+ option("--foo") { before_action { |_,_,_| report :foo } }
+ option("--bar", '-b') { before_action { |_,_,_| report :bar } }
+ option("-q", "--quux") { before_action { |_,_,_| report :quux } }
+ option("-f") { before_action { |_,_,_| report :f } }
+ option("--baz") { before_action { |_,_,_| report :baz } }
+ when_invoked { }
end
- face.expects(:report).with(:action_option)
- face.baz :baz => true
+ face.boo :foo => 1, :bar => 1, :quux => 1, :f => 1, :baz => 1
+ face.reported.should == [ :foo, :bar, :quux, :f, :baz ]
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
+ it "should execute after advice on action options in declaration order" do
+ face.action(:boo) do
+ option("--foo") { after_action { |_,_,_| report :foo } }
+ option("--bar", '-b') { after_action { |_,_,_| report :bar } }
+ option("-q", "--quux") { after_action { |_,_,_| report :quux } }
+ option("-f") { after_action { |_,_,_| report :f } }
+ option("--baz") { after_action { |_,_,_| report :baz } }
+ when_invoked { }
end
- face.expects(:report).with(:action_option)
- face.baz :baz => true
+ face.boo :foo => 1, :bar => 1, :quux => 1, :f => 1, :baz => 1
+ face.reported.should == [ :foo, :bar, :quux, :f, :baz ].reverse
end
- it "should call local before decorators" do
- face.option "--foo FOO" do
- before_action do |action, args, options|
- report(:before)
- end
+ it "should execute before advice on face options in declaration order" do
+ face.instance_eval do
+ option("--foo") { before_action { |_,_,_| report :foo } }
+ option("--bar", '-b') { before_action { |_,_,_| report :bar } }
+ option("-q", "--quux") { before_action { |_,_,_| report :quux } }
+ option("-f") { before_action { |_,_,_| report :f } }
+ option("--baz") { before_action { |_,_,_| report :baz } }
end
- face.expects(:report).with(:before)
- face.bar({:foo => 12})
+ face.script(:boo) { }
+
+ face.boo :foo => 1, :bar => 1, :quux => 1, :f => 1, :baz => 1
+ face.reported.should == [ :foo, :bar, :quux, :f, :baz ]
end
- it "should call local after decorators" do
- face.option "--foo FOO" do
- after_action do |action, args, options| report(:after) end
+ it "should execute after advice on face options in declaration order" do
+ face.instance_eval do
+ option("--foo") { after_action { |_,_,_| report :foo } }
+ option("--bar", '-b') { after_action { |_,_,_| report :bar } }
+ option("-q", "--quux") { after_action { |_,_,_| report :quux } }
+ option("-f") { after_action { |_,_,_| report :f } }
+ option("--baz") { after_action { |_,_,_| report :baz } }
end
- face.expects(:report).with(:after)
- face.bar({:foo => 12})
+ face.script(:boo) { }
+
+ face.boo :foo => 1, :bar => 1, :quux => 1, :f => 1, :baz => 1
+ face.reported.should == [ :foo, :bar, :quux, :f, :baz ].reverse
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
+ it "should execute before advice on face options before action options" do
+ face.instance_eval do
+ option("--face-foo") { before_action { |_,_,_| report :face_foo } }
+ option("--face-bar", '-r') { before_action { |_,_,_| report :face_bar } }
+ action(:boo) do
+ option("--action-foo") { before_action { |_,_,_| report :action_foo } }
+ option("--action-bar", '-b') { before_action { |_,_,_| report :action_bar } }
+ 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 { }
+ end
+ option("-u", "--face-quux") { before_action { |_,_,_| report :face_quux } }
+ option("-f") { before_action { |_,_,_| report :f } }
+ option("--face-baz") { before_action { |_,_,_| report :face_baz } }
+ end
+
+ expected_calls = [ :face_foo, :face_bar, :face_quux, :f, :face_baz,
+ :action_foo, :action_bar, :action_quux, :a, :action_baz ]
+ face.boo Hash[ *expected_calls.zip([]).flatten ]
+ face.reported.should == expected_calls
+ end
+
+ it "should execute after advice on face options in declaration order" do
+ face.instance_eval do
+ option("--face-foo") { after_action { |_,_,_| report :face_foo } }
+ option("--face-bar", '-r') { after_action { |_,_,_| report :face_bar } }
+ action(:boo) do
+ option("--action-foo") { after_action { |_,_,_| report :action_foo } }
+ option("--action-bar", '-b') { after_action { |_,_,_| report :action_bar } }
+ 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 { |options| warn options.inspect }
end
- face.expects(:report).never # I am testing the negative.
- face.bar
+ option("-u", "--face-quux") { after_action { |_,_,_| report :face_quux } }
+ option("-f") { after_action { |_,_,_| report :f } }
+ option("--face-baz") { after_action { |_,_,_| report :face_baz } }
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
+ expected_calls = [ :face_foo, :face_bar, :face_quux, :f, :face_baz,
+ :action_foo, :action_bar, :action_quux, :a, :action_baz ]
+ face.boo Hash[ *expected_calls.zip([]).flatten ]
+ face.reported.should == expected_calls.reverse
+ end
- it "should work with the foo option" do
- face.expects(:report).with(:foo)
- face.bar(:foo => true)
- end
+ it "should not invoke a decorator if the options are empty" do
+ face.option("--foo FOO") { before_action { |_,_,_| report :before_action } }
+ face.expects(:report).never
+ face.bar
+ end
- it "should work with the bar option" do
- face.expects(:report).with(:bar)
- face.bar(:bar => true)
- end
+ context "passing a subset of the options" do
+ before :each do
+ face.option("--foo") { before_action { |_,_,_| report :foo } }
+ face.option("--bar") { before_action { |_,_,_| report :bar } }
+ 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
+ it "should invoke only foo's advice when passed only 'foo'" do
+ face.bar(:foo => true)
+ face.reported.should == [ :foo ]
+ end
+
+ it "should invoke only bar's advice when passed only 'bar'" do
+ face.bar(:bar => true)
+ face.reported.should == [ :bar ]
+ end
+
+ it "should invoke advice for all passed options" do
+ face.bar(:foo => true, :bar => true)
+ face.reported.should == [ :foo, :bar ]
end
end
end
- context "with inherited decorators" do
+ context "and inheritance" 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
+ Class.new(Puppet::Interface) do
+ script(:on_parent) { :on_parent }
+
+ def reported; @reported; end
+ def report(arg)
+ (@reported ||= []) << arg
+ end
+ end
end
let :child do
- child = parent.new(:inherited_decorators, '0.0.1') do
- script :on_child do :on_child end
+ parent.new(:inherited_decorators, '0.0.1') do
+ script(:on_child) { :on_child }
end
end
- context "with a child decorator" do
+ context "locally declared face options" 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.option("--foo=") { before_action { |_,_,_| report :child_before } }
child
end
- it "child actions should invoke the decorator" do
- subject.on_child({:foo => true, :bar => true}).should == :on_child
+ it "should be invoked when calling a child action" do
+ subject.on_child(:foo => true, :bar => true).should == :on_child
+ subject.reported.should == [ :child_before ]
end
- it "parent actions should invoke the decorator" do
- subject.on_parent({:foo => true, :bar => true}).should == :on_parent
+ it "should be invoked when calling a parent action" do
+ subject.on_parent(:foo => true, :bar => true).should == :on_parent
+ subject.reported.should == [ :child_before ]
end
end
- context "with a parent decorator" do
+ context "inherited face option decorators" 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)
+ parent.option("--foo=") { before_action { |_,_,_| report :parent_before } }
child
end
- it "child actions should invoke the decorator" do
- subject.on_child({:foo => true, :bar => true}).should == :on_child
+ it "should be invoked when calling a child action" do
+ subject.on_child(:foo => true, :bar => true).should == :on_child
+ subject.reported.should == [ :parent_before ]
end
- it "parent actions should invoke the decorator" do
- subject.on_parent({:foo => true, :bar => true}).should == :on_parent
+ it "should be invoked when calling a parent action" do
+ subject.on_parent(:foo => true, :bar => true).should == :on_parent
+ subject.reported.should == [ :parent_before ]
end
end
- context "with child and parent decorators" do
+ context "with both inherited and local face options" do
+ # Decorations should be invoked in declaration order, according to
+ # inheritance (e.g. parent class options should be handled before
+ # subclass options).
subject do
- parent.option "--foo FOO" do
- before_action { |action, args, options| report(:parent_before) }
- after_action { |action, args, options| report(:parent_after) }
+ child.option "-c" do
+ before_action { |action, args, options| report :c_before }
+ after_action { |action, args, options| report :c_after }
end
- child.option "--bar BAR" do
- before_action { |action, args, options| report(:child_before) }
- after_action { |action, args, options| report(:child_after) }
+
+ parent.option "-a" do
+ before_action { |action, args, options| report :a_before }
+ after_action { |action, args, options| report :a_after }
+ end
+
+ child.option "-d" do
+ before_action { |action, args, options| report :d_before }
+ after_action { |action, args, options| report :d_after }
+ end
+
+ parent.option "-b" do
+ before_action { |action, args, options| report :b_before }
+ after_action { |action, args, options| report :b_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.script(:decorations) { report :invoked }
child
end
- it "child actions should invoke all the decorator" do
- subject.on_child({:foo => true, :bar => true}).should == :on_child
+ it "should invoke all decorations when calling a child action" do
+ subject.decorations(:a => 1, :b => 1, :c => 1, :d => 1)
+ subject.reported.should == [
+ :a_before, :b_before, :c_before, :d_before,
+ :invoked,
+ :d_after, :c_after, :b_after, :a_after
+ ]
end
- it "parent actions should invoke all the decorator" do
- subject.on_parent({:foo => true, :bar => true}).should == :on_parent
+ it "should invoke all decorations when calling a parent action" do
+ subject.decorations(:a => 1, :b => 1, :c => 1, :d => 1)
+ subject.reported.should == [
+ :a_before, :b_before, :c_before, :d_before,
+ :invoked,
+ :d_after, :c_after, :b_after, :a_after
+ ]
end
end
end
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list