Merge pull request #208 from glennsarti/maint-rubocop

(maint) Fix rubocop violations

.rubocop.yml

@@ -17,6 +17,9 @@ Style/Documentation:
 Metrics/LineLength:
   Enabled: false
 
+# Empty method definitions over more than one line is ok
+Style/EmptyMethod:
+  Enabled: false
 
 # Due to legacy codebase
 #   - Globals are used liberally
@@ -29,6 +32,8 @@ Metrics/PerceivedComplexity:
   Enabled: false
 Metrics/CyclomaticComplexity:
   Enabled: false
+Metrics/BlockNesting:
+  Enabled: false
 #   - Long Methods, Classes, Blocks, and Modules
 Metrics/MethodLength:
   Enabled: false
@@ -39,6 +44,9 @@ Metrics/BlockLength:
 Metrics/ModuleLength:
   Enabled: false
 
+# WordArray is crashing rubocop in lib/vmpooler/api/helpers.rb
+Style/WordArray:
+  Enabled: false
 
 # Either sytnax for regex is ok
 Style/RegexpLiteral:

.rubocop_todo.yml

@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2017-03-02 20:16:29 -0800 using RuboCop version 0.47.1.
+# on 2017-03-16 15:37:18 -0700 using RuboCop version 0.47.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -13,10 +13,9 @@ Lint/AssignmentInCondition:
     - 'lib/vmpooler/api/helpers.rb'
     - 'lib/vmpooler/api/v1.rb'
 
-# Offense count: 6
+# Offense count: 5
 Lint/ConditionPosition:
   Exclude:
-    - 'lib/vmpooler/api/dashboard.rb'
     - 'lib/vmpooler/pool_manager.rb'
 
 # Offense count: 3
@@ -30,13 +29,12 @@ Lint/ShadowingOuterLocalVariable:
   Exclude:
     - 'lib/vmpooler/api/helpers.rb'
 
-# Offense count: 8
+# Offense count: 3
 # Cop supports --auto-correct.
 # Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods.
 Lint/UnusedMethodArgument:
   Exclude:
     - 'lib/vmpooler/api/helpers.rb'
-    - 'lib/vmpooler/dummy_statsd.rb'
     - 'lib/vmpooler/pool_manager.rb'
     - 'lib/vmpooler/vsphere_helper.rb'
 
@@ -46,11 +44,6 @@ Lint/UselessAssignment:
     - 'lib/vmpooler/api/dashboard.rb'
     - 'lib/vmpooler/api/helpers.rb'
 
-# Offense count: 5
-# Configuration parameters: CountBlocks.
-Metrics/BlockNesting:
-  Max: 5
-
 # Offense count: 1
 # Configuration parameters: CountKeywordArgs.
 Metrics/ParameterLists:
@@ -134,13 +127,12 @@ Style/EmptyLines:
   Exclude:
     - 'lib/vmpooler/api/dashboard.rb'
 
-# Offense count: 2
+# Offense count: 1
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: empty_lines, empty_lines_except_namespace, empty_lines_special, no_empty_lines
 Style/EmptyLinesAroundClassBody:
   Exclude:
-    - 'lib/vmpooler/api/dashboard.rb'
     - 'lib/vmpooler/api/helpers.rb'
 
 # Offense count: 3
@@ -151,27 +143,12 @@ Style/EmptyLinesAroundModuleBody:
   Exclude:
     - 'lib/vmpooler/api/helpers.rb'
 
-# Offense count: 2
-# Cop supports --auto-correct.
-Style/EmptyLiteral:
-  Exclude:
-    - 'lib/vmpooler/api/dashboard.rb'
-
 # Offense count: 1
 # Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyle, SupportedStyles.
-# SupportedStyles: compact, expanded
-Style/EmptyMethod:
-  Exclude:
-    - 'lib/vmpooler/dummy_statsd.rb'
-
-# Offense count: 3
-# Cop supports --auto-correct.
 # Configuration parameters: AllowForAlignment, ForceEqualSignAlignment.
 Style/ExtraSpacing:
   Exclude:
     - 'lib/vmpooler/pool_manager.rb'
-    - 'lib/vmpooler/statsd.rb'
 
 # Offense count: 2
 # Cop supports --auto-correct.
@@ -244,13 +221,11 @@ Style/IndentHash:
     - 'lib/vmpooler/api/helpers.rb'
     - 'lib/vmpooler/api/v1.rb'
 
-# Offense count: 4
+# Offense count: 2
 # Cop supports --auto-correct.
 # Configuration parameters: Width.
 Style/IndentationWidth:
   Exclude:
-    - 'lib/vmpooler/api/dashboard.rb'
-    - 'lib/vmpooler/api/reroute.rb'
     - 'lib/vmpooler/api/v1.rb'
     - 'lib/vmpooler/vsphere_helper.rb'
 
@@ -412,13 +387,12 @@ Style/SpaceAroundKeyword:
   Exclude:
     - 'lib/vmpooler/api/v1.rb'
 
-# Offense count: 4
+# Offense count: 1
 # Cop supports --auto-correct.
 # Configuration parameters: AllowForAlignment.
 Style/SpaceAroundOperators:
   Exclude:
     - 'lib/vmpooler/api/v1.rb'
-    - 'lib/vmpooler/statsd.rb'
 
 # Offense count: 4
 # Cop supports --auto-correct.
@@ -442,25 +416,14 @@ Style/SpaceInsideParens:
   Exclude:
     - 'lib/vmpooler/api/v1.rb'
 
-# Offense count: 2
+# Offense count: 12
-# Cop supports --auto-correct.
-Style/SpaceInsidePercentLiteralDelimiters:
-  Exclude:
-    - 'lib/vmpooler/api.rb'
-
-# Offense count: 47
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles, ConsistentQuotesInMultiline.
 # SupportedStyles: single_quotes, double_quotes
 Style/StringLiterals:
   Exclude:
-    - 'lib/vmpooler/api.rb'
-    - 'lib/vmpooler/api/dashboard.rb'
-    - 'lib/vmpooler/api/reroute.rb'
     - 'lib/vmpooler/api/v1.rb'
-    - 'lib/vmpooler/graphite.rb'
     - 'lib/vmpooler/pool_manager.rb'
-    - 'lib/vmpooler/statsd.rb'
     - 'lib/vmpooler/vsphere_helper.rb'
     - 'vmpooler'
 

lib/vmpooler/api.rb

@@ -22,7 +22,7 @@ module Vmpooler
 
     # Load dashboard components
     begin
-      require "dashboard"
+      require 'dashboard'
     rescue LoadError
       require File.expand_path(File.join(File.dirname(__FILE__), 'dashboard'))
     end

lib/vmpooler/api/dashboard.rb

@@ -1,7 +1,6 @@
 module Vmpooler
   class API
     class Dashboard < Sinatra::Base
-
       # handle to the App's configuration information
       def config
         @config ||= Vmpooler::API.settings.config
@@ -27,7 +26,7 @@ module Vmpooler
         return @graph_prefix if @graph_prefix
 
         if config[:graphs]
-        return "vmpooler" unless config[:graphs]['prefix']
+          return 'vmpooler' unless config[:graphs]['prefix']
           @graph_prefix = config[:graphs]['prefix']
         elsif config[:graphite]
           return false unless config[:graphite]['prefix']
@@ -44,8 +43,8 @@ module Vmpooler
       end
 
       # return a full URL to a viewable graph for a given metrics target (graphite syntax)
-    def graph_link(target = "")
+      def graph_link(target = '')
-      return "" unless graph_url
+        return '' unless graph_url
         graph_url + target
       end
 
@@ -74,7 +73,7 @@ module Vmpooler
 
                   if result[pool['name']]
                     pool['last'] = result[pool['name']]['size']
-                  result[pool['name']]['history'] ||= Array.new
+                    result[pool['name']]['history'] ||= []
 
                     pool['datapoints'].each do |metric|
                       8.times do |_n|
@@ -120,12 +119,10 @@ module Vmpooler
                 if pool['target'] =~ /.*\.(.*)$/
                   pool['name'] = Regexp.last_match[1]
                   pool['major'] = Regexp.last_match[1] if pool['name'] =~ /^(\w+)\-/
-                result[pool['major']]['history'] ||= Array.new
+                  result[pool['major']]['history'] ||= []
 
                   for i in 0..pool['datapoints'].length
-                  if
+                    if pool['datapoints'][i] && pool['datapoints'][i][0]
-                    pool['datapoints'][i] &&
-                    pool['datapoints'][i][0]
                       pool['last'] = pool['datapoints'][i][0]
                       result[pool['major']]['history'][i] ||= 0
                       result[pool['major']]['history'][i] = result[pool['major']]['history'][i].to_i + pool['datapoints'][i][0].to_i

lib/vmpooler/api/reroute.rb

@@ -4,67 +4,67 @@ module Vmpooler
       api_version = '1'
 
       get '/status/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/status")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/status")
       end
 
       get '/summary/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/summary")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/summary")
       end
 
       get '/summary/:route/?:key?/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/summary/#{params[:route]}/#{params[:key]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/summary/#{params[:route]}/#{params[:key]}")
       end
 
       get '/token/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/token")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/token")
       end
 
       post '/token/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/token")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/token")
       end
 
       get '/token/:token/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/token/#{params[:token]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/token/#{params[:token]}")
       end
 
       delete '/token/:token/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/token/#{params[:token]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/token/#{params[:token]}")
       end
 
       get '/vm/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm")
       end
 
       post '/vm/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm")
       end
 
       post '/vm/:template/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm/#{params[:template]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm/#{params[:template]}")
       end
 
       get '/vm/:hostname/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm/#{params[:hostname]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm/#{params[:hostname]}")
       end
 
       delete '/vm/:hostname/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm/#{params[:hostname]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm/#{params[:hostname]}")
       end
 
       put '/vm/:hostname/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm/#{params[:hostname]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm/#{params[:hostname]}")
       end
 
       post '/vm/:hostname/snapshot/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm/#{params[:hostname]}/snapshot")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm/#{params[:hostname]}/snapshot")
       end
 
       post '/vm/:hostname/snapshot/:snapshot/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm/#{params[:hostname]}/snapshot/#{params[:snapshot]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm/#{params[:hostname]}/snapshot/#{params[:snapshot]}")
       end
 
       put '/vm/:hostname/disk/:size/?' do
-      call env.merge("PATH_INFO" => "/api/v#{api_version}/vm/#{params[:hostname]}/disk/#{params[:size]}")
+        call env.merge('PATH_INFO' => "/api/v#{api_version}/vm/#{params[:hostname]}/disk/#{params[:size]}")
       end
     end
   end

lib/vmpooler/dummy_statsd.rb

@@ -2,18 +2,18 @@ module Vmpooler
   class DummyStatsd
     attr_reader :server, :port, :prefix
 
-    def initialize(params = {})
+    def initialize(*)
     end
 
-    def increment(label)
+    def increment(*)
       true
     end
 
-    def gauge(label, value)
+    def gauge(*)
       true
     end
 
-    def timing(label, duration)
+    def timing(*)
       true
     end
   end

lib/vmpooler/graphite.rb

@@ -5,13 +5,13 @@ module Vmpooler
     attr_reader :server, :port, :prefix
 
     def initialize(params = {})
-      if params["server"].nil? || params["server"].empty?
+      if params['server'].nil? || params['server'].empty?
         raise ArgumentError, "Graphite server is required. Config: #{params.inspect}"
       end
 
-      @server = params["server"]
+      @server = params['server']
-      @port   = params["port"]   || 2003
+      @port   = params['port'] || 2003
-      @prefix = params["prefix"] || "vmpooler"
+      @prefix = params['prefix'] || 'vmpooler'
     end
 
     def increment(label)

lib/vmpooler/providers/base.rb

@@ -19,16 +19,16 @@ module Vmpooler
         # returns
         #   hashtable
         #     name : name of the device   <---- TODO is this all?
-        def vms_in_pool(pool)
+        def vms_in_pool(_pool)
-          fail "#{self.class.name} does not implement vms_in_pool"
+          raise("#{self.class.name} does not implement vms_in_pool")
         end
 
         # inputs
         #   vm_name: string
         # returns
         #    [String] hostname   = Name of the host computer running the vm.  If this is not a Virtual Machine, it returns the vm_name
-        def get_vm_host(vm_name)
+        def get_vm_host(_vm_name)
-          fail "#{self.class.name} does not implement get_vm_host"
+          raise("#{self.class.name} does not implement get_vm_host")
         end
 
         # inputs
@@ -36,8 +36,8 @@ module Vmpooler
         # returns
         #    [String] hostname   = Name of the most appropriate host computer to run this VM.  Useful for load balancing VMs in a cluster
         #                          If this is not a Virtual Machine, it returns the vm_name
-        def find_least_used_compatible_host(vm_name)
+        def find_least_used_compatible_host(_vm_name)
-          fail "#{self.class.name} does not implement find_least_used_compatible_host"
+          raise("#{self.class.name} does not implement find_least_used_compatible_host")
         end
 
         # inputs
@@ -45,8 +45,8 @@ module Vmpooler
         #   dest_host_name: string (Name of the host to migrate `vm_name` to)
         # returns
         #    [Boolean] Returns true on success or false on failure
-        def migrate_vm_to_host(vm_name, dest_host_name)
+        def migrate_vm_to_host(_vm_name, _dest_host_name)
-          fail "#{self.class.name} does not implement migrate_vm_to_host"
+          raise("#{self.class.name} does not implement migrate_vm_to_host")
         end
 
         # inputs
@@ -61,8 +61,8 @@ module Vmpooler
         #    [Time]   boottime   = Time when the VM was created/booted
         #    [String] powerstate = Current power state of a VM.  Valid values (as per vCenter API)
         #                            - 'PoweredOn','PoweredOff'
-        def get_vm(vm_name)
+        def get_vm(_vm_name)
-          fail "#{self.class.name} does not implement get_vm"
+          raise("#{self.class.name} does not implement get_vm")
         end
 
         # inputs
@@ -70,8 +70,8 @@ module Vmpooler
         #   new_vmname : string      Name the new VM should use
         # returns
         #   Hashtable of the VM as per get_vm
-        def create_vm(pool,new_vmname)
+        def create_vm(_pool, _new_vmname)
-          fail "#{self.class.name} does not implement create_vm"
+          raise("#{self.class.name} does not implement create_vm")
         end
 
         # inputs
@@ -79,8 +79,8 @@ module Vmpooler
         #   pool: string
         # returns
         #   boolean : true if success, false on error
-        def destroy_vm(vm_name,pool)
+        def destroy_vm(_vm_name, _pool)
-          fail "#{self.class.name} does not implement destroy_vm"
+          raise("#{self.class.name} does not implement destroy_vm")
         end
 
         # inputs
@@ -89,8 +89,8 @@ module Vmpooler
         # timeout: int (Seconds)
         # returns
         #   result: boolean
-        def is_vm_ready?(vm,pool,timeout)
+        def vm_ready?(_vm, _pool, _timeout)
-          fail "#{self.class.name} does not implement is_vm_ready?"
+          raise("#{self.class.name} does not implement vm_ready?")
         end
 
         # inputs
@@ -100,7 +100,6 @@ module Vmpooler
         def vm_exists?(vm)
           !get_vm(vm).nil?
         end
-
       end
     end
   end

lib/vmpooler/providers/vsphere.rb

@@ -2,7 +2,6 @@ module Vmpooler
   class PoolManager
     class Provider
       class VSphere < Vmpooler::PoolManager::Provider::Base
-
         def initialize(options)
           super(options)
         end
@@ -10,7 +9,6 @@ module Vmpooler
         def name
           'vsphere'
         end
-
       end
     end
   end

lib/vmpooler/statsd.rb

@@ -6,30 +6,30 @@ module Vmpooler
     attr_reader :server, :port, :prefix
 
     def initialize(params = {})
-      if params["server"].nil? || params["server"].empty?
+      if params['server'].nil? || params['server'].empty?
         raise ArgumentError, "Statsd server is required. Config: #{params.inspect}"
       end
 
-      host    = params["server"]
+      host    = params['server']
-      @port    = params["port"]   || 8125
+      @port   = params['port'] || 8125
-      @prefix = params["prefix"] || 'vmpooler'
+      @prefix = params['prefix'] || 'vmpooler'
       @server = ::Statsd.new(host, @port)
     end
 
     def increment(label)
-      server.increment(prefix + "." + label)
+      server.increment(prefix + '.' + label)
     rescue => err
       $stderr.puts "Failure incrementing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
     end
 
     def gauge(label, value)
-      server.gauge(prefix + "." + label, value)
+      server.gauge(prefix + '.' + label, value)
     rescue => err
       $stderr.puts "Failure updating gauge #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
     end
 
     def timing(label, duration)
-      server.timing(prefix + "." + label, duration)
+      server.timing(prefix + '.' + label, duration)
     rescue => err
       $stderr.puts "Failure updating timing #{prefix}.#{label} on statsd server [#{server}:#{port}]: #{err}"
     end

spec/unit/providers/base_spec.rb

@@ -66,9 +66,9 @@ describe 'Vmpooler::PoolManager::Provider::Base' do
     end
   end
 
-  describe '#is_vm_ready?' do
+  describe '#vm_ready?' do
     it 'should raise error' do
-      expect{subject.is_vm_ready?('vm','pool','timeout')}.to raise_error(/does not implement is_vm_ready?/)
+      expect{subject.vm_ready?('vm','pool','timeout')}.to raise_error(/does not implement vm_ready?/)
     end
   end
 

spec/unit/providers/vsphere_spec.rb

@@ -63,9 +63,9 @@ describe 'Vmpooler::PoolManager::Provider::VSphere' do
     end
   end
 
-  describe '#is_vm_ready?' do
+  describe '#vm_ready?' do
     it 'should raise error' do
-      expect{subject.is_vm_ready?('vm','pool','timeout')}.to raise_error(/does not implement is_vm_ready?/)
+      expect{subject.vm_ready?('vm','pool','timeout')}.to raise_error(/does not implement vm_ready?/)
     end
   end