diff options
author | Max Martin <max@puppetlabs.com> | 2011-04-21 19:04:39 -0700 |
---|---|---|
committer | Max Martin <max@puppetlabs.com> | 2011-04-21 19:04:39 -0700 |
commit | 25593abbb044aca86c71cd60e91665eca0ce1bd1 (patch) | |
tree | d8703468b9b598c196348b748fbe47333b31a745 | |
parent | 01f610bb223b435dc52f491260af3ea002930102 (diff) | |
parent | e4b31b411a4b3d7cce7f45197491eebc36d047aa (diff) | |
download | puppet-25593abbb044aca86c71cd60e91665eca0ce1bd1.tar.gz puppet-25593abbb044aca86c71cd60e91665eca0ce1bd1.tar.xz puppet-25593abbb044aca86c71cd60e91665eca0ce1bd1.zip |
Merge branch 'next'
* next:
(#6928) Don't blow up when the method is undefined...
(#6928) backport Symbol#to_proc for Ruby < 1.8.7
(#7183) Implement "invisible glob" version matching for faces
maint: better disabling of Signal#trap in our tests.
maint: more robust listing of valid faces.
maint: clean up testing code a fraction...
maint: better error report for a missing version of a face.
maint: handle face clear/reset sanely in the interface spec.
maint: stop stubbing log level setting.
Move tests from Puppet-acceptance repo
(#7116) Handle application-level options in parse_options
maint: fix gratuitous whitespace in the code.
maint: remove redundant context from the test.
(#7062) better argument handling in the action wrapper methods
maint: move method comments outside the comment.
Fixed #7166 - Replaced deprecated stomp "send" method with "publish"
maint: Remove unused faces code
26 files changed, 387 insertions, 316 deletions
diff --git a/lib/puppet/application/face_base.rb b/lib/puppet/application/face_base.rb index 9da48af55..7bebd18bb 100644 --- a/lib/puppet/application/face_base.rb +++ b/lib/puppet/application/face_base.rb @@ -92,8 +92,8 @@ class Puppet::Application::FaceBase < Puppet::Application # REVISIT: These should be configurable versions, through a global # '--version' option, but we don't implement that yet... --daniel 2011-03-29 - @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym - @face = Puppet::Face[@type, :current] + @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym + @face = Puppet::Face[@type, :current] # Now, walk the command line and identify the action. We skip over # arguments based on introspecting the action and all, and find the first @@ -122,6 +122,8 @@ class Puppet::Application::FaceBase < Puppet::Application # a mandatory argument. --daniel 2011-04-05 index += 1 # ...so skip the argument. end + elsif option = find_application_argument(item) then + index += 1 if (option[:argument] and option[:optional]) else raise OptionParser::InvalidOption.new(item.sub(/=.*$/, '')) end @@ -158,6 +160,21 @@ class Puppet::Application::FaceBase < Puppet::Application return nil # nothing found. end + def find_application_argument(item) + self.class.option_parser_commands.each do |options, function| + options.each do |option| + next unless option =~ /^-/ + pattern = /^#{option.sub('[no-]', '').sub(/[ =].*$/, '')}(?:[ =].*)?$/ + next unless pattern.match(item) + return { + :argument => option =~ /[ =]/, + :optional => option =~ /[ =]\[/ + } + end + end + return nil # not found + end + def setup Puppet::Util::Log.newdestination :console diff --git a/lib/puppet/faces/help/action.erb b/lib/puppet/faces/help/action.erb deleted file mode 100644 index eaf131464..000000000 --- a/lib/puppet/faces/help/action.erb +++ /dev/null @@ -1,3 +0,0 @@ -Use: puppet <%= face.name %> [options] <%= action.name %> [options] - -Summary: <%= action.summary %> diff --git a/lib/puppet/faces/help/face.erb b/lib/puppet/faces/help/face.erb deleted file mode 100644 index efe5fd809..000000000 --- a/lib/puppet/faces/help/face.erb +++ /dev/null @@ -1,7 +0,0 @@ -Use: puppet <%= face.name %> [options] <action> [options] - -Available actions: -% face.actions.each do |actionname| -% action = face.get_action(actionname) - <%= action.name.to_s.ljust(16) %> <%= action.summary %> -% end diff --git a/lib/puppet/faces/help/global.erb b/lib/puppet/faces/help/global.erb deleted file mode 100644 index e123367a2..000000000 --- a/lib/puppet/faces/help/global.erb +++ /dev/null @@ -1,20 +0,0 @@ -puppet <subcommand> [options] <action> [options] - -Available subcommands, from Puppet Faces: -% Puppet::Faces.faces.sort.each do |name| -% face = Puppet::Faces[name, :current] - <%= face.name.to_s.ljust(16) %> <%= face.summary %> -% end - -% unless legacy_applications.empty? then # great victory when this is true! -Available applications, soon to be ported to Faces: -% legacy_applications.each do |appname| -% summary = horribly_extract_summary_from appname - <%= appname.to_s.ljust(16) %> <%= summary %> -% end -% end - -See 'puppet help <subcommand> <action>' for help on a specific subcommand action. -See 'puppet help <subcommand>' for help on a specific subcommand. -See 'puppet man <subcommand>' for the full man page. -Puppet v<%= Puppet::PUPPETVERSION %> diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb index 5c8ade749..ced00863d 100644 --- a/lib/puppet/interface.rb +++ b/lib/puppet/interface.rb @@ -26,18 +26,13 @@ class Puppet::Interface Puppet::Interface::FaceCollection.faces end - def face?(name, version) - Puppet::Interface::FaceCollection.face?(name, version) - end - def register(instance) Puppet::Interface::FaceCollection.register(instance) end def define(name, version, &block) - if face?(name, version) - face = Puppet::Interface::FaceCollection[name, version] - else + face = Puppet::Interface::FaceCollection[name, version] + if face.nil? then face = self.new(name, version) Puppet::Interface::FaceCollection.register(face) # REVISIT: Shouldn't this be delayed until *after* we evaluate the @@ -50,10 +45,14 @@ class Puppet::Interface return face end + def face?(name, version) + Puppet::Interface::FaceCollection[name, version] + end + def [](name, version) unless face = Puppet::Interface::FaceCollection[name, version] if current = Puppet::Interface::FaceCollection[name, :current] - raise Puppet::Error, "Could not find version #{version} of #{current}" + raise Puppet::Error, "Could not find version #{version} of #{name}" else raise Puppet::Error, "Could not find Puppet Face #{name.inspect}" end diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index 23366b407..08bc0a345 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -129,38 +129,59 @@ class Puppet::Interface::Action # @face.send(name, *args, &block) # end + + # We need to build an instance method as a wrapper, using normal code, to be + # able to expose argument defaulting between the caller and definer in the + # Ruby API. An extra method is, sadly, required for Ruby 1.8 to work since + # it doesn't expose bind on a block. + # + # Hopefully we can improve this when we finally shuffle off the last of Ruby + # 1.8 support, but that looks to be a few "enterprise" release eras away, so + # we are pretty stuck with this for now. + # + # Patches to make this work more nicely with Ruby 1.9 using runtime version + # checking and all are welcome, provided that they don't change anything + # outside this little ol' bit of code and all. + # + # Incidentally, we though about vendoring evil-ruby and actually adjusting + # the internal C structure implementation details under the hood to make + # 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 def when_invoked=(block) - # We need to build an instance method as a wrapper, using normal code, to - # be able to expose argument defaulting between the caller and definer in - # the Ruby API. An extra method is, sadly, required for Ruby 1.8 to work. - # - # In future this also gives us a place to hook in additional behaviour - # such as calling out to the action instance to validate and coerce - # parameters, which avoids any exciting context switching and all. - # - # Hopefully we can improve this when we finally shuffle off the last of - # Ruby 1.8 support, but that looks to be a few "enterprise" release eras - # away, so we are pretty stuck with this for now. - # - # Patches to make this work more nicely with Ruby 1.9 using runtime - # version checking and all are welcome, but they can't actually help if - # the results are not totally hidden away in here. - # - # Incidentally, we though about vendoring evil-ruby and actually adjusting - # the internal C structure implementation details under the hood to make - # 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 internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym - file = __FILE__ + "+eval" - line = __LINE__ + 1 + + arity = 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, + # because this just begs for us to wind up in the horrible situation + # where a 1.8 vs 1.9 error bites our end users. --daniel 2011-04-19 + raise ArgumentError, "action when_invoked requires at least one argument (options)" + elsif arity > 0 then + range = Range.new(1, arity - 1) + decl = range.map { |x| "arg#{x}" } << "options = {}" + optn = "" + args = "[" + (range.map { |x| "arg#{x}" } << "options").join(", ") + "]" + else + range = Range.new(1, arity.abs - 1) + decl = range.map { |x| "arg#{x}" } << "*rest" + optn = "rest << {} unless rest.last.is_a?(Hash)" + if arity == -1 then + args = "rest" + else + args = "[" + range.map { |x| "arg#{x}" }.join(", ") + "] + rest" + end + end + + file = __FILE__ + "+eval[wrapper]" + line = __LINE__ + 2 # <== points to the same line as 'def' in the wrapper. wrapper = <<WRAPPER -def #{@name}(*args) - if args.last.is_a? Hash then - options = args.last - else - args << (options = {}) - end +def #{@name}(#{decl.join(", ")}) + #{optn} + args = #{args} + options = args.last action = get_action(#{name.inspect}) action.validate_args(args) diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 591471d4b..6e6afc545 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -24,19 +24,21 @@ module Puppet::Interface::FaceCollection end end end - return @faces.keys + return @faces.keys.select {|name| @faces[name].length > 0 } end def self.validate_version(version) !!(SEMVER_VERSION =~ version.to_s) end + def self.semver_to_array(v) + parts = SEMVER_VERSION.match(v).to_a[1..4] + parts[0..2] = parts[0..2].map { |e| e.to_i } + parts + end + def self.cmp_semver(a, b) - a, b = [a, b].map do |x| - parts = SEMVER_VERSION.match(x).to_a[1..4] - parts[0..2] = parts[0..2].map { |e| e.to_i } - parts - end + a, b = [a, b].map do |x| semver_to_array(x) end cmp = a[0..2] <=> b[0..2] if cmp == 0 @@ -47,18 +49,38 @@ module Puppet::Interface::FaceCollection cmp end - def self.[](name, version) - @faces[underscorize(name)][version] if face?(name, version) + def self.prefix_match?(desired, target) + # Can't meaningfully do a prefix match with current on either side. + return false if desired == :current + return false if target == :current + + # REVISIT: Should probably fail if the matcher is not valid. + prefix = desired.split('.').map {|x| x =~ /^\d+$/ and x.to_i } + have = semver_to_array(target) + + while want = prefix.shift do + return false unless want == have.shift + end + return true end - def self.face?(name, version) + def self.[](name, version) name = underscorize(name) + get_face(name, version) or load_face(name, version) + end - # Note: be careful not to accidentally create the top level key, either, - # because it will result in confusion when people try to enumerate the - # list of valid faces later. --daniel 2011-04-11 - return true if @faces.has_key?(name) and @faces[name].has_key?(version) + # get face from memory, without loading. + def self.get_face(name, desired_version) + return nil unless @faces.has_key? name + return @faces[name][:current] if desired_version == :current + + found = @faces[name].keys.select {|v| prefix_match?(desired_version, v) }.sort.last + return @faces[name][found] + end + + # try to load the face, and return it. + def self.load_face(name, version) # We always load the current version file; the common case is that we have # the expected version and any compatibility versions in the same file, # the default. Which means that this is almost always the case. @@ -104,17 +126,7 @@ module Puppet::Interface::FaceCollection # ...guess we didn't find the file; return a much better problem. end - # Now, either we have the version in our set of faces, or we didn't find - # the version they were looking for. In the future we will support - # loading versioned stuff from some look-aside part of the Ruby load path, - # but we don't need that right now. - # - # So, this comment is a place-holder for that. --daniel 2011-04-06 - # - # Note: be careful not to accidentally create the top level key, either, - # because it will result in confusion when people try to enumerate the - # list of valid faces later. --daniel 2011-04-11 - return !! (@faces.has_key?(name) and @faces[name].has_key?(version)) + return get_face(name, version) end def self.register(face) diff --git a/lib/puppet/util/monkey_patches.rb b/lib/puppet/util/monkey_patches.rb index 10a268409..bd954c665 100644 --- a/lib/puppet/util/monkey_patches.rb +++ b/lib/puppet/util/monkey_patches.rb @@ -104,3 +104,10 @@ class Array end end unless method_defined? :combination end + + +class Symbol + def to_proc + Proc.new { |*args| args.shift.__send__(self, *args) } + end unless method_defined? :to_proc +end diff --git a/spec/lib/puppet/face/version_matching.rb b/spec/lib/puppet/face/version_matching.rb new file mode 100644 index 000000000..bfd0013f7 --- /dev/null +++ b/spec/lib/puppet/face/version_matching.rb @@ -0,0 +1,10 @@ +require 'puppet/face' + +# The set of versions here are used explicitly in the interface_spec; if you +# change this you need to ensure that is still correct. --daniel 2011-04-21 +['1.0.0', '1.0.1', '1.1.0', '1.1.1', '2.0.0'].each do |version| + Puppet::Face.define(:version_matching, version) do + summary "version matching face #{version}" + script :version do version end + end +end diff --git a/spec/monkey_patches/disable_signal_trap.rb b/spec/monkey_patches/disable_signal_trap.rb new file mode 100644 index 000000000..5159626e3 --- /dev/null +++ b/spec/monkey_patches/disable_signal_trap.rb @@ -0,0 +1,5 @@ +module Signal + def trap(*args) + # The goggles, they do nothing! + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1187c1caf..01ffabc48 100755 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -22,6 +22,7 @@ require 'lib/puppet_spec/files' require 'lib/puppet_spec/fixtures' require 'monkey_patches/alias_should_to_must' require 'monkey_patches/publicize_methods' +require 'monkey_patches/disable_signal_trap' Pathname.glob("#{dir}/shared_behaviours/**/*.rb") do |behaviour| require behaviour.relative_path_from(Pathname.new(dir)) @@ -38,6 +39,10 @@ RSpec.configure do |config| # these globals are set by Application $puppet_application_mode = nil $puppet_application_name = nil + + # REVISIT: I think this conceals other bad tests, but I don't have time to + # fully diagnose those right now. When you read this, please come tell me + # I suck for letting this float. --daniel 2011-04-21 Signal.stubs(:trap) # Set the confdir and vardir to gibberish so that tests @@ -50,6 +55,8 @@ RSpec.configure do |config| @logs = [] Puppet::Util::Log.newdestination(Puppet::Test::LogCollector.new(@logs)) + + @log_level = Puppet::Util::Log.level end config.after :each do @@ -62,6 +69,7 @@ RSpec.configure do |config| @logs.clear Puppet::Util::Log.close_all + Puppet::Util::Log.level = @log_level GC.enable end diff --git a/spec/unit/application/agent_spec.rb b/spec/unit/application/agent_spec.rb index 03cf14429..b30a8cc6c 100755 --- a/spec/unit/application/agent_spec.rb +++ b/spec/unit/application/agent_spec.rb @@ -18,7 +18,6 @@ describe Puppet::Application::Agent do Puppet::Agent.stubs(:new).returns(@agent) @puppetd.preinit Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) Puppet::Node.indirection.stubs(:terminus_class=) Puppet::Node.indirection.stubs(:cache_class=) @@ -223,18 +222,14 @@ describe Puppet::Application::Agent do it "should set log level to debug if --debug was passed" do @puppetd.options.stubs(:[]).with(:debug).returns(true) - - Puppet::Util::Log.expects(:level=).with(:debug) - @puppetd.setup_logs + Puppet::Util::Log.level.should == :debug end it "should set log level to info if --verbose was passed" do @puppetd.options.stubs(:[]).with(:verbose).returns(true) - - Puppet::Util::Log.expects(:level=).with(:info) - @puppetd.setup_logs + Puppet::Util::Log.level.should == :info end [:verbose, :debug].each do |level| diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb index dca2a4156..ec3f083db 100755 --- a/spec/unit/application/apply_spec.rb +++ b/spec/unit/application/apply_spec.rb @@ -9,7 +9,6 @@ describe Puppet::Application::Apply do before :each do @apply = Puppet::Application[:apply] Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) end [:debug,:loadclasses,:verbose,:use_nodes,:detailed_exitcodes].each do |option| @@ -51,7 +50,6 @@ describe Puppet::Application::Apply do before :each do Puppet::Log.stubs(:newdestination) - Puppet::Log.stubs(:level=) Puppet.stubs(:parse_config) Puppet::FileBucket::Dipper.stubs(:new) STDIN.stubs(:read) @@ -84,18 +82,14 @@ describe Puppet::Application::Apply do it "should set log level to debug if --debug was passed" do @apply.options.stubs(:[]).with(:debug).returns(true) - - Puppet::Log.expects(:level=).with(:debug) - @apply.setup + Puppet::Log.level.should == :debug end it "should set log level to info if --verbose was passed" do @apply.options.stubs(:[]).with(:verbose).returns(true) - - Puppet::Log.expects(:level=).with(:info) - @apply.setup + Puppet::Log.level.should == :info end it "should print puppet config if asked to in Puppet config" do diff --git a/spec/unit/application/cert_spec.rb b/spec/unit/application/cert_spec.rb index a1b5eb19a..4a91c1e6c 100755 --- a/spec/unit/application/cert_spec.rb +++ b/spec/unit/application/cert_spec.rb @@ -6,7 +6,6 @@ describe Puppet::Application::Cert do before :each do @cert_app = Puppet::Application[:cert] Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) end it "should operate in master run_mode" do @@ -28,22 +27,17 @@ describe Puppet::Application::Cert do end it "should set log level to info with the --verbose option" do - - Puppet::Log.expects(:level=).with(:info) - @cert_app.handle_verbose(0) + Puppet::Log.level.should == :info end it "should set log level to debug with the --debug option" do - - Puppet::Log.expects(:level=).with(:debug) - @cert_app.handle_debug(0) + Puppet::Log.level.should == :debug end it "should set the fingerprint digest with the --digest option" do @cert_app.handle_digest(:digest) - @cert_app.digest.should == :digest end diff --git a/spec/unit/application/device_spec.rb b/spec/unit/application/device_spec.rb index 086e321e9..832b7e55b 100644..100755 --- a/spec/unit/application/device_spec.rb +++ b/spec/unit/application/device_spec.rb @@ -10,10 +10,8 @@ require 'puppet/configurer' describe Puppet::Application::Device do before :each do @device = Puppet::Application[:device] -# @device.stubs(:puts) @device.preinit Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) Puppet::Node.indirection.stubs(:terminus_class=) Puppet::Node.indirection.stubs(:cache_class=) @@ -144,18 +142,14 @@ describe Puppet::Application::Device do it "should set log level to debug if --debug was passed" do @device.options.stubs(:[]).with(:debug).returns(true) - - Puppet::Util::Log.expects(:level=).with(:debug) - @device.setup_logs + Puppet::Util::Log.level.should == :debug end it "should set log level to info if --verbose was passed" do @device.options.stubs(:[]).with(:verbose).returns(true) - - Puppet::Util::Log.expects(:level=).with(:info) - @device.setup_logs + Puppet::Util::Log.level.should == :info end [:verbose, :debug].each do |level| diff --git a/spec/unit/application/doc_spec.rb b/spec/unit/application/doc_spec.rb index 66a833b9d..43a4b9849 100755 --- a/spec/unit/application/doc_spec.rb +++ b/spec/unit/application/doc_spec.rb @@ -11,7 +11,6 @@ describe Puppet::Application::Doc do @doc.stubs(:puts) @doc.preinit Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) end it "should ask Puppet::Application to not parse Puppet configuration file" do @@ -186,7 +185,6 @@ describe Puppet::Application::Doc do before :each do @doc.options.stubs(:[]).returns(false) Puppet.stubs(:parse_config) - Puppet::Util::Log.stubs(:level=) Puppet::Util::Log.stubs(:newdestination) end @@ -232,16 +230,14 @@ describe Puppet::Application::Doc do it "should set log level to debug if --debug" do @doc.options.stubs(:[]).with(:debug).returns(true) - Puppet::Util::Log.expects(:level=).with(:debug) - @doc.setup_rdoc + Puppet::Util::Log.level.should == :debug end it "should set log level to info if --verbose" do @doc.options.stubs(:[]).with(:verbose).returns(true) - Puppet::Util::Log.expects(:level=).with(:info) - @doc.setup_rdoc + Puppet::Util::Log.level.should == :info end it "should set log destination to console if --verbose" do diff --git a/spec/unit/application/face_base_spec.rb b/spec/unit/application/face_base_spec.rb index 5403608cf..f7c55c556 100755 --- a/spec/unit/application/face_base_spec.rb +++ b/spec/unit/application/face_base_spec.rb @@ -40,127 +40,132 @@ describe Puppet::Application::FaceBase do app.command_line.stubs(:args).returns %w{} end - describe "parsing the command line" do - context "with just an action" do - before :all do - # We have to stub Signal.trap to avoid a crazy mess where we take - # over signal handling and make it impossible to cancel the test - # suite run. - # - # It would be nice to fix this elsewhere, but it is actually hard to - # capture this in rspec 2.5 and all. :( --daniel 2011-04-08 - Signal.stubs(:trap) - app.command_line.stubs(:args).returns %w{foo} - app.preinit - app.parse_options - end - - it "should set the face based on the type" do - app.face.name.should == :basetest - end - - it "should find the action" do - app.action.should be - app.action.name.should == :foo - end + describe "with just an action" do + before :all do + # We have to stub Signal.trap to avoid a crazy mess where we take + # over signal handling and make it impossible to cancel the test + # suite run. + # + # It would be nice to fix this elsewhere, but it is actually hard to + # capture this in rspec 2.5 and all. :( --daniel 2011-04-08 + Signal.stubs(:trap) + app.command_line.stubs(:args).returns %w{foo} + app.preinit + app.parse_options end - it "should use the default action if not given any arguments" do - app.command_line.stubs(:args).returns [] - action = stub(:options => []) - Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action) - app.stubs(:main) - app.run - app.action.should == action - app.arguments.should == [ { } ] + it "should set the face based on the type" do + app.face.name.should == :basetest end - it "should use the default action if not given a valid one" do - app.command_line.stubs(:args).returns %w{bar} - action = stub(:options => []) - Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action) - app.stubs(:main) - app.run - app.action.should == action - app.arguments.should == [ 'bar', { } ] + it "should find the action" do + app.action.should be + app.action.name.should == :foo end + end - it "should have no action if not given a valid one and there is no default action" do - app.command_line.stubs(:args).returns %w{bar} - Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil) - app.stubs(:main) - app.run - app.action.should be_nil - app.arguments.should == [ 'bar', { } ] - end + it "should use the default action if not given any arguments" do + app.command_line.stubs(:args).returns [] + action = stub(:options => []) + Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action) + app.stubs(:main) + app.run + app.action.should == action + app.arguments.should == [ { } ] + end - it "should report a sensible error when options with = fail" do - app.command_line.stubs(:args).returns %w{--action=bar foo} - expect { app.preinit; app.parse_options }. - to raise_error OptionParser::InvalidOption, /invalid option: --action/ - end + it "should use the default action if not given a valid one" do + app.command_line.stubs(:args).returns %w{bar} + action = stub(:options => []) + Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(action) + app.stubs(:main) + app.run + app.action.should == action + app.arguments.should == [ 'bar', { } ] + end - it "should fail if an action option is before the action" do - app.command_line.stubs(:args).returns %w{--action foo} - expect { app.preinit; app.parse_options }. - to raise_error OptionParser::InvalidOption, /invalid option: --action/ - end + it "should have no action if not given a valid one and there is no default action" do + app.command_line.stubs(:args).returns %w{bar} + Puppet::Face[:basetest, '0.0.1'].expects(:get_default_action).returns(nil) + app.stubs(:main) + app.run + app.action.should be_nil + app.arguments.should == [ 'bar', { } ] + end - it "should fail if an unknown option is before the action" do - app.command_line.stubs(:args).returns %w{--bar foo} - expect { app.preinit; app.parse_options }. - to raise_error OptionParser::InvalidOption, /invalid option: --bar/ - end + it "should report a sensible error when options with = fail" do + app.command_line.stubs(:args).returns %w{--action=bar foo} + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --action/ + end - it "should fail if an unknown option is after the action" do - app.command_line.stubs(:args).returns %w{foo --bar} - expect { app.preinit; app.parse_options }. - to raise_error OptionParser::InvalidOption, /invalid option: --bar/ - end + it "should fail if an action option is before the action" do + app.command_line.stubs(:args).returns %w{--action foo} + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --action/ + end + + it "should fail if an unknown option is before the action" do + app.command_line.stubs(:args).returns %w{--bar foo} + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --bar/ + end + + it "should fail if an unknown option is after the action" do + app.command_line.stubs(:args).returns %w{foo --bar} + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --bar/ + end + + it "should accept --bar as an argument to a mandatory option after action" do + app.command_line.stubs(:args).returns %w{foo --mandatory --bar} + app.preinit + app.parse_options + app.action.name.should == :foo + app.options.should == { :mandatory => "--bar" } + end + + it "should accept --bar as an argument to a mandatory option before action" do + app.command_line.stubs(:args).returns %w{--mandatory --bar foo} + app.preinit + app.parse_options + app.action.name.should == :foo + app.options.should == { :mandatory => "--bar" } + end - it "should accept --bar as an argument to a mandatory option after action" do - app.command_line.stubs(:args).returns %w{foo --mandatory --bar} + it "should not skip when --foo=bar is given" do + app.command_line.stubs(:args).returns %w{--mandatory=bar --bar foo} + expect { app.preinit; app.parse_options }. + to raise_error OptionParser::InvalidOption, /invalid option: --bar/ + end + + { "boolean options before" => %w{--trace foo}, + "boolean options after" => %w{foo --trace} + }.each do |name, args| + it "should accept global boolean settings #{name} the action" do + app.command_line.stubs(:args).returns args app.preinit app.parse_options - app.action.name.should == :foo - app.options.should == { :mandatory => "--bar" } + Puppet[:trace].should be_true end + end - it "should accept --bar as an argument to a mandatory option before action" do - app.command_line.stubs(:args).returns %w{--mandatory --bar foo} + { "before" => %w{--syslogfacility user1 foo}, + " after" => %w{foo --syslogfacility user1} + }.each do |name, args| + it "should accept global settings with arguments #{name} the action" do + app.command_line.stubs(:args).returns args app.preinit app.parse_options - app.action.name.should == :foo - app.options.should == { :mandatory => "--bar" } - end - - it "should not skip when --foo=bar is given" do - app.command_line.stubs(:args).returns %w{--mandatory=bar --bar foo} - expect { app.preinit; app.parse_options }. - to raise_error OptionParser::InvalidOption, /invalid option: --bar/ - end - - { "boolean options before" => %w{--trace foo}, - "boolean options after" => %w{foo --trace} - }.each do |name, args| - it "should accept global boolean settings #{name} the action" do - app.command_line.stubs(:args).returns args - app.preinit - app.parse_options - Puppet[:trace].should be_true - end + Puppet[:syslogfacility].should == "user1" end + end - { "before" => %w{--syslogfacility user1 foo}, - " after" => %w{foo --syslogfacility user1} - }.each do |name, args| - it "should accept global settings with arguments #{name} the action" do - app.command_line.stubs(:args).returns args - app.preinit - app.parse_options - Puppet[:syslogfacility].should == "user1" - end - end + it "should handle application-level options" do + app.command_line.stubs(:args).returns %w{help --verbose help} + app.preinit + app.parse_options + app.face.name.should == :basetest end end diff --git a/spec/unit/application/filebucket_spec.rb b/spec/unit/application/filebucket_spec.rb index 8ba86be9e..92bc0410a 100755 --- a/spec/unit/application/filebucket_spec.rb +++ b/spec/unit/application/filebucket_spec.rb @@ -41,7 +41,6 @@ describe Puppet::Application::Filebucket do before :each do Puppet::Log.stubs(:newdestination) Puppet.stubs(:settraps) - Puppet::Log.stubs(:level=) Puppet.stubs(:parse_config) Puppet::FileBucket::Dipper.stubs(:new) @filebucket.options.stubs(:[]).with(any_parameters) @@ -62,18 +61,14 @@ describe Puppet::Application::Filebucket do it "should set log level to debug if --debug was passed" do @filebucket.options.stubs(:[]).with(:debug).returns(true) - - Puppet::Log.expects(:level=).with(:debug) - @filebucket.setup + Puppet::Log.level.should == :debug end it "should set log level to info if --verbose was passed" do @filebucket.options.stubs(:[]).with(:verbose).returns(true) - - Puppet::Log.expects(:level=).with(:info) - @filebucket.setup + Puppet::Log.level.should == :info end it "should Parse puppet config" do @@ -140,7 +135,6 @@ describe Puppet::Application::Filebucket do before :each do Puppet::Log.stubs(:newdestination) Puppet.stubs(:settraps) - Puppet::Log.stubs(:level=) Puppet.stubs(:parse_config) Puppet::FileBucket::Dipper.stubs(:new) @filebucket.options.stubs(:[]).with(any_parameters) diff --git a/spec/unit/application/kick_spec.rb b/spec/unit/application/kick_spec.rb index 29e4caea4..742c0fff6 100755 --- a/spec/unit/application/kick_spec.rb +++ b/spec/unit/application/kick_spec.rb @@ -10,7 +10,6 @@ describe Puppet::Application::Kick, :if => Puppet.features.posix? do Puppet::Util::Ldap::Connection.stubs(:new).returns(stub_everything) @kick = Puppet::Application[:kick] Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) end describe ".new" do @@ -120,7 +119,6 @@ describe Puppet::Application::Kick, :if => Puppet.features.posix? do @kick.classes = [] @kick.tags = [] @kick.hosts = [] - Puppet::Log.stubs(:level=) @kick.stubs(:trap) @kick.stubs(:puts) Puppet.stubs(:parse_config) @@ -130,18 +128,14 @@ describe Puppet::Application::Kick, :if => Puppet.features.posix? do it "should set log level to debug if --debug was passed" do @kick.options.stubs(:[]).with(:debug).returns(true) - - Puppet::Log.expects(:level=).with(:debug) - @kick.setup + Puppet::Log.level.should == :debug end it "should set log level to info if --verbose was passed" do @kick.options.stubs(:[]).with(:verbose).returns(true) - - Puppet::Log.expects(:level=).with(:info) - @kick.setup + Puppet::Log.level.should == :info end it "should Parse puppet config" do diff --git a/spec/unit/application/master_spec.rb b/spec/unit/application/master_spec.rb index ea5d3f518..2a24086f3 100755 --- a/spec/unit/application/master_spec.rb +++ b/spec/unit/application/master_spec.rb @@ -11,7 +11,6 @@ describe Puppet::Application::Master do @daemon = stub_everything 'daemon' Puppet::Daemon.stubs(:new).returns(@daemon) Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) Puppet::Node.indirection.stubs(:terminus_class=) Puppet::Node.indirection.stubs(:cache_class=) @@ -111,7 +110,6 @@ describe Puppet::Application::Master do before :each do Puppet::Log.stubs(:newdestination) Puppet.stubs(:settraps) - Puppet::Log.stubs(:level=) Puppet::SSL::CertificateAuthority.stubs(:instance) Puppet::SSL::CertificateAuthority.stubs(:ca?) Puppet.settings.stubs(:use) @@ -121,18 +119,14 @@ describe Puppet::Application::Master do it "should set log level to debug if --debug was passed" do @master.options.stubs(:[]).with(:debug).returns(true) - - Puppet::Log.expects(:level=).with(:debug) - @master.setup + Puppet::Log.level.should == :debug end it "should set log level to info if --verbose was passed" do @master.options.stubs(:[]).with(:verbose).returns(true) - - Puppet::Log.expects(:level=).with(:info) - @master.setup + Puppet::Log.level.should == :info end it "should set console as the log destination if no --logdest and --daemonize" do diff --git a/spec/unit/application/queue_spec.rb b/spec/unit/application/queue_spec.rb index 87713ca97..d71c879a9 100755 --- a/spec/unit/application/queue_spec.rb +++ b/spec/unit/application/queue_spec.rb @@ -10,7 +10,6 @@ describe Puppet::Application::Queue do @queue.stubs(:puts) @daemon = stub_everything 'daemon', :daemonize => nil Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) Puppet::Resource::Catalog.indirection.stubs(:terminus_class=) end @@ -113,18 +112,14 @@ describe Puppet::Application::Queue do it "should set log level to debug if --debug was passed" do @queue.options.stubs(:[]).with(:debug).returns(true) - - Puppet::Util::Log.expects(:level=).with(:debug) - @queue.setup_logs + Puppet::Util::Log.level.should == :debug end it "should set log level to info if --verbose was passed" do @queue.options.stubs(:[]).with(:verbose).returns(true) - - Puppet::Util::Log.expects(:level=).with(:info) - @queue.setup_logs + Puppet::Util::Log.level.should == :info end [:verbose, :debug].each do |level| diff --git a/spec/unit/application/resource_spec.rb b/spec/unit/application/resource_spec.rb index 673bd65d7..af60f12c1 100755 --- a/spec/unit/application/resource_spec.rb +++ b/spec/unit/application/resource_spec.rb @@ -7,7 +7,6 @@ describe Puppet::Application::Resource do before :each do @resource = Puppet::Application[:resource] Puppet::Util::Log.stubs(:newdestination) - Puppet::Util::Log.stubs(:level=) Puppet::Resource.indirection.stubs(:terminus_class=) end @@ -95,7 +94,6 @@ describe Puppet::Application::Resource do describe "during setup" do before :each do Puppet::Log.stubs(:newdestination) - Puppet::Log.stubs(:level=) Puppet.stubs(:parse_config) end @@ -108,19 +106,15 @@ describe Puppet::Application::Resource do it "should set log level to debug if --debug was passed" do @resource.options.stubs(:[]).with(:debug).returns(true) - - Puppet::Log.expects(:level=).with(:debug) - @resource.setup + Puppet::Log.level.should == :debug end it "should set log level to info if --verbose was passed" do @resource.options.stubs(:[]).with(:debug).returns(false) @resource.options.stubs(:[]).with(:verbose).returns(true) - - Puppet::Log.expects(:level=).with(:info) - @resource.setup + Puppet::Log.level.should == :info end it "should Parse puppet config" do diff --git a/spec/unit/application_spec.rb b/spec/unit/application_spec.rb index f46959092..de1ca1257 100755 --- a/spec/unit/application_spec.rb +++ b/spec/unit/application_spec.rb @@ -369,10 +369,8 @@ describe Puppet::Application do it "should honor option #{level}" do @app.options.stubs(:[]).with(level).returns(true) Puppet::Util::Log.stubs(:newdestination) - - Puppet::Util::Log.expects(:level=).with(level == :verbose ? :info : :debug) - @app.setup + Puppet::Util::Log.level.should == (level == :verbose ? :info : :debug) end end diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb index 0eb450ee2..24826a6ef 100755 --- a/spec/unit/interface/action_spec.rb +++ b/spec/unit/interface/action_spec.rb @@ -12,6 +12,62 @@ describe Puppet::Interface::Action do end end + describe "#when_invoked=" do + it "should fail if the block has arity 0" do + pending "Ruby 1.8 (painfully) treats argument-free blocks as arity -1" if + RUBY_VERSION =~ /^1\.8/ + + expect { + Puppet::Interface.new(:action_when_invoked, '1.0.0') do + action :foo do + when_invoked do + end + end + end + }.to raise_error ArgumentError, /foobra/ + end + + it "should work with arity 1 blocks" do + face = Puppet::Interface.new(:action_when_invoked, '1.0.0') do + action :foo do + when_invoked {|one| } + end + end + # -1, because we use option defaulting. :( + face.method(:foo).arity.should == -1 + end + + it "should work with arity 2 blocks" do + face = Puppet::Interface.new(:action_when_invoked, '1.0.0') do + action :foo do + when_invoked {|one, two| } + end + end + # -2, because we use option defaulting. :( + face.method(:foo).arity.should == -2 + end + + it "should work with arity 1 blocks that collect arguments" do + face = Puppet::Interface.new(:action_when_invoked, '1.0.0') do + action :foo do + when_invoked {|*one| } + end + end + # -1, because we use only varargs + face.method(:foo).arity.should == -1 + end + + it "should work with arity 2 blocks that collect arguments" do + face = Puppet::Interface.new(:action_when_invoked, '1.0.0') do + action :foo do + when_invoked {|one, *two| } + end + end + # -2, because we take one mandatory argument, and one varargs + face.method(:foo).arity.should == -2 + end + end + describe "when invoking" do it "should be able to call other actions on the same object" do face = Puppet::Interface.new(:my_face, '0.0.1') do diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb index f9498cbb8..890e06a9e 100755 --- a/spec/unit/interface/face_collection_spec.rb +++ b/spec/unit/interface/face_collection_spec.rb @@ -24,29 +24,33 @@ describe Puppet::Interface::FaceCollection do $".clear ; @original_required.each do |item| $" << item end end - describe "::validate_version" do - it 'should permit three number versions' do - subject.validate_version('10.10.10').should == true - end - - it 'should permit versions with appended descriptions' do - subject.validate_version('10.10.10beta').should == true - end - - it 'should not permit versions with more than three numbers' do - subject.validate_version('1.2.3.4').should == false - end - - it 'should not permit versions with only two numbers' do - subject.validate_version('10.10').should == false - end - - it 'should not permit versions with only one number' do - subject.validate_version('123').should == false + describe "::prefix_match?" do + # want have + { ['1.0.0', '1.0.0'] => true, + ['1.0', '1.0.0'] => true, + ['1', '1.0.0'] => true, + ['1.0.0', '1.1.0'] => false, + ['1.0', '1.1.0'] => false, + ['1', '1.1.0'] => true, + ['1.0.1', '1.0.0'] => false, + }.each do |data, result| + it "should return #{result.inspect} for prefix_match?(#{data.join(', ')})" do + subject.prefix_match?(*data).should == result + end end + end - it 'should not permit versions with text in any position but at the end' do - subject.validate_version('v1.1.1').should == false + describe "::validate_version" do + { '10.10.10' => true, + '1.2.3.4' => false, + '10.10.10beta' => true, + '10.10' => false, + '123' => false, + 'v1.1.1' => false, + }.each do |input, result| + it "should#{result ? '' : ' not'} permit #{input.inspect}" do + subject.validate_version(input).should(result ? be_true : be_false) + end end end @@ -68,12 +72,10 @@ describe Puppet::Interface::FaceCollection do subject.expects(:require).with('puppet/face/fozzie') subject['fozzie', :current] end - end - describe "::face?" do it "should return true if the face specified is registered" do subject.instance_variable_get("@faces")[:foo]['0.0.1'] = 10 - subject.face?("foo", '0.0.1').should == true + subject["foo", '0.0.1'].should == 10 end it "should attempt to require the face if it is not registered" do @@ -81,24 +83,18 @@ describe Puppet::Interface::FaceCollection do subject.instance_variable_get("@faces")[:bar]['0.0.1'] = true file == 'puppet/face/bar' end - subject.face?("bar", '0.0.1').should == true - end - - it "should return true if requiring the face registered it" do - subject.stubs(:require).with do - subject.instance_variable_get("@faces")[:bar]['0.0.1'] = 20 - end + subject["bar", '0.0.1'].should be_true end it "should return false if the face is not registered" do subject.stubs(:require).returns(true) - subject.face?("bar", '0.0.1').should be_false + subject["bar", '0.0.1'].should be_false end it "should return false if the face file itself is missing" do subject.stubs(:require). raises(LoadError, 'no such file to load -- puppet/face/bar') - subject.face?("bar", '0.0.1').should be_false + subject["bar", '0.0.1'].should be_false end it "should register the version loaded by `:current` as `:current`" do @@ -106,26 +102,26 @@ describe Puppet::Interface::FaceCollection do subject.instance_variable_get("@faces")[:huzzah]['2.0.1'] = :huzzah_face file == 'puppet/face/huzzah' end - subject.face?("huzzah", :current) + subject["huzzah", :current] subject.instance_variable_get("@faces")[:huzzah][:current].should == :huzzah_face end context "with something on disk" do it "should register the version loaded from `puppet/face/{name}` as `:current`" do - subject.should be_face "huzzah", '2.0.1' - subject.should be_face "huzzah", :current + subject["huzzah", '2.0.1'].should be + subject["huzzah", :current].should be Puppet::Face[:huzzah, '2.0.1'].should == Puppet::Face[:huzzah, :current] end it "should index :current when the code was pre-required" do subject.instance_variable_get("@faces")[:huzzah].should_not be_key :current require 'puppet/face/huzzah' - subject.face?(:huzzah, :current).should be_true + subject[:huzzah, :current].should be_true end end it "should not cause an invalid face to be enumerated later" do - subject.face?(:there_is_no_face, :current).should be_false + subject[:there_is_no_face, :current].should be_false subject.faces.should_not include :there_is_no_face end end diff --git a/spec/unit/interface_spec.rb b/spec/unit/interface_spec.rb index b4fef0307..a1d70cf64 100755 --- a/spec/unit/interface_spec.rb +++ b/spec/unit/interface_spec.rb @@ -5,16 +5,17 @@ require 'puppet/interface' describe Puppet::Interface do subject { Puppet::Interface } - before :all do - @faces = Puppet::Interface::FaceCollection.instance_variable_get("@faces").dup - end - before :each do + @faces = Puppet::Interface::FaceCollection. + instance_variable_get("@faces").dup + @dq = $".dup + $".delete_if do |path| path =~ %r{/face/.*\.rb$} end Puppet::Interface::FaceCollection.instance_variable_get("@faces").clear end - after :all do + after :each do Puppet::Interface::FaceCollection.instance_variable_set("@faces", @faces) + $".clear ; @dq.each do |item| $" << item end end describe "#[]" do @@ -29,6 +30,28 @@ describe Puppet::Interface do it "should raise an exception when the requested face doesn't exist" do expect { subject[:burrble_toot, :current] }.should raise_error, Puppet::Error end + + describe "version matching" do + { '1' => '1.1.1', + '1.0' => '1.0.1', + '1.0.1' => '1.0.1', + '1.1' => '1.1.1', + '1.1.1' => '1.1.1' + }.each do |input, expect| + it "should match #{input.inspect} to #{expect.inspect}" do + face = subject[:version_matching, input] + face.should be + face.version.should == expect + end + end + + %w{1.0.2 1.2}.each do |input| + it "should not match #{input.inspect} to any version" do + expect { subject[:version_matching, input] }. + to raise_error Puppet::Error, /Could not find version/ + end + end + end end describe "#define" do |