[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:46 UTC 2011


The following commit has been merged in the experimental branch:
commit 07efb2463dfc4720df3996bb2f7a80844914f0da
Merge: 97ae812f0a67ef01daed4e9220981e2bc7c70603 207deae2dc06ca439e3b5ee9b044221a1c2899bb
Author: Pieter van de Bruggen <pieter at puppetlabs.com>
Date:   Fri Apr 29 13:36:36 2011 -0700

    Merge branch 'tickets/2.7.x/7289' into 2.7.x

diff --combined lib/puppet/interface.rb
index c7a167d,eb376c4..10e2ec8
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@@ -1,11 -1,7 +1,11 @@@
  require 'puppet'
  require 'puppet/util/autoload'
 +require 'puppet/interface/documentation'
 +require 'prettyprint'
  
  class Puppet::Interface
 +  include FullDocs
 +
    require 'puppet/interface/face_collection'
  
    require 'puppet/interface/action_manager'
@@@ -69,33 -65,27 +69,33 @@@
      Puppet.warning("set_default_format is deprecated (and ineffective); use render_as on your actions instead.")
    end
  
 +
    ########################################################################
    # Documentation.  We currently have to rewrite both getters because we share
    # the same instance between build-time and the runtime instance.  When that
    # splits out this should merge into a module that both the action and face
    # include. --daniel 2011-04-17
 -  attr_accessor :summary, :description
 -  def summary(value = nil)
 -    self.summary = value unless value.nil?
 -    @summary
 -  end
 -  def summary=(value)
 -    value = value.to_s
 -    value =~ /\n/ and
 -      raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
 -
 -    @summary = value
 -  end
 -
 -  def description(value = nil)
 -    self.description = value unless value.nil?
 -    @description
 +  def synopsis
 +    output = PrettyPrint.format do |s|
 +      s.text("puppet #{name} <action>")
 +      s.breakable
 +
 +      options.each do |option|
 +        option = get_option(option)
 +        wrap = option.required? ? %w{ < > } : %w{ [ ] }
 +
 +        s.group(0, *wrap) do
 +          option.optparse.each do |item|
 +            unless s.current_group.first?
 +              s.breakable
 +              s.text '|'
 +              s.breakable
 +            end
 +            s.text item
 +          end
 +        end
 +      end
 +    end
    end
  
  
@@@ -107,15 -97,9 +107,15 @@@
        raise ArgumentError, "Cannot create face #{name.inspect} with invalid version number '#{version}'!"
      end
  
 -    @name = Puppet::Interface::FaceCollection.underscorize(name)
 +    @name    = Puppet::Interface::FaceCollection.underscorize(name)
      @version = version
  
 +    # The few bits of documentation we actually demand.  The default license
 +    # is a favour to our end users; if you happen to get that in a core face
 +    # report it as a bug, please. --daniel 2011-04-26
 +    @authors  = []
 +    @license  = 'All Rights Reserved'
 +
      instance_eval(&block) if block_given?
    end
  
@@@ -155,6 -139,8 +155,8 @@@
        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 --combined lib/puppet/interface/action.rb
index b842c28,96bb5b4..0dbdd57
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@@ -1,24 -1,18 +1,27 @@@
 -# -*- coding: utf-8 -*-
  require 'puppet/interface'
 -require 'puppet/interface/option'
 +require 'puppet/interface/documentation'
 +require 'prettyprint'
  
  class Puppet::Interface::Action
 +  include Puppet::Interface::FullDocs
 +
    def initialize(face, name, attrs = {})
      raise "#{name.inspect} is an invalid action name" unless name.to_s =~ /^[a-z]\w*$/
      @face    = face
      @name    = name.to_sym
 +
 +    # The few bits of documentation we actually demand.  The default license
 +    # is a favour to our end users; if you happen to get that in a core face
 +    # report it as a bug, please. --daniel 2011-04-26
 +    @authors = []
 +    @license  = 'All Rights Reserved'
 +
      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
  
@@@ -39,31 -33,8 +42,31 @@@
      !!@default
    end
  
 -  attr_accessor :summary
 -
 +  ########################################################################
 +  # Documentation...
 +  def synopsis
 +    output = PrettyPrint.format do |s|
 +      s.text("puppet #{@face.name}")
 +      s.text(" #{name}") unless default?
 +      s.breakable
 +
 +      options.each do |option|
 +        option = get_option(option)
 +        wrap = option.required? ? %w{ < > } : %w{ [ ] }
 +
 +        s.group(0, *wrap) do
 +          option.optparse.each do |item|
 +            unless s.current_group.first?
 +              s.breakable
 +              s.text '|'
 +              s.breakable
 +            end
 +            s.text item
 +          end
 +        end
 +      end
 +    end
 +  end
  
    ########################################################################
    # Support for rendering formats and all.
@@@ -115,6 -86,18 +118,6 @@@
  
  
    ########################################################################
 -  # Documentation stuff, whee!
 -  attr_accessor :summary, :description
 -  def summary=(value)
 -    value = value.to_s
 -    value =~ /\n/ and
 -      raise ArgumentError, "Face summary should be a single line; put the long text in 'description' instead."
 -
 -    @summary = value
 -  end
 -
 -
 -  ########################################################################
    # Initially, this was defined to allow the @action.invoke pattern, which is
    # a very natural way to invoke behaviour given our introspection
    # capabilities.   Heck, our initial plan was to have the faces delegate to
@@@ -168,12 -151,11 +171,12 @@@
    # this stuff work, because it would have been cleaner.  Which gives you an
    # idea how motivated we were to make this cleaner.  Sorry.
    # --daniel 2011-03-31
 +  attr_reader :positional_arg_count
    def when_invoked=(block)
  
      internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym
  
 -    arity = block.arity
 +    arity = @positional_arg_count = block.arity
      if arity == 0 then
        # This will never fire on 1.8.7, which treats no arguments as "*args",
        # but will on 1.9.2, which treats it as "no arguments".  Which bites,
@@@ -232,22 -214,23 +235,23 @@@ WRAPPE
      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
@@@ -255,20 -238,6 +259,20 @@@
    end
  
    def validate_args(args)
 +    # Check for multiple aliases for the same option...
 +    args.last.keys.each do |name|
 +      # #7290: If this isn't actually an option, ignore it for now.  We should
 +      # probably fail, but that wasn't our API, and I don't want to perturb
 +      # behaviour this late in the RC cycle. --daniel 2011-04-29
 +      if option = get_option(name) then
 +        overlap = (option.aliases & args.last.keys)
 +        unless overlap.length == 1 then
 +          raise ArgumentError, "Multiple aliases for the same option passed: #{overlap.join(', ')}"
 +        end
 +      end
 +    end
 +
 +    # Check for missing mandatory options.
      required = options.map do |name|
        get_option(name)
      end.select(&:required?).collect(&:name) - args.last.keys
diff --combined spec/shared_behaviours/things_that_declare_options.rb
index 3d33bab,4ffe55d..6e70561
--- a/spec/shared_behaviours/things_that_declare_options.rb
+++ b/spec/shared_behaviours/things_that_declare_options.rb
@@@ -28,8 -28,6 +28,8 @@@ shared_examples_for "things that declar
      thing = add_options_to do
        option "--foo" do
          desc text
 +        description text
 +        summary text
        end
      end
  
@@@ -39,9 -37,12 +39,12 @@@
    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 --combined spec/unit/interface/action_spec.rb
index f6796a8,8d0606d..f8102f4
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@@ -249,180 -249,247 +249,247 @@@ describe Puppet::Interface::Action d
      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
@@@ -445,23 -512,4 +512,23 @@@
      it "should fail if a second block is given for the same type"
      it "should return the block if asked"
    end
 +
 +  context "#validate_args" do
 +    subject do
 +      Puppet::Interface.new(:validate_args, '1.0.0') do
 +        script :test do true end
 +      end
 +    end
 +
 +    it "should fail if a required option is not passed" do
 +      subject.option "--foo" do required end
 +      expect { subject.test }.to raise_error ArgumentError, /options are required/
 +    end
 +
 +    it "should fail if two aliases to one option are passed" do
 +      subject.option "--foo", "-f"
 +      expect { subject.test :foo => true, :f => true }.
 +        to raise_error ArgumentError, /Multiple aliases for the same option/
 +    end
 +  end
  end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list