Merge pull request redhat-openstack#251 from nibalizer/fix_340

Use -q flag instead of array slices
jguiditta · Nov 5, 2014 · 61991d7 · 61991d7
2 parents 4832bd6 + 66ffaaf
commit 61991d7
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 91 deletions.
diff --git a/lib/puppet/provider/rabbitmq_user/rabbitmqctl.rb b/lib/puppet/provider/rabbitmq_user/rabbitmqctl.rb
@@ -13,7 +13,7 @@
   defaultfor :feature => :posix
 
   def self.instances
-    rabbitmqctl('list_users').split(/\n/)[1..-2].collect do |line|
+    rabbitmqctl('-q', 'list_users').split(/\n/).collect do |line|
       if line =~ /^(\S+)(\s+\[.*?\]|)$/
         new(:name => $1)
       else
@@ -37,7 +37,7 @@ def destroy
   end
 
   def exists?
-    rabbitmqctl('list_users').split(/\n/)[1..-2].detect do |line|
+    rabbitmqctl('-q', 'list_users').split(/\n/).detect do |line|
       line.match(/^#{Regexp.escape(resource[:name])}(\s+(\[.*?\]|\S+)|)$/)
     end
   end
@@ -90,7 +90,7 @@ def make_user_admin
 
   private
   def get_user_tags
-    match = rabbitmqctl('list_users').split(/\n/)[1..-2].collect do |line|
+    match = rabbitmqctl('-q', 'list_users').split(/\n/).collect do |line|
       line.match(/^#{Regexp.escape(resource[:name])}\s+\[(.*?)\]/)
     end.compact.first
     Set.new(match[1].split(/, /)) if match

diff --git a/lib/puppet/provider/rabbitmq_user_permissions/rabbitmqctl.rb b/lib/puppet/provider/rabbitmq_user_permissions/rabbitmqctl.rb
@@ -15,7 +15,7 @@ def self.users(name, vhost)
     @users = {} unless @users
     unless @users[name]
       @users[name] = {}
-      rabbitmqctl('list_user_permissions', name).split(/\n/)[1..-2].each do |line|
+      rabbitmqctl('-q', 'list_user_permissions', name).split(/\n/).each do |line|
         if line =~ /^(\S+)\s+(\S*)\s+(\S*)\s+(\S*)$/
           @users[name][$1] =
             {:configure => $2, :read => $4, :write => $3}

diff --git a/lib/puppet/provider/rabbitmq_vhost/rabbitmqctl.rb b/lib/puppet/provider/rabbitmq_vhost/rabbitmqctl.rb
@@ -9,7 +9,7 @@
   end
 
   def self.instances
-    rabbitmqctl('list_vhosts').split(/\n/)[1..-2].map do |line|
+    rabbitmqctl('-q', 'list_vhosts').split(/\n/).map do |line|
       if line =~ /^(\S+)$/
         new(:name => $1)
       else
@@ -27,7 +27,7 @@ def destroy
   end
 
   def exists?
-    out = rabbitmqctl('list_vhosts').split(/\n/)[1..-2].detect do |line|
+    out = rabbitmqctl('-q', 'list_vhosts').split(/\n/).detect do |line|
       line.match(/^#{Regexp.escape(resource[:name])}$/)
     end
   end

diff --git a/spec/unit/puppet/provider/rabbitmq_user/rabbitmqctl_spec.rb b/spec/unit/puppet/provider/rabbitmq_user/rabbitmqctl_spec.rb
@@ -12,44 +12,34 @@
     @provider = provider_class.new(@resource)
   end
   it 'should match user names' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo
-...done.
 EOT
     @provider.exists?.should == 'foo'
   end
   it 'should match user names with 2.4.1 syntax' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo bar
-...done.
 EOT
     @provider.exists?.should == 'foo bar'
   end
   it 'should not match if no users on system' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
-...done.
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 EOT
     @provider.exists?.should be_nil
   end
   it 'should not match if no matching users on system' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 fooey
-...done.
 EOT
     @provider.exists?.should be_nil
   end
   it 'should match user names from list' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 one
 two three
 foo
 bar
-...done.
 EOT
     @provider.exists?.should == 'foo'
   end
@@ -62,13 +52,11 @@
     @resource[:password] = 'bar'
     @resource[:admin] = 'true'
     @provider.expects(:rabbitmqctl).with('add_user', 'foo', 'bar')
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo   []
 icinga  [monitoring]
 kitchen []
 kitchen2        [abc, def, ghi]
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['administrator'])
     @provider.create
@@ -78,129 +66,107 @@
     @provider.destroy
   end
   it 'should be able to retrieve admin value' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo [administrator]
-...done.
 EOT
     @provider.admin.should == :true
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 one [administrator]
 foo []
-...done.
 EOT
     @provider.admin.should == :false
   end
   it 'should fail if admin value is invalid' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo fail
-...done.
 EOT
     expect { @provider.admin }.to raise_error(Puppet::Error, /Could not match line/)
   end
   it 'should be able to set admin value' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo   []
 icinga  [monitoring]
 kitchen []
 kitchen2        [abc, def, ghi]
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['administrator'])
     @provider.admin=:true
   end
   it 'should not interfere with existing tags on the user when setting admin value' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo   [bar, baz]
 icinga  [monitoring]
 kitchen []
 kitchen2        [abc, def, ghi]
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['bar','baz', 'administrator'].sort) 
     @provider.admin=:true
   end
   it 'should be able to unset admin value' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo     [administrator]
 guest   [administrator]
 icinga  []
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', [])
     @provider.admin=:false
   end
   it 'should not interfere with existing tags on the user when unsetting admin value' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo   [administrator, bar, baz]
 icinga  [monitoring]
 kitchen []
 kitchen2        [abc, def, ghi]
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['bar','baz'].sort) 
     @provider.admin=:false
   end
 
   it 'should clear all tags on existing user' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 one [administrator]
 foo [tag1,tag2]
 icinga  [monitoring]
 kitchen []
 kitchen2        [abc, def, ghi]
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', [])
     @provider.tags=[]
   end
 
   it 'should set multiple tags' do
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 one [administrator]
 foo []
 icinga  [monitoring]
 kitchen []
 kitchen2        [abc, def, ghi]
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ['tag1','tag2'])
     @provider.tags=['tag1','tag2']
   end
 
   it 'should clear tags while keep admin tag' do
     @resource[:admin]  = true
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 one [administrator]
 foo [administrator, tag1, tag2]
 icinga  [monitoring]
 kitchen []
 kitchen2        [abc, def, ghi]
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ["administrator"])
     @provider.tags=[]
   end
 
   it 'should change tags while keep admin tag' do
     @resource[:admin]  = true
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 one [administrator]
 foo [administrator, tag1, tag2]
 icinga  [monitoring]
 kitchen []
 kitchen2        [abc, def, ghi]
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ["administrator","tag1","tag3","tag7"])
     @provider.tags=['tag1','tag7','tag3']
@@ -210,10 +176,8 @@
     @resource[:tags] = [ "tag1", "tag2" ]
     @provider.expects(:rabbitmqctl).with('add_user', 'foo', 'bar')
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ["tag1","tag2"])
-    @provider.expects(:rabbitmqctl).with('list_users').returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').returns <<-EOT
 foo []
-...done.
 EOT
     @provider.create
   end
@@ -222,15 +186,12 @@
     @resource[:tags] = [ "tag1", "tag2" ]
     @resource[:admin]  = true
     @provider.expects(:rabbitmqctl).with('add_user', 'foo', 'bar')
-    @provider.expects(:rabbitmqctl).with('list_users').twice.returns <<-EOT
-Listing users ...
+    @provider.expects(:rabbitmqctl).with('-q', 'list_users').twice.returns <<-EOT
 foo []
-...done.
 EOT
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ["administrator"])
     @provider.expects(:rabbitmqctl).with('set_user_tags', 'foo', ["administrator","tag1","tag2"])
     @provider.create
   end
-
 
 end
diff --git a/spec/unit/puppet/provider/rabbitmq_user_permissions/rabbitmqctl_spec.rb b/spec/unit/puppet/provider/rabbitmq_user_permissions/rabbitmqctl_spec.rb
@@ -15,33 +15,25 @@
     @provider_class.instance_variable_set(:@users, nil)
   end
   it 'should match user permissions from list' do
-    @provider.class.expects(:rabbitmqctl).with('list_user_permissions', 'foo').returns <<-EOT
-Listing users ...
+    @provider.class.expects(:rabbitmqctl).with('-q', 'list_user_permissions', 'foo').returns <<-EOT
 bar 1 2 3
-...done.
 EOT
     @provider.exists?.should == {:configure=>"1", :write=>"2", :read=>"3"}
   end
   it 'should match user permissions with empty columns' do
-    @provider.class.expects(:rabbitmqctl).with('list_user_permissions', 'foo').returns <<-EOT
-Listing users ...
+    @provider.class.expects(:rabbitmqctl).with('-q', 'list_user_permissions', 'foo').returns <<-EOT
 bar			3
-...done.
 EOT
     @provider.exists?.should == {:configure=>"", :write=>"", :read=>"3"}
   end
   it 'should not match user permissions with more than 3 columns' do
-    @provider.class.expects(:rabbitmqctl).with('list_user_permissions', 'foo').returns <<-EOT
-Listing users ...
+    @provider.class.expects(:rabbitmqctl).with('-q', 'list_user_permissions', 'foo').returns <<-EOT
 bar 1 2 3 4
-...done.
 EOT
     expect { @provider.exists? }.to raise_error(Puppet::Error, /cannot parse line from list_user_permissions/)
   end
   it 'should not match an empty list' do
-    @provider.class.expects(:rabbitmqctl).with('list_user_permissions', 'foo').returns <<-EOT
-Listing users ...
-...done.
+    @provider.class.expects(:rabbitmqctl).with('-q', 'list_user_permissions', 'foo').returns <<-EOT
 EOT
     @provider.exists?.should == nil
   end
@@ -59,20 +51,16 @@
   end
   {:configure_permission => '1', :write_permission => '2', :read_permission => '3'}.each do |k,v|
     it "should be able to retrieve #{k}" do
-      @provider.class.expects(:rabbitmqctl).with('list_user_permissions', 'foo').returns <<-EOT
-Listing users ...
+      @provider.class.expects(:rabbitmqctl).with('-q', 'list_user_permissions', 'foo').returns <<-EOT
 bar 1 2 3
-...done.
 EOT
       @provider.send(k).should == v
     end
   end
   {:configure_permission => '1', :write_permission => '2', :read_permission => '3'}.each do |k,v|
     it "should be able to retrieve #{k} after exists has been called" do
-      @provider.class.expects(:rabbitmqctl).with('list_user_permissions', 'foo').returns <<-EOT
-Listing users ...
+      @provider.class.expects(:rabbitmqctl).with('-q', 'list_user_permissions', 'foo').returns <<-EOT
 bar 1 2 3
-...done.
 EOT
       @provider.exists?
       @provider.send(k).should == v
@@ -83,21 +71,17 @@
    :write_permission     => ['1', 'foo', '3']
   }.each do |perm, columns|
     it "should be able to sync #{perm}" do
-      @provider.class.expects(:rabbitmqctl).with('list_user_permissions', 'foo').returns <<-EOT
-Listing users ...
+      @provider.class.expects(:rabbitmqctl).with('-q', 'list_user_permissions', 'foo').returns <<-EOT
 bar 1 2 3
-...done.
 EOT
       @provider.resource[perm] = 'foo'
       @provider.expects(:rabbitmqctl).with('set_permissions', '-p', 'bar', 'foo', *columns)
       @provider.send("#{perm}=".to_sym, 'foo')
     end
   end
   it 'should only call set_permissions once' do
-    @provider.class.expects(:rabbitmqctl).with('list_user_permissions', 'foo').returns <<-EOT
-Listing users ...
+    @provider.class.expects(:rabbitmqctl).with('-q', 'list_user_permissions', 'foo').returns <<-EOT
 bar 1 2 3
-...done.
 EOT
     @provider.resource[:configure_permission] = 'foo'
     @provider.resource[:read_permission] = 'foo'

diff --git a/spec/unit/puppet/provider/rabbitmq_vhost/rabbitmqctl_spec.rb b/spec/unit/puppet/provider/rabbitmq_vhost/rabbitmqctl_spec.rb
@@ -12,22 +12,22 @@
     @provider = provider_class.new(@resource)
   end
   it 'should match vhost names' do
-    @provider.expects(:rabbitmqctl).with('list_vhosts').returns <<-EOT
+    @provider.expects(:rabbitmqctl).with('-q', 'list_vhosts').returns <<-EOT
 Listing vhosts ...
 foo
 ...done.
 EOT
     @provider.exists?.should == 'foo'
   end
   it 'should not match if no vhosts on system' do
-    @provider.expects(:rabbitmqctl).with('list_vhosts').returns <<-EOT
+    @provider.expects(:rabbitmqctl).with('-q', 'list_vhosts').returns <<-EOT
 Listing vhosts ...
 ...done.
 EOT
     @provider.exists?.should be_nil
   end
   it 'should not match if no matching vhosts on system' do
-    @provider.expects(:rabbitmqctl).with('list_vhosts').returns <<-EOT
+    @provider.expects(:rabbitmqctl).with('-q', 'list_vhosts').returns <<-EOT
 Listing vhosts ...
 fooey
 ...done.