Significant rubocop fixups.

This merge-request contains almost entirely mechanical changes,
with a few exceptions:

* I changed `do_ipv4` and `do_ipv6` to `ipv4` and `ipv6` respectively.
  * This fixed a warning about normal-casing.
* I changed a test-case to compare against both `Integer` and `Fixnum`
  * Suspect this is a ruby-versionism.

The tests continue to pass, so I believe this is safe to merge,
but of course it is still not 100%:

    lib/custodian/queue.rb:135:21: W: Assignment in condition - you probably meant to use ==.
              added = true
                    ^
    lib/custodian/protocoltest/ssl.rb:218:5: W: Do not shadow rescued Exceptions
        rescue OpenSSL::SSL::SSLError => err ...
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    lib/custodian/protocoltest/ssl.rb:286:5: W: Do not shadow rescued Exceptions
        rescue OpenSSL::SSL::SSLError => err ...
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    lib/custodian/protocoltest/http.rb:307:7: C: Assignment Branch Condition size for run_test is too high. [84.53/72]
          def run_test
          ^^^
    lib/custodian/protocoltest/http.rb:307:7: C: Cyclomatic complexity for run_test is too high. [22/19]
          def run_test
          ^^^
    lib/custodian/protocoltest/http.rb:307:7: C: Method has too many lines. [97/87]
          def run_test ...
          ^^^^^^^^^^^^
    lib/custodian/protocoltest/http.rb:307:7: C: Perceived complexity for run_test is too high. [23/21]
          def run_test

In short this takes care of _most_ of the warnings, but updates requiring
significant code-change have not been applied.

15 files changed, 67 insertions, 65 deletions

diff --git a/bin/multi-ping b/bin/multi-ping
index 6d905bb..1045635 100755
--- a/bin/multi-ping
+++ b/bin/multi-ping
@@ -14,7 +14,8 @@ manual = false
 
 opts = GetoptLong.new(
   ['--help',   '-h', GetoptLong::NO_ARGUMENT],
-  ['--manual', '-m', GetoptLong::NO_ARGUMENT])
+  ['--manual', '-m', GetoptLong::NO_ARGUMENT]
+)
 
 begin
   opts.each do |opt, _arg|
diff --git a/lib/custodian/alerts/mauve.rb b/lib/custodian/alerts/mauve.rb
index 0aed390..bf4d08f 100644
--- a/lib/custodian/alerts/mauve.rb
+++ b/lib/custodian/alerts/mauve.rb
@@ -110,7 +110,7 @@ module Custodian
         # If we're Monday-Friday, between the start & end time, then
         # we're in the working day.
         #
-        if  ((wday != 0) && (wday != 6)) &&
+        if  ((wday.nonzero?) && (wday != 6)) &&
             (hour >= day_start && hour < day_end)
           working = true
         end
@@ -229,7 +229,7 @@ module Custodian
         alert.id  = Digest::SHA1.hexdigest(id_key)
 
         # Look for a subject-prefix
-        subject_prefix = Custodian::Util::Prefix.text()
+        subject_prefix = Custodian::Util::Prefix.text
 
         alert.subject = subject_prefix + subject
         alert.summary = "The #{test_type} test failed against #{test_host}"
diff --git a/lib/custodian/parser.rb b/lib/custodian/parser.rb
index 941a549..0f00e0d 100644
--- a/lib/custodian/parser.rb
+++ b/lib/custodian/parser.rb
@@ -96,7 +96,7 @@ module Custodian
         case response
         when Net::HTTPRedirection
         then
-          newURL = (response['location'] =~ /^http/) ? response['Location'] : uri_str + response['Location']
+          newURL = response['location'] =~ /^http/ ? response['Location'] : uri_str + response['Location']
           return(get_url_contents(newURL))
         else
           return response.body
diff --git a/lib/custodian/protocoltest/http.rb b/lib/custodian/protocoltest/http.rb
index 0bf1a68..084e9aa 100644
--- a/lib/custodian/protocoltest/http.rb
+++ b/lib/custodian/protocoltest/http.rb
@@ -59,7 +59,7 @@ module Custodian
       #    raise an alert, but if it is this false-error then we
       #    will silently disable this test-run.
       #
-      def ignore_failure?( protocol )
+      def ignore_failure?(protocol)
 
         # Get the hostname we're connecting to.
         u = URI.parse(@url)
@@ -116,7 +116,7 @@ module Custodian
         #
         #   "ips" being empty because the DNS failure was genuine
         #
-        return ( ! ips.empty? )
+        return (!ips.empty?)
       end
 
 
@@ -189,8 +189,8 @@ module Custodian
         #
         #  Save username/password if they were specified
         #
-        @username = u.user if ( u.user )
-        @password = u.password if ( u.password )
+        @username = u.user if (u.user)
+        @password = u.password if (u.password)
 
         #
         # Expected status
@@ -278,7 +278,7 @@ module Custodian
       #  Do we have basic auth?
       #
       def basic_auth?
-        ( @username.nil? == false ) && ( @password.nil? == false )
+        (@username.nil? == false) && (@password.nil? == false)
       end
 
       #
@@ -341,7 +341,7 @@ module Custodian
         #  If we're running with HTTP-basic-auth we should remove
         # the username/password from the URL we're passing to curb.
         #
-        if ( basic_auth? )
+        if (basic_auth?)
           u = URI.parse(test_url)
           u.user     = nil
           u.password = nil
@@ -365,7 +365,7 @@ module Custodian
           #
           # Should we use HTTP basic-auth?
           #
-          if  basic_auth?
+          if basic_auth?
             c.http_auth_types = :basic
             c.username = basic_auth_username
             c.password = basic_auth_password
@@ -421,7 +421,7 @@ module Custodian
           rescue Curl::Err::TooManyRedirectsError
             errors << "#{protocol_msg}: More than 10 redirections."
           rescue Curl::Err::HostResolutionError => x
-            resolution_errors << "#{protocol_msg}: #{x.class}: #{x.message}\n  #{x.backtrace.join("\n  ")}." unless ignore_failure?( resolve_mode)
+            resolution_errors << "#{protocol_msg}: #{x.class}: #{x.message}\n  #{x.backtrace.join("\n  ")}." unless ignore_failure?(resolve_mode)
 
           rescue => x
             errors << "#{protocol_msg}: #{x.class}: #{x.message}\n  #{x.backtrace.join("\n  ")}."
diff --git a/lib/custodian/protocoltest/ping.rb b/lib/custodian/protocoltest/ping.rb
index 44da04d..3ee722e 100644
--- a/lib/custodian/protocoltest/ping.rb
+++ b/lib/custodian/protocoltest/ping.rb
@@ -112,17 +112,17 @@ module Custodian
         #
         #  Both types?
         #
-        do_ipv6 = true
-        do_ipv4 = true
+        ipv6 = true
+        ipv4 = true
 
         #
         # Allow the test to disable one/both
         #
         if @line =~ /ipv4_only/
-          do_ipv6 = false
+          ipv6 = false
         end
         if @line =~ /ipv6_only/
-          do_ipv4 = false
+          ipv4 = false
         end
 
         #
@@ -133,11 +133,11 @@ module Custodian
           timeout(period) do
 
             Resolv::DNS.open do |dns|
-              if do_ipv4
+              if ipv4
                 ress = dns.getresources(@host, Resolv::DNS::Resource::IN::A)
                 ress.map { |r| ips.push(r.address.to_s) }
               end
-              if do_ipv6
+              if ipv6
                 ress = dns.getresources(@host, Resolv::DNS::Resource::IN::AAAA)
                 ress.map { |r| ips.push(r.address.to_s) }
               end
diff --git a/lib/custodian/protocoltest/ssl.rb b/lib/custodian/protocoltest/ssl.rb
index 82754c6..6bc1a04 100644
--- a/lib/custodian/protocoltest/ssl.rb
+++ b/lib/custodian/protocoltest/ssl.rb
@@ -162,25 +162,25 @@ class SSLCheck
   # is valid.
   #
   def certificate_fallback
-    cert = ""
+    cert = ''
     in_cert = false
 
     # Run the command.
     out = `echo "" | openssl s_client -servername #{uri.host} -connect #{uri.host}:#{uri.port} 2>/dev/null`
     # For each line of the output
-    out.split( /[\r\n]/ ).each do |line|
+    out.split(/[\r\n]/).each do |line|
 
       # Are we in a certificate?
-      in_cert = true if ( line =~ /BEGIN CERT/ )
+      in_cert = true if (line =~ /BEGIN CERT/)
 
       # If so append the line.
-      if ( in_cert )
+      if (in_cert)
         cert += line
         cert += "\n"
       end
 
       # Are we at the end?
-      in_cert = false if ( line =~ /END CERT/ )
+      in_cert = false if (line =~ /END CERT/)
     end
 
     # Return the certificate
@@ -248,10 +248,10 @@ class SSLCheck
     if self.certificate.nil?
 
       # Use our fallback method.
-      fallback = certificate_fallback()
+      fallback = certificate_fallback
 
       # If we failed to fetch it then we cannot do anything useful.
-      if ( fallback.nil? )
+      if (fallback.nil?)
         self.errors << verbose("Failed to fetch certificate for #{self.domain}")
 	return nil
       else
@@ -373,9 +373,9 @@ class SSLCheck
     # signature is valid, because we're missing the bundle that
     # the remote server should have sent us.
     #
-    if ( @fallback )
+    if (@fallback)
       verbose "Skipping certificate signature validation for #{self.domain} because fallback SSL-certificate had to be used and we think we'll fail"
-      return true;
+      return true
     end
 
     unless self.tests.include?(:signature)
@@ -514,7 +514,7 @@ module Custodian
           return Custodian::TestResult::TEST_SKIPPED
         end
 
-        s = SSLCheck.new(@host,@expiry_days)
+        s = SSLCheck.new(@host, @expiry_days)
         result = s.verify
 
         if true == result
@@ -522,7 +522,7 @@ module Custodian
           return Custodian::TestResult::TEST_PASSED
         elsif result.nil?
           puts("SSL Verification returned no result #{@host}")
-          @error = "SSL Verification for #{@host} failed - TLS negotiation failure?\n";
+          @error = "SSL Verification for #{@host} failed - TLS negotiation failure?\n"
           @error += s.errors.join("\n")
           return Custodian::TestResult::TEST_FAILED
         else
diff --git a/lib/custodian/protocoltest/tcp.rb b/lib/custodian/protocoltest/tcp.rb
index 0081a18..c62303f 100644
--- a/lib/custodian/protocoltest/tcp.rb
+++ b/lib/custodian/protocoltest/tcp.rb
@@ -113,7 +113,7 @@ module Custodian
         # reset the error, in case we were previously executed.
         @error = nil
 
-        (run_test_internal(@host, @port, @banner, (! @banner.nil?)))
+        (run_test_internal(@host, @port, @banner, (!@banner.nil?)))
       end
 
 
@@ -165,17 +165,17 @@ module Custodian
         #
         #  Both types?
         #
-        do_ipv6 = true
-        do_ipv4 = true
+        ipv6 = true
+        ipv4 = true
 
         #
         # Allow the test to disable one/both
         #
         if @line =~ /ipv4_only/
-          do_ipv6 = false
+          ipv6 = false
         end
         if @line =~ /ipv6_only/
-          do_ipv4 = false
+          ipv4 = false
         end
 
 
@@ -188,11 +188,11 @@ module Custodian
 
             Resolv::DNS.open do |dns|
 
-              if do_ipv4
+              if ipv4
                 ress = dns.getresources(host, Resolv::DNS::Resource::IN::A)
                 ress.map { |r| ips.push(r.address.to_s) }
               end
-              if do_ipv6
+              if ipv6
                 ress = dns.getresources(host, Resolv::DNS::Resource::IN::AAAA)
                 ress.map { |r| ips.push(r.address.to_s) }
               end
diff --git a/lib/custodian/queue.rb b/lib/custodian/queue.rb
index 9c33825..ceb84b5 100644
--- a/lib/custodian/queue.rb
+++ b/lib/custodian/queue.rb
@@ -3,7 +3,7 @@
 #
 # Without this we cannot connect to our queue.
 #
-%w( redis ).each do |library|
+%w(redis).each do |library|
   begin
     require library
   rescue LoadError
diff --git a/lib/custodian/util/ping.rb b/lib/custodian/util/ping.rb
index 21836ac..f0aa15b 100644
--- a/lib/custodian/util/ping.rb
+++ b/lib/custodian/util/ping.rb
@@ -45,7 +45,7 @@ module Custodian
       # Does the hostname resolve to an IPv4 address?
       #
       def is_ipv4?
-        if (! @resolved.nil?) && (@resolved =~ /^([0-9]+).([0-9]+).([0-9]+).([0-9]+)$/)
+        if (!@resolved.nil?) && (@resolved =~ /^([0-9]+).([0-9]+).([0-9]+).([0-9]+)$/)
           true
         else
           false
@@ -57,7 +57,7 @@ module Custodian
       # Does the hostname resolve to an IPv6 address?
       #
       def is_ipv6?
-        if (! @resolved.nil?) && (@resolved =~ /^([a-f0-9:]+)$/i)
+        if (!@resolved.nil?) && (@resolved =~ /^([a-f0-9:]+)$/i)
           true
         else
           false
diff --git a/lib/custodian/util/prefix.rb b/lib/custodian/util/prefix.rb
index 743a422..9ac027c 100644
--- a/lib/custodian/util/prefix.rb
+++ b/lib/custodian/util/prefix.rb
@@ -14,25 +14,25 @@ module Custodian
       #
       # Return the custom-prefix to use, if any.
       #
-      def Prefix.text()
+      def Prefix.text
         # Default to no prefix.
-        default = ""
+        default = ''
 
         # Look for matches - last one wins.
-        Dir.glob( "/store/clients/*/custodian-prefix.cfg" ).each do |file|
+        Dir.glob('/store/clients/*/custodian-prefix.cfg').each do |file|
           begin
-            default = File.read( file )
+            default = File.read(file)
           rescue Errno::EACCES
             # Permission-denied.
           end
         end
 
         # Remove any newline characters
-        default.gsub!( /[\r\n]/, '' )
+        default.gsub!(/[\r\n]/, '')
 
         # Truncate, if required.
         max = 32
-        default = default[0...max] if ( default.length > max )
+        default = default[0...max] if (default.length > max)
 
         default
       end
diff --git a/t/test-custodian-parser.rb b/t/test-custodian-parser.rb
index 2239503..62a39dc 100755
--- a/t/test-custodian-parser.rb
+++ b/t/test-custodian-parser.rb
@@ -438,15 +438,15 @@ EOF
     # test data
     #
     data = {
-      'http://bob:steve@example must run http.'  =>
-      { username: 'bob', password: 'steve'},
+      'http://bob:steve@example must run http.' =>
+      { :username => 'bob', :password => 'steve' },
       'http://stee\':steve@example must run http.' =>
-      { username: 'stee\'', password: 'steve'},
+      { :username => 'stee\'', :password => 'steve' },
       'http://e\'e:pa$$w0rd@example must run http.' =>
-      { username: 'e\'e', password: 'pa$$w0rd'},
+      { :username => 'e\'e', :password => 'pa$$w0rd' }
     }
 
-    data.each do |str, hash |
+    data.each do |str, hash|
       assert_nothing_raised do
 
         #
@@ -461,8 +461,8 @@ EOF
 
         # There should be auth-enabled
         assert(obj[0].basic_auth?)
-        assert(obj[0].basic_auth_username == hash[:username] )
-        assert(obj[0].basic_auth_password == hash[:password] )
+        assert(obj[0].basic_auth_username == hash[:username])
+        assert(obj[0].basic_auth_password == hash[:password])
 
       end
     end
@@ -488,7 +488,7 @@ EOF
     #
     # Test the parser with this text
     #
-    expiries.each do |str,days|
+    expiries.each do |str, days|
       assert_nothing_raised do
 
         #
@@ -505,13 +505,13 @@ EOF
 
         # Test both of them to make sure we got our expiry period.
         obj.each do |x|
-          if ( x.class.name =~ /SSL/ )
-            found_days =  x.expiry_days
+          if (x.class.name =~ /SSL/)
+            found_days = x.expiry_days
           end
         end
 
         # Ensure we did find a match.
-        assert(found_days != -1 )
+        assert(found_days != -1)
         assert(found_days == days)
 
       end
diff --git a/t/test-custodian-settings.rb b/t/test-custodian-settings.rb
index 74f881b..1630859 100755
--- a/t/test-custodian-settings.rb
+++ b/t/test-custodian-settings.rb
@@ -47,36 +47,37 @@ class TestConfigurationSingleton < Test::Unit::TestCase
 
     # retry delay - probably unset.
     a = settings.retry_delay
-    assert(a.class == Fixnum)
+    assert(a.class == Integer ||
+           a.class == Fixnum)
 
     # store a number
     settings._store('retry_delay', 5)
     a = settings.retry_delay
-    assert(a.class == Fixnum)
+    assert(a.class == Integer || a.class == Fixnum )
     assert(a == 5)
 
     # store a string
     settings._store('retry_delay', '35')
     a = settings.retry_delay
-    assert(a.class == Fixnum)
+    assert(a.class == Integer || a.class == Fixnum )
     assert(a == 35)
 
 
 
     # timeout - probably unset.
     a = settings.timeout
-    assert(a.class == Fixnum)
+    assert(a.class == Integer || a.class == Fixnum )
 
     # store a number
     settings._store('timeout', 5)
     a = settings.timeout
-    assert(a.class == Fixnum)
+    assert(a.class == Integer || a.class == Fixnum )
     assert(a == 5)
 
     # store a string
     settings._store('timeout', '35')
     a = settings.timeout
-    assert(a.class == Fixnum)
+    assert(a.class == Integer || a.class == Fixnum )
     assert(a == 35)
 
 
diff --git a/t/test-custodian-util-ping.rb b/t/test-custodian-util-ping.rb
index 99b61c1..d7d16b8 100755
--- a/t/test-custodian-util-ping.rb
+++ b/t/test-custodian-util-ping.rb
@@ -63,7 +63,7 @@ class TestPingUtil < Test::Unit::TestCase
   #  Test lookup of hosts that don't work
   #
   def test_lookup_fail
-    %w( tessf.dfsdf.sdf.sdfsdf fdsfkljflj3.fdsfds.f3.dfs ).each do |name|
+    %w(tessf.dfsdf.sdf.sdfsdf fdsfkljflj3.fdsfds.f3.dfs).each do |name|
       assert_nothing_raised do
         helper = Custodian::Util::Ping.new(name)
 
diff --git a/t/test-custodian-util-timespan.rb b/t/test-custodian-util-timespan.rb
index fec079f..5098589 100755
--- a/t/test-custodian-util-timespan.rb
+++ b/t/test-custodian-util-timespan.rb
@@ -214,7 +214,7 @@ class TestTimeSpanUtil < Test::Unit::TestCase
   #
   def test_wrap_around
 
-    for h in 00..23
+    for h in 0o0..23
       assert_equal(1, Custodian::Util::TimeSpan.to_hours(h, h).size)
     end
 
diff --git a/t/test-rubocop.rb b/t/test-rubocop.rb
index 57457b7..cb597e8 100755
--- a/t/test-rubocop.rb
+++ b/t/test-rubocop.rb
@@ -20,7 +20,7 @@ class TestRubocop < Test::Unit::TestCase
 
       cli = RuboCop::CLI.new
       result = cli.run
-      assert(result == 0, 'No errors found')
+      assert(result.zero?, 'No errors found')
 
     rescue LoadError => ex
       if methods.include?(:skip)