From 9366c1eda2967d6931efd6e73134dc79fb8a5cd2 Mon Sep 17 00:00:00 2001 From: Steve Kemp Date: Fri, 22 Apr 2016 22:05:25 +0300 Subject: Updated to fix the last remaining rubocop warnings. This involved silencing a few issues that were judged to be minor, and changing various whitespaces and function-calls. The most obvious example was changing this: assert(ret.kind_of? Array) To this: assert(ret.kind_of?(Array)) --- .rubocop.yml | 15 +++++++- lib/custodian/parser.rb | 10 +++--- t/test-custodian-alertfactory.rb | 5 ++- t/test-custodian-parser.rb | 76 +++++++++++++++++++-------------------- t/test-custodian-queue.rb | 6 ++-- t/test-custodian-testfactory.rb | 24 ++++++------- t/test-custodian-util-bytemark.rb | 4 +-- t/test-custodian-util-tftp.rb | 2 +- t/test-custodian-util-timespan.rb | 2 +- t/test-http-vs-https.rb | 5 ++- t/test-ldap-probe.rb | 4 +-- 11 files changed, 82 insertions(+), 71 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6fb8719..9e32091 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -48,7 +48,7 @@ Lint/UnreachableCode: # Offense count: 2 # Cop supports --auto-correct. Lint/UnusedBlockArgument: - Enabled: true + Enabled: false # Offense count: 3 # Cop supports --auto-correct. @@ -453,3 +453,16 @@ Style/VariableName: # Configuration parameters: WordRegex. Style/WordArray: MinSize: 2 + +Style/BlockDelimiters: + Enabled: false + + +Style/WordArray: + Enabled: false + +Style/MutableConstant: + Enabled: false + +Style/WhileUntilDo: + Enabled: false diff --git a/lib/custodian/parser.rb b/lib/custodian/parser.rb index ae771db..941a549 100644 --- a/lib/custodian/parser.rb +++ b/lib/custodian/parser.rb @@ -65,7 +65,7 @@ module Custodian # def get_url_contents(uri_str) begin - uri_str = 'http://' + uri_str unless uri_str.match(/^http/) + uri_str = 'http://' + uri_str unless uri_str =~ /^http/ url = URI.parse(uri_str) http = Net::HTTP.new(url.host, url.port) @@ -96,7 +96,7 @@ module Custodian case response when Net::HTTPRedirection then - newURL = response['location'].match(/^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 @@ -138,7 +138,7 @@ module Custodian text = get_url_contents(uri) text.split(/[\r\n]/).each do |line| - val.push(line) if line.length > 0 + val.push(line) if !line.empty? end elsif line =~ /\s(is|are)\s+(.*)\.*$/ @@ -245,7 +245,7 @@ module Custodian # # A blank line, or a comment may be skipped. # - return nil if (line.nil?) || (line =~ /^#/) || (line.length < 1) + return nil if (line.nil?) || (line =~ /^#/) || (line.empty?) # # Look for a time period. @@ -411,7 +411,7 @@ module Custodian def parse_file(filename) raise ArgumentError, 'Missing configuration file!' if filename.nil? - raise ArgumentError, "File not found: #{@file}" unless File.exist?(filename) + raise ArgumentError, "File not found: #{@file}" unless File.exist?(filename) # # Read the configuration file. diff --git a/t/test-custodian-alertfactory.rb b/t/test-custodian-alertfactory.rb index 3138fa6..cceec2b 100755 --- a/t/test-custodian-alertfactory.rb +++ b/t/test-custodian-alertfactory.rb @@ -75,8 +75,8 @@ class TestAlertFactory < Test::Unit::TestCase # Ensure that the object implements the raise() + clear() # methods we mandate. # - assert(obj.respond_to? 'raise') - assert(obj.respond_to? 'clear') + assert(obj.respond_to?('raise')) + assert(obj.respond_to?('clear')) end @@ -100,4 +100,3 @@ class TestAlertFactory < Test::Unit::TestCase end end - diff --git a/t/test-custodian-parser.rb b/t/test-custodian-parser.rb index bc2bc2b..cfecbb3 100755 --- a/t/test-custodian-parser.rb +++ b/t/test-custodian-parser.rb @@ -84,7 +84,7 @@ class TestCustodianParser < Test::Unit::TestCase # 1.c. Adding a test will return an array of test-objects. result = parser.parse_line("smtp.bytemark.co.uk must run smtp on 25 otherwise 'failure'.") assert(!result.nil?) - assert(result.kind_of? Array) + assert(result.kind_of?(Array)) assert(result.size == 1) @@ -101,7 +101,7 @@ class TestCustodianParser < Test::Unit::TestCase tmp = [] tmp.push("smtp.bytemark.co.uk must run ssh on 22 otherwise 'oops'.") ret = parser.parse_lines(tmp) - assert(ret.kind_of? Array) + assert(ret.kind_of?(Array)) assert(ret.size == 1) # @@ -121,7 +121,7 @@ smtp.bytemark.co.uk must run smtp on 25. google.com must run ping otherwise 'internet broken?'. EOF ret = parser.parse_lines(str) - assert(ret.kind_of? Array) + assert(ret.kind_of?(Array)) assert(ret.size == 1) end @@ -184,33 +184,33 @@ EOF # We should now have two macros. # macros = parser.macros - assert(!macros.empty?, "We found some macros") - assert(macros.size == 2, "We found two macros") + assert(!macros.empty?, 'We found some macros') + assert(macros.size == 2, 'We found two macros') # # Ensure they were defined. # - assert( parser.is_macro?( "ONE" ), "The macro ONE exists" ) - assert( parser.is_macro?( "TWO" ), "The macro TWO exists" ) + assert(parser.is_macro?('ONE'), 'The macro ONE exists') + assert(parser.is_macro?('TWO'), 'The macro TWO exists') # # Ensure we can get the values. # - one = parser.get_macro_targets("ONE") - two = parser.get_macro_targets("TWO") - assert( one.kind_of? Array ) - assert( one.size() == 2, "Both targets are in the macro" ) - assert( one.find_index( "kvm1.vm.bytemark.co.uk" ) >= 0 , - "We found the expected host: kvm1") - assert( one.find_index( "kvm2.vm.bytemark.co.uk" ) >= 0 , - "We found the expected host: kvm2") - - assert( two.kind_of? Array ) - assert( two.size() == 2, "Both targets are in the macro" ) - assert( two.find_index( "kvm1.vm.bytemark.co.uk" ) >= 0 , - "We found the expected host: kvm1") - assert( two.find_index( "kvm2.vm.bytemark.co.uk" ) >= 0 , - "We found the expected host: kvm2") + one = parser.get_macro_targets('ONE') + two = parser.get_macro_targets('TWO') + assert(one.kind_of?(Array)) + assert(one.size == 2, 'Both targets are in the macro') + assert(one.find_index('kvm1.vm.bytemark.co.uk') >= 0, + 'We found the expected host: kvm1') + assert(one.find_index('kvm2.vm.bytemark.co.uk') >= 0, + 'We found the expected host: kvm2') + + assert(two.kind_of?(Array)) + assert(two.size == 2, 'Both targets are in the macro') + assert(two.find_index('kvm1.vm.bytemark.co.uk') >= 0, + 'We found the expected host: kvm1') + assert(two.find_index('kvm2.vm.bytemark.co.uk') >= 0, + 'We found the expected host: kvm2') end @@ -301,7 +301,7 @@ EOF # # The difference is the return value will be an array # - assert(out_txt.kind_of? Array) + assert(out_txt.kind_of?(Array)) assert(out_txt.size == 1) assert(out_txt[0] == in_txt) @@ -321,7 +321,7 @@ EOF # # The result should be an array # - assert(ret.kind_of? Array) + assert(ret.kind_of?(Array)) assert_equal(ret.size, 2) assert(ret[0] =~ /example1/) assert(ret[1] =~ /example2/) @@ -343,7 +343,7 @@ EOF "http://example must run http with content 'bar'." => true, 'http://example must run http following redirects.' => true, 'http://example must run http not following redirects.' => false, - 'http://example must run http not following redirect.' => false, + 'http://example must run http not following redirect.' => false } data.each do |str, follow| @@ -355,11 +355,11 @@ EOF obj = Custodian::TestFactory.create(str) assert(!obj.nil?) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(obj.size == 1) assert_equal(obj[0].to_s, str) - if follow + if follow assert(obj[0].follow_redirects?) else assert(!obj[0].follow_redirects?) @@ -373,18 +373,18 @@ EOF # def test_http_with_content_parsing content_strings = { - "'bar in single quotes'" => 'bar in single quotes', - '"bar in double quotes"' => "bar in double quotes", - "'bar in single quotes with \"embedded double quotes\"'" => 'bar in single quotes with "embedded double quotes"', - '"bar in double quotes with \'embedded double quotes\'"' => "bar in double quotes with 'embedded double quotes'", - "'bar testing greediness' with host header 'but dont be greedy'" => "bar testing greediness", + "'bar in single quotes'" => 'bar in single quotes', + '"bar in double quotes"' => 'bar in double quotes', + "'bar in single quotes with \"embedded double quotes\"'" => 'bar in single quotes with "embedded double quotes"', + '"bar in double quotes with \'embedded double quotes\'"' => "bar in double quotes with 'embedded double quotes'", + "'bar testing greediness' with host header 'but dont be greedy'" => 'bar testing greediness' } content_strings.each do |cs, ex| str = "http://example must run http with content #{cs}." obj = Custodian::TestFactory.create(str) assert(!obj.nil?) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(obj.size == 1) assert_equal(obj[0].to_s, str) @@ -405,7 +405,7 @@ EOF 'http://example must run http.' => true, 'http://example must run http with status 200.' => true, "http://example must run http with content 'bar'." => true, - 'http://example must run http without cache busting.' => false, + 'http://example must run http without cache busting.' => false } data.each do |str, cb| @@ -417,11 +417,11 @@ EOF obj = Custodian::TestFactory.create(str) assert(!obj.nil?) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(obj.size == 1) assert_equal(obj[0].to_s, str) - if cb + if cb assert(obj[0].cache_busting?) else assert(!obj[0].cache_busting?) @@ -490,11 +490,11 @@ EOF obj = Custodian::TestFactory.create(str) assert(!obj.nil?) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(obj.size == 1) assert_equal(obj[0].to_s, str) - if fail.nil? + if fail.nil? assert(obj[0].get_notification_text.nil?) else assert_equal(obj[0].get_notification_text, fail) diff --git a/t/test-custodian-queue.rb b/t/test-custodian-queue.rb index c25fc24..626eb8b 100755 --- a/t/test-custodian-queue.rb +++ b/t/test-custodian-queue.rb @@ -21,9 +21,9 @@ class TestCustodianQueue < Test::Unit::TestCase def setup unless defined? ::Redis if methods.include? :skip - skip("Redis library missing -- skipping tests") + skip('Redis library missing -- skipping tests') else - omit("Redis library missing -- skipping tests") + omit('Redis library missing -- skipping tests') end end end @@ -45,7 +45,7 @@ class TestCustodianQueue < Test::Unit::TestCase q = nil assert_nothing_raised do - q = Custodian::RedisQueueType.new() + q = Custodian::RedisQueueType.new end # diff --git a/t/test-custodian-testfactory.rb b/t/test-custodian-testfactory.rb index fa5129f..5dbc675 100755 --- a/t/test-custodian-testfactory.rb +++ b/t/test-custodian-testfactory.rb @@ -63,7 +63,7 @@ class TestTestFactory < Test::Unit::TestCase 'foo must run ftp.' => '21', 'ftp://ftp.example.com/ must run ftp.' => '21', "foo must run ftp on 1 otherwise 'x'." => '1', - 'foo must run ftp on 33 otherwise' => '33', + 'foo must run ftp on 33 otherwise' => '33' } # @@ -74,7 +74,7 @@ class TestTestFactory < Test::Unit::TestCase obj = Custodian::TestFactory.create(str) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(!obj.empty?) assert_equal(obj[0].get_type, 'ftp') assert_equal(obj[0].port.to_s, prt) @@ -98,9 +98,9 @@ class TestTestFactory < Test::Unit::TestCase "foo must run rEDIs otherwise 'alert'" => '6379', "foo must run rdp otherwise 'alert'" => '3389', "foo must run RDP otherwise 'alert'" => '3389', - "foo must run tcp on 22 otherwise 'alert'" => '22', - "foo must run tcp on port 22 otherwise 'alert'" => '22', - "foo must run mysql on 33 otherwise 'alert'" => '33', + "foo must run tcp on 22 otherwise 'alert'" => '22', + "foo must run tcp on port 22 otherwise 'alert'" => '22', + "foo must run mysql on 33 otherwise 'alert'" => '33' } # @@ -112,7 +112,7 @@ class TestTestFactory < Test::Unit::TestCase obj = Custodian::TestFactory.create(str) assert(obj) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(!obj.empty?) assert(obj[0].port.to_s == prt, "'#{str}' gave expected port '#{prt}'.") end @@ -148,7 +148,7 @@ class TestTestFactory < Test::Unit::TestCase 'foo must run rsync.' => '873', 'rsync://foo/ must run rsync.' => '873', "foo must run rsync on 1 otherwise 'x'." => '1', - 'foo must run rsync on 33 otherwise' => '33', + 'foo must run rsync on 33 otherwise' => '33' } # @@ -159,7 +159,7 @@ class TestTestFactory < Test::Unit::TestCase obj = Custodian::TestFactory.create(str) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(!obj.empty?) assert(obj[0].port.to_s == prt, "'#{str}' gave expected port '#{prt}'.") end @@ -224,7 +224,7 @@ class TestTestFactory < Test::Unit::TestCase 'rsync://foo/ must run rsync.' => false, 'foo must run ping otherwise' => false, 'foo must not run ping otherwise' => true, - 'foo must not run ssh otherwise' => true, + 'foo must not run ssh otherwise' => true } # @@ -235,7 +235,7 @@ class TestTestFactory < Test::Unit::TestCase obj = Custodian::TestFactory.create(str) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(!obj.empty?) # @@ -260,7 +260,7 @@ class TestTestFactory < Test::Unit::TestCase # for each handler .. registered[type].each do |name| - if name.to_s =~ /protocoltest::(.*)Test$/i + if name.to_s =~ /protocoltest::(.*)Test$/i tst = $1.dup.downcase # @@ -309,7 +309,7 @@ class TestTestFactory < Test::Unit::TestCase assert_nothing_raised do obj = Custodian::TestFactory.create(entry) assert(obj) - assert(obj.kind_of? Array) + assert(obj.kind_of?(Array)) assert(!obj.empty?) assert_equal('test.host.example.com', obj[0].target) end diff --git a/t/test-custodian-util-bytemark.rb b/t/test-custodian-util-bytemark.rb index 9bd775a..47fce9b 100755 --- a/t/test-custodian-util-bytemark.rb +++ b/t/test-custodian-util-bytemark.rb @@ -46,13 +46,13 @@ class TestBytemarkUtil < Test::Unit::TestCase # '127.0.0.1' => false, '192.168.1.1' => false, - '2a00:1450:400c:c00::93' => false, + '2a00:1450:400c:c00::93' => false } to_test.each do |name, inside| - if inside + if inside assert(Custodian::Util::Bytemark.inside?(name) == true) else assert(Custodian::Util::Bytemark.inside?(name) == false) diff --git a/t/test-custodian-util-tftp.rb b/t/test-custodian-util-tftp.rb index 0774124..a471226 100755 --- a/t/test-custodian-util-tftp.rb +++ b/t/test-custodian-util-tftp.rb @@ -92,7 +92,7 @@ class TestTftpUtil < Test::Unit::TestCase # def test_tftp_failed_fetch helper = Custodian::Util::Tftp.new('foo') - def helper.tftp(args) + def helper.tftp(_args) return false end diff --git a/t/test-custodian-util-timespan.rb b/t/test-custodian-util-timespan.rb index 3ac37a6..fec079f 100755 --- a/t/test-custodian-util-timespan.rb +++ b/t/test-custodian-util-timespan.rb @@ -74,7 +74,7 @@ class TestTimeSpanUtil < Test::Unit::TestCase # # Valid hours are 0-23, inclusive. Test outside that range. # - for i in 24..100 + for i in 24..100 assert_raise ArgumentError do result = Custodian::Util::TimeSpan.inside?(i, 2) end diff --git a/t/test-http-vs-https.rb b/t/test-http-vs-https.rb index 1905b0c..3612e28 100755 --- a/t/test-http-vs-https.rb +++ b/t/test-http-vs-https.rb @@ -46,7 +46,7 @@ class TestTestName < Test::Unit::TestCase end assert(test) - assert(test.kind_of? Array) + assert(test.kind_of?(Array)) assert(!test.empty?) assert_equal(test[0].get_type, 'http') end @@ -63,7 +63,7 @@ class TestTestName < Test::Unit::TestCase end assert(test) - assert(test.kind_of? Array) + assert(test.kind_of?(Array)) assert(!test.empty?) assert_equal(test[0].get_type, 'https') end @@ -94,4 +94,3 @@ class TestTestName < Test::Unit::TestCase end end - diff --git a/t/test-ldap-probe.rb b/t/test-ldap-probe.rb index 9e537ac..6564d93 100755 --- a/t/test-ldap-probe.rb +++ b/t/test-ldap-probe.rb @@ -32,7 +32,7 @@ class TestLDAPProbe < Test::Unit::TestCase test = Custodian::TestFactory.create("auth.bytemark.co.uk must run ldap on 389 with username 'testing' with password 'bob' otherwise 'LDAP dead?'.") end - assert(test.kind_of? Array) + assert(test.kind_of?(Array)) assert(!test.empty?) assert_equal(test[0].get_type, 'ldap') end @@ -59,7 +59,7 @@ class TestLDAPProbe < Test::Unit::TestCase assert_raise ArgumentError do test = Custodian::TestFactory.create(str) - assert(test.kind_of? Array) + assert(test.kind_of?(Array)) assert(!test.empty?) end -- cgit v1.2.1