summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSteve Kemp <steve@steve.org.uk>2017-08-10 10:17:51 +0300
committerSteve Kemp <steve@steve.org.uk>2017-08-10 10:17:51 +0300
commit63e2d2f2ec87d833408b7e8b6fb33e3f0fa0c804 (patch)
tree18cd4b93e09a5c56f3e47d0fd2693372e51634a7
parent005013d98d742989d3c000b04054e15bb0482a69 (diff)
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.
-rwxr-xr-xbin/multi-ping3
-rw-r--r--lib/custodian/alerts/mauve.rb4
-rw-r--r--lib/custodian/parser.rb2
-rw-r--r--lib/custodian/protocoltest/http.rb16
-rw-r--r--lib/custodian/protocoltest/ping.rb12
-rw-r--r--lib/custodian/protocoltest/ssl.rb22
-rw-r--r--lib/custodian/protocoltest/tcp.rb14
-rw-r--r--lib/custodian/queue.rb2
-rw-r--r--lib/custodian/util/ping.rb4
-rw-r--r--lib/custodian/util/prefix.rb12
-rwxr-xr-xt/test-custodian-parser.rb22
-rwxr-xr-xt/test-custodian-settings.rb13
-rwxr-xr-xt/test-custodian-util-ping.rb2
-rwxr-xr-xt/test-custodian-util-timespan.rb2
-rwxr-xr-xt/test-rubocop.rb2
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)