From 37225200eb39af65dc9eeabb7ae3e6e7571fa602 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Sat, 7 May 2011 17:43:58 +0200 Subject: Fix #7299 - do not require net/ssh for running rake spec This is a different fix than the one proposed by Stefan Schulte, based on Luke comments. Signed-off-by: Brice Figureau --- lib/puppet/util/network_device/transport/ssh.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/puppet/util/network_device/transport/ssh.rb b/lib/puppet/util/network_device/transport/ssh.rb index bf0e7193c..3d7976543 100644 --- a/lib/puppet/util/network_device/transport/ssh.rb +++ b/lib/puppet/util/network_device/transport/ssh.rb @@ -2,7 +2,6 @@ require 'puppet/util/network_device' require 'puppet/util/network_device/transport' require 'puppet/util/network_device/transport/base' -require 'net/ssh' # This is an adaptation/simplification of gem net-ssh-telnet, which aims to have # a sane interface to Net::SSH. Credits goes to net-ssh-telnet authors @@ -12,6 +11,9 @@ class Puppet::Util::NetworkDevice::Transport::Ssh < Puppet::Util::NetworkDevice: def initialize super + unless Puppet.features.ssh? + raise 'Connecting with ssh to a network device requires the \'net/ssh\' ruby library' + end end def handles_login? -- cgit From 5db214c486b82ef7cc7960831a44531f3892f3f6 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Sat, 7 May 2011 17:45:15 +0200 Subject: Prevent spec failure caused by network device mock leak We were leaking some mocks in the network device singleton from tests to tests. Signed-off-by: Brice Figureau --- lib/puppet/util/network_device.rb | 5 +++++ spec/unit/util/network_device_spec.rb | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/lib/puppet/util/network_device.rb b/lib/puppet/util/network_device.rb index d9c1aa34d..a650670ce 100644 --- a/lib/puppet/util/network_device.rb +++ b/lib/puppet/util/network_device.rb @@ -9,4 +9,9 @@ class Puppet::Util::NetworkDevice rescue => detail raise "Can't load #{device.provider} for #{device.name}: #{detail}" end + + # for tests reset + def self.clear + @current = nil + end end \ No newline at end of file diff --git a/spec/unit/util/network_device_spec.rb b/spec/unit/util/network_device_spec.rb index 70cb509b4..347986ac6 100644 --- a/spec/unit/util/network_device_spec.rb +++ b/spec/unit/util/network_device_spec.rb @@ -10,6 +10,10 @@ describe Puppet::Util::NetworkDevice do @device = OpenStruct.new(:name => "name", :provider => "test") end + after(:each) do + Puppet::Util::NetworkDevice.clear + end + class Puppet::Util::NetworkDevice::Test class Device def initialize(device) -- cgit From 9377507499f70c11c87615675656dbffed30a0bf Mon Sep 17 00:00:00 2001 From: Matt Robinson Date: Mon, 9 May 2011 22:02:50 -0700 Subject: (#7468) Stub spec that tries to connect to pypi.python.org I noticed a test failure when I ran the specs without an internet connection. Specs should never need an internet connection to pass. Reviewed-by: Max Martin --- spec/unit/provider/package/pip_spec.rb | 44 ++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/spec/unit/provider/package/pip_spec.rb b/spec/unit/provider/package/pip_spec.rb index b56271029..97b3b5e73 100755 --- a/spec/unit/provider/package/pip_spec.rb +++ b/spec/unit/provider/package/pip_spec.rb @@ -6,16 +6,20 @@ provider_class = Puppet::Type.type(:package).provider(:pip) describe provider_class do before do - @resource = Puppet::Resource.new(:package, "sdsfdssdhdfyjymdgfcjdfjxdrssf") + @resource = Puppet::Resource.new(:package, "fake_package") @provider = provider_class.new(@resource) + client = stub_everything('client') + client.stubs(:call).with('package_releases', 'real_package').returns(["1.3", "1.2.5", "1.2.4"]) + client.stubs(:call).with('package_releases', 'fake_package').returns([]) + XMLRPC::Client.stubs(:new2).returns(client) end describe "parse" do it "should return a hash on valid input" do - provider_class.parse("Django==1.2.5").should == { + provider_class.parse("real_package==1.2.5").should == { :ensure => "1.2.5", - :name => "Django", + :name => "real_package", :provider => :pip, } end @@ -31,7 +35,7 @@ describe provider_class do it "should return an array when pip is present" do provider_class.expects(:which).with('pip').returns("/fake/bin/pip") p = stub("process") - p.expects(:collect).yields("Django==1.2.5") + p.expects(:collect).yields("real_package==1.2.5") provider_class.expects(:execpipe).with("/fake/bin/pip freeze").yields(p) provider_class.instances end @@ -46,19 +50,19 @@ describe provider_class do describe "query" do before do - @resource[:name] = "Django" + @resource[:name] = "real_package" end it "should return a hash when pip and the package are present" do provider_class.expects(:instances).returns [provider_class.new({ :ensure => "1.2.5", - :name => "Django", + :name => "real_package", :provider => :pip, })] @provider.query.should == { :ensure => "1.2.5", - :name => "Django", + :name => "real_package", :provider => :pip, } end @@ -72,13 +76,13 @@ describe provider_class do describe "latest" do - it "should find a version number for Django" do - @resource[:name] = "Django" + it "should find a version number for real_package" do + @resource[:name] = "real_package" @provider.latest.should_not == nil end - it "should not find a version number for sdsfdssdhdfyjymdgfcjdfjxdrssf" do - @resource[:name] = "sdsfdssdhdfyjymdgfcjdfjxdrssf" + it "should not find a version number for fake_package" do + @resource[:name] = "fake_package" @provider.latest.should == nil end @@ -87,15 +91,15 @@ describe provider_class do describe "install" do before do - @resource[:name] = "sdsfdssdhdfyjymdgfcjdfjxdrssf" - @url = "git+https://example.com/sdsfdssdhdfyjymdgfcjdfjxdrssf.git" + @resource[:name] = "fake_package" + @url = "git+https://example.com/fake_package.git" end it "should install" do @resource[:ensure] = :installed @resource[:source] = nil @provider.expects(:lazy_pip). - with("install", '-q', "sdsfdssdhdfyjymdgfcjdfjxdrssf") + with("install", '-q', "fake_package") @provider.install end @@ -103,7 +107,7 @@ describe provider_class do @resource[:ensure] = :installed @resource[:source] = @url @provider.expects(:lazy_pip). - with("install", '-q', '-e', "#{@url}#egg=sdsfdssdhdfyjymdgfcjdfjxdrssf") + with("install", '-q', '-e', "#{@url}#egg=fake_package") @provider.install end @@ -111,14 +115,14 @@ describe provider_class do @resource[:ensure] = "0123456" @resource[:source] = @url @provider.expects(:lazy_pip). - with("install", "-q", "-e", "#{@url}@0123456#egg=sdsfdssdhdfyjymdgfcjdfjxdrssf") + with("install", "-q", "-e", "#{@url}@0123456#egg=fake_package") @provider.install end it "should install a particular version" do @resource[:ensure] = "0.0.0" @resource[:source] = nil - @provider.expects(:lazy_pip).with("install", "-q", "sdsfdssdhdfyjymdgfcjdfjxdrssf==0.0.0") + @provider.expects(:lazy_pip).with("install", "-q", "fake_package==0.0.0") @provider.install end @@ -126,7 +130,7 @@ describe provider_class do @resource[:ensure] = :latest @resource[:source] = nil @provider.expects(:lazy_pip). - with("install", "-q", "--upgrade", "sdsfdssdhdfyjymdgfcjdfjxdrssf") + with("install", "-q", "--upgrade", "fake_package") @provider.install end @@ -135,9 +139,9 @@ describe provider_class do describe "uninstall" do it "should uninstall" do - @resource[:name] = "sdsfdssdhdfyjymdgfcjdfjxdrssf" + @resource[:name] = "fake_package" @provider.expects(:lazy_pip). - with('uninstall', '-y', '-q', 'sdsfdssdhdfyjymdgfcjdfjxdrssf') + with('uninstall', '-y', '-q', 'fake_package') @provider.uninstall end -- cgit From a44cbb1b9a032d7aeabe59662e60794569e7bcbd Mon Sep 17 00:00:00 2001 From: nfagerlund Date: Wed, 11 May 2011 13:49:24 -0700 Subject: (#7264) Docs: Clarify that subscribe/notify imply require/before This is a doc string only commit. The metaparameter reference was not clear about subscribe and notify being supersets of require and before, respectively. This commit also cleans up some unrelated quoting, arrow-alignment, and language flow issues. --- lib/puppet/type.rb | 84 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index 656b8f264..1dbf12451 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -957,13 +957,13 @@ class Type schedule object, and then reference the name of that object to use that for your schedule: - schedule { daily: + schedule { 'daily': period => daily, - range => \"2-4\" + range => \"2-4\" } exec { \"/usr/bin/apt-get update\": - schedule => daily + schedule => 'daily' } The creation of the schedule object does not need to appear in the @@ -1055,9 +1055,9 @@ class Type newmetaparam(:alias) do desc "Creates an alias for the object. Puppet uses this internally when you - provide a symbolic name: + provide a symbolic title: - file { sshdconfig: + file { 'sshdconfig': path => $operatingsystem ? { solaris => \"/usr/local/etc/ssh/sshd_config\", default => \"/etc/ssh/sshd_config\" @@ -1065,30 +1065,30 @@ class Type source => \"...\" } - service { sshd: - subscribe => File[sshdconfig] + service { 'sshd': + subscribe => File['sshdconfig'] } - When you use this feature, the parser sets `sshdconfig` as the name, + When you use this feature, the parser sets `sshdconfig` as the title, and the library sets that as an alias for the file so the dependency - lookup for `sshd` works. You can use this parameter yourself, + lookup in `Service['sshd']` works. You can use this metaparameter yourself, but note that only the library can use these aliases; for instance, the following code will not work: file { \"/etc/ssh/sshd_config\": owner => root, group => root, - alias => sshdconfig + alias => 'sshdconfig' } - file { sshdconfig: + file { 'sshdconfig': mode => 644 } There's no way here for the Puppet parser to know that these two stanzas should be affecting the same file. - See the [Language Tutorial](http://docs.puppetlabs.com/guides/language_tutorial.html) for more information. + See the [Language Guide](http://docs.puppetlabs.com/guides/language_guide.html) for more information. " @@ -1222,7 +1222,7 @@ class Type # solution, but it works. newmetaparam(:require, :parent => RelationshipMetaparam, :attributes => {:direction => :in, :events => :NONE}) do - desc "One or more objects that this object depends on. + desc "References to one or more objects that this object depends on. This is used purely for guaranteeing that changes to required objects happen before the dependent object. For instance: @@ -1232,8 +1232,8 @@ class Type } file { \"/usr/local/scripts/myscript\": - source => \"puppet://server/module/myscript\", - mode => 755, + source => \"puppet://server/module/myscript\", + mode => 755, require => File[\"/usr/local/scripts\"] } @@ -1247,7 +1247,7 @@ class Type ways to autorequire objects, so if you think Puppet could be smarter here, let us know. - In fact, the above code was redundant -- Puppet will autorequire + In fact, the above code was redundant --- Puppet will autorequire any parent directories that are being managed; it will automatically realize that the parent directory should be created before the script is pulled down. @@ -1263,40 +1263,41 @@ class Type end newmetaparam(:subscribe, :parent => RelationshipMetaparam, :attributes => {:direction => :in, :events => :ALL_EVENTS, :callback => :refresh}) do - desc "One or more objects that this object depends on. Changes in the - subscribed to objects result in the dependent objects being - refreshed (e.g., a service will get restarted). For instance: + desc "References to one or more objects that this object depends on. This + metaparameter creates a dependency relationship like **require,** + and also causes the dependent object to be refreshed when the + subscribed object is changed. For instance: class nagios { - file { \"/etc/nagios/nagios.conf\": + file { 'nagconf': + path => \"/etc/nagios/nagios.conf\" source => \"puppet://server/module/nagios.conf\", - alias => nagconf # just to make things easier for me } - service { nagios: - ensure => running, - subscribe => File[nagconf] + service { 'nagios': + ensure => running, + subscribe => File['nagconf'] } } - Currently the `exec`, `mount` and `service` type support + Currently the `exec`, `mount` and `service` types support refreshing. " end newmetaparam(:before, :parent => RelationshipMetaparam, :attributes => {:direction => :out, :events => :NONE}) do - desc %{This parameter is the opposite of **require** -- it guarantees - that the specified object is applied later than the specifying - object: + desc %{References to one or more objects that depend on this object. This + parameter is the opposite of **require** --- it guarantees that + the specified object is applied later than the specifying object: file { "/var/nagios/configuration": source => "...", recurse => true, - before => Exec["nagios-rebuid"] + before => Exec["nagios-rebuid"] } exec { "nagios-rebuild": command => "/usr/bin/make", - cwd => "/var/nagios/configuration" + cwd => "/var/nagios/configuration" } This will make sure all of the files are up to date before the @@ -1304,15 +1305,18 @@ class Type end newmetaparam(:notify, :parent => RelationshipMetaparam, :attributes => {:direction => :out, :events => :ALL_EVENTS, :callback => :refresh}) do - desc %{This parameter is the opposite of **subscribe** -- it sends events - to the specified object: + desc %{References to one or more objects that depend on this object. This + parameter is the opposite of **subscribe** --- it creates a + dependency relationship like **before,** and also causes the + dependent object(s) to be refreshed when this object is changed. For + instance: file { "/etc/sshd_config": source => "....", - notify => Service[sshd] + notify => Service['sshd'] } - service { sshd: + service { 'sshd': ensure => running } @@ -1328,24 +1332,24 @@ class Type By default, all classes get directly added to the 'main' stage. You can create new stages as resources: - stage { [pre, post]: } + stage { ['pre', 'post']: } To order stages, use standard relationships: - stage { pre: before => Stage[main] } + stage { 'pre': before => Stage['main'] } Or use the new relationship syntax: - Stage[pre] -> Stage[main] -> Stage[post] + Stage['pre'] -> Stage['main'] -> Stage['post'] Then use the new class parameters to specify a stage: - class { foo: stage => pre } + class { 'foo': stage => 'pre' } Stages can only be set on classes, not individual resources. This will fail: - file { '/foo': stage => pre, ensure => file } + file { '/foo': stage => 'pre', ensure => file } } end @@ -1478,7 +1482,7 @@ class Type newparam(:provider) do desc "The specific backend for #{self.name.to_s} to use. You will - seldom need to specify this -- Puppet will usually discover the + seldom need to specify this --- Puppet will usually discover the appropriate provider for your platform." # This is so we can refer back to the type to get a list of -- cgit From 09f5d9cbfcbc6b0428d02fd4b5e9c0cfe550eb19 Mon Sep 17 00:00:00 2001 From: Max Martin Date: Thu, 12 May 2011 15:43:51 -0700 Subject: (#7469) Add license to test face so tests pass The TestIndirection test face defined in indirection_base_spec did not have copyright or license information defined; this was causing order-dependent test failures when unit tests were run before other specs (as in rake spec). This commit adds license and copyright info to the test face to prevent these failures. Paired-with: Daniel Pittman --- spec/unit/application/indirection_base_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/unit/application/indirection_base_spec.rb b/spec/unit/application/indirection_base_spec.rb index 910774c14..d72def6cf 100755 --- a/spec/unit/application/indirection_base_spec.rb +++ b/spec/unit/application/indirection_base_spec.rb @@ -10,6 +10,8 @@ end face = Puppet::Indirector::Face.define(:testindirection, '0.0.1') do summary "fake summary" + copyright "Puppet Labs", 2011 + license "Apache 2 license; see COPYING" end # REVISIT: This horror is required because we don't allow anything to be # :current except for if it lives on, and is loaded from, disk. --daniel 2011-03-29 -- cgit From d63fc34d0c6fdfbe72dafdf5d07a6cc9c6dd388e Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 6 May 2011 12:09:09 -0700 Subject: Revert "(#7220) Add the ability to "inherit" options." This reverts commit 9329a1f33b4d7df81ad8661de74f8a3656428570. Conflicts: spec/unit/interface/action_builder_spec.rb spec/unit/interface/action_spec.rb --- lib/puppet/interface/action.rb | 5 --- lib/puppet/interface/action_builder.rb | 10 ----- spec/unit/interface/action_builder_spec.rb | 62 ------------------------------ spec/unit/interface/action_spec.rb | 59 ---------------------------- 4 files changed, 136 deletions(-) diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb index 114e5341b..622371a4e 100644 --- a/lib/puppet/interface/action.rb +++ b/lib/puppet/interface/action.rb @@ -254,11 +254,6 @@ WRAPPER option end - def inherit_options_from(action) - options = action.options.map { |opt| action.get_option(opt, false) } - options.reject!(&:nil?).uniq.each { |option| add_option(option) } - end - def option?(name) @options_hash.include? name.to_sym end diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb index ba5531f1d..62db8de06 100644 --- a/lib/puppet/interface/action_builder.rb +++ b/lib/puppet/interface/action_builder.rb @@ -31,16 +31,6 @@ class Puppet::Interface::ActionBuilder @action.add_option(option) end - def inherit_options_from(action) - if action.is_a? Symbol - name = action - action = @face.get_action(name) or - raise ArgumentError, "This Face has no action named #{name}!" - end - - @action.inherit_options_from(action) - end - def default(value = true) @action.default = !!value end diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb index e9f10a1a6..c39860591 100755 --- a/spec/unit/interface/action_builder_spec.rb +++ b/spec/unit/interface/action_builder_spec.rb @@ -62,68 +62,6 @@ describe Puppet::Interface::ActionBuilder do end end - describe "#inherit_options_from" do - let :face do - Puppet::Interface.new(:face_with_some_options, '0.0.1') do - option '-w' - - action(:foo) do - when_invoked do true end - option '-x', '--ex' - option '-y', '--why' - end - - action(:bar) do - when_invoked do true end - option '-z', '--zee' - end - - action(:baz) do - when_invoked do true end - option '-z', '--zed' - end - end - end - - it 'should add the options from the specified action' do - foo = face.get_action(:foo) - action = Puppet::Interface::ActionBuilder.build(face, :inherit_options) do - when_invoked do true end - inherit_options_from foo - end - action.options.should == foo.options - end - - it 'should add the options from multiple actions' do - foo = face.get_action(:foo) - bar = face.get_action(:bar) - action = Puppet::Interface::ActionBuilder.build(face, :inherit_options) do - when_invoked do true end - inherit_options_from foo - inherit_options_from bar - end - action.options.should == (foo.options + bar.options).uniq - end - - it 'should permit symbolic names for actions in the same face' do - foo = face.get_action(:foo) - action = Puppet::Interface::ActionBuilder.build(face, :inherit_options) do - when_invoked do true end - inherit_options_from :foo - end - action.options.should == foo.options - end - - it 'should raise a useful error if you supply a bad action name' do - expect do - Puppet::Interface::ActionBuilder.build(face, :inherit_options) do - when_invoked do true end - inherit_options_from :nowhere - end - end.to raise_error /nowhere/ - end - end - context "inline documentation" do it "should set the summary" do action = Puppet::Interface::ActionBuilder.build(face, :foo) do diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb index f43968709..9e539c68e 100755 --- a/spec/unit/interface/action_spec.rb +++ b/spec/unit/interface/action_spec.rb @@ -68,65 +68,6 @@ describe Puppet::Interface::Action do end end - describe "#inherit_options_from" do - let :face do - Puppet::Interface.new(:face_with_some_options, '0.0.1') do - option '-w' - - action(:foo) do - when_invoked do true end - option '-x', '--ex' - option '-y', '--why' - end - - action(:bar) do - when_invoked do true end - option '-z', '--zee' - end - - action(:baz) do - when_invoked do true end - option '-z', '--zed' - end - - action(:noopts) do - # no options declared - when_invoked do true end - end - end - end - - subject { action = face.action(:new_action) { when_invoked do true end } } - - it 'should add the options from the specified action' do - subject.inherit_options_from(foo = face.get_action(:foo)) - subject.options.should == foo.options - end - - it 'should not die when the specified action has no options' do - original_options = subject.options - subject.inherit_options_from(face.get_action(:noopts)) - subject.options.should == original_options - end - - it 'should add the options from multiple actions' do - subject.inherit_options_from(foo = face.get_action(:foo)) - subject.inherit_options_from(bar = face.get_action(:bar)) - subject.options.should == (foo.options + bar.options).uniq - end - - it 'should not inherit face options' do - subject.expects(:add_option) - subject.expects(:add_option).with(face.get_option(:w)).never - subject.inherit_options_from(face.get_action(:bar)) - end - - it 'should raise an error if inheritance would duplicate options' do - subject.inherit_options_from(face.get_action(:bar)) - expect { subject.inherit_options_from(face.get_action(:baz)) }.to raise_error - 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 -- cgit