From: Tom Hughes Date: Mon, 16 Feb 2015 22:44:30 +0000 (+0000) Subject: Fix rubocop lint issues X-Git-Tag: live~4274 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/baf10cd39289cd7e94a819305e46f43e85a136c6 Fix rubocop lint issues --- diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 75f088e1a..b857f5495 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -18,52 +18,14 @@ Lint/AmbiguousRegexpLiteral: Lint/AssignmentInCondition: Enabled: false -# Offense count: 1 -# Configuration parameters: AlignWith, SupportedStyles. -Lint/DefEndAlignment: - Enabled: false - -# Offense count: 2 -Lint/DuplicateMethods: - Enabled: false - # Offense count: 5 -# Configuration parameters: AlignWith, SupportedStyles. -Lint/EndAlignment: - Enabled: false - -# Offense count: 8 Lint/HandleExceptions: Enabled: false -# Offense count: 1 -Lint/LiteralInCondition: - Enabled: false - # Offense count: 8 Lint/ParenthesesAsGroupedExpression: Enabled: false -# Offense count: 18 -Lint/RescueException: - Enabled: false - -# Offense count: 3 -Lint/ShadowingOuterLocalVariable: - Enabled: false - -# Offense count: 2 -Lint/UselessAccessModifier: - Enabled: false - -# Offense count: 48 -Lint/UselessAssignment: - Enabled: false - -# Offense count: 2 -Lint/Void: - Enabled: false - # Offense count: 546 Metrics/AbcSize: Max: 194 diff --git a/app/controllers/amf_controller.rb b/app/controllers/amf_controller.rb index 75dc16458..aa2156e8e 100644 --- a/app/controllers/amf_controller.rb +++ b/app/controllers/amf_controller.rb @@ -114,7 +114,7 @@ class AmfController < ApplicationController return [-2, "Sorry - I can't get the map for that area. The server said: #{ex}"] rescue OSM::APIError => ex return [-1, ex.to_s] - rescue Exception => ex + rescue StandardError => ex return [-2, "An unusual error happened (in #{call}). The server said: #{ex}"] end @@ -375,6 +375,7 @@ class AmfController < ApplicationController rescue ArgumentError # thrown by date parsing method. leave old_way as nil for # the error handler below. + old_way = nil end end @@ -691,7 +692,7 @@ class AmfController < ApplicationController new_node.id = id.to_i begin node.delete_with_history!(new_node, user) - rescue OSM::APIPreconditionFailedError => ex + rescue OSM::APIPreconditionFailedError # We don't do anything here as the node is being used elsewhere # and we don't want to delete it end @@ -827,7 +828,7 @@ class AmfController < ApplicationController begin node.delete_with_history!(new_node, user) nodeversions[node.id] = node.version - rescue OSM::APIPreconditionFailedError => ex + rescue OSM::APIPreconditionFailedError # We don't do anything with the exception as the node is in use # elsewhere and we don't want to delete it end diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 1b9a1ad69..659d35477 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -25,7 +25,7 @@ class ApiController < ApplicationController bbox = BoundingBox.from_bbox_params(params) bbox.check_boundaries bbox.check_size - rescue Exception => err + rescue StandardError => err report_error(err.message) return end @@ -121,7 +121,7 @@ class ApiController < ApplicationController bbox = BoundingBox.from_bbox_params(params) bbox.check_boundaries bbox.check_size - rescue Exception => err + rescue StandardError => err report_error(err.message) return end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index e3f0391d1..5ef50eb3f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -24,13 +24,13 @@ class ApplicationController < ActionController::Base else redirect_to :controller => "user", :action => "terms", :referer => request.fullpath end - end + end elsif session[:token] if @user = User.authenticate(:token => session[:token]) session[:user] = @user.id end end - rescue Exception => ex + rescue StandardError => ex logger.info("Exception authorizing user: #{ex}") reset_session @user = nil @@ -345,7 +345,7 @@ class ApplicationController < ActionController::Base report_error ex.message, ex.status rescue AbstractController::ActionNotFound => ex raise - rescue Exception => ex + rescue StandardError => ex logger.info("API threw unexpected #{ex.class} exception: #{ex.message}") ex.backtrace.each { |l| logger.info(l) } report_error "#{ex.class}: #{ex.message}", :internal_server_error @@ -441,7 +441,7 @@ class ApplicationController < ActionController::Base @user.preferred_editor else DEFAULT_EDITOR - end + end if request.env['HTTP_USER_AGENT'] =~ /MSIE|Trident/ && editor == 'id' editor = 'potlatch2' diff --git a/app/controllers/geocoder_controller.rb b/app/controllers/geocoder_controller.rb index 25042c474..0d39cdf0a 100644 --- a/app/controllers/geocoder_controller.rb +++ b/app/controllers/geocoder_controller.rb @@ -73,7 +73,7 @@ class GeocoderController < ApplicationController end render :action => "results" - rescue Exception => ex + rescue StandardError => ex @error = "Error contacting rpc.geocoder.us: #{ex}" render :action => "error" end @@ -99,7 +99,7 @@ class GeocoderController < ApplicationController end render :action => "results" - rescue Exception => ex + rescue StandardError => ex @error = "Error contacting www.npemap.org.uk: #{ex}" render :action => "error" end @@ -121,7 +121,7 @@ class GeocoderController < ApplicationController end render :action => "results" - rescue Exception => ex + rescue StandardError => ex @error = "Error contacting geocoder.ca: #{ex}" render :action => "error" end @@ -188,7 +188,7 @@ class GeocoderController < ApplicationController end render :action => "results" - # rescue Exception => ex + # rescue StandardError => ex # @error = "Error contacting nominatim.openstreetmap.org: #{ex.to_s}" # render :action => "error" end @@ -219,7 +219,7 @@ class GeocoderController < ApplicationController end render :action => "results" - rescue Exception => ex + rescue StandardError => ex @error = "Error contacting ws.geonames.org: #{ex}" render :action => "error" end @@ -251,7 +251,7 @@ class GeocoderController < ApplicationController end render :action => "results" - rescue Exception => ex + rescue StandardError => ex @error = "Error contacting nominatim.openstreetmap.org: #{ex}" render :action => "error" end @@ -281,7 +281,7 @@ class GeocoderController < ApplicationController end render :action => "results" - rescue Exception => ex + rescue StandardError => ex @error = "Error contacting ws.geonames.org: #{ex}" render :action => "error" end diff --git a/app/controllers/swf_controller.rb b/app/controllers/swf_controller.rb index 541185d03..92a70457e 100644 --- a/app/controllers/swf_controller.rb +++ b/app/controllers/swf_controller.rb @@ -102,10 +102,10 @@ class SwfController < ApplicationController # Line-drawing def startShape - s = 0.chr # No fill styles - s += 2.chr # Two line styles - s += packUI16(0) + 0.chr + 255.chr + 255.chr # Width 5, RGB #00FFFF - s += packUI16(0) + 255.chr + 0.chr + 255.chr # Width 5, RGB #FF00FF + s = 0.chr # No fill styles + s += 2.chr # Two line styles + s += packUI16(0) + 0.chr + 255.chr + 255.chr # Width 5, RGB #00FFFF + s += packUI16(0) + 255.chr + 0.chr + 255.chr # Width 5, RGB #FF00FF s += 34.chr # 2 fill, 2 line index bits s end @@ -115,10 +115,11 @@ class SwfController < ApplicationController end def startAndMove(x, y, col) - d = '001001' # Line style change, moveTo + d = '001001' # Line style change, moveTo l = [lengthSB(x), lengthSB(y)].max d += sprintf("%05b%0#{l}b%0#{l}b", l, x, y) - d += col # Select line style + d += col # Select line style + d end def drawTo(absx, absy, x, y) @@ -131,7 +132,7 @@ class SwfController < ApplicationController xstep = dx / mstep ystep = dy / mstep d = '' - for i in (1..mstep) + 1.upto(mstep).each do d += drawSection(x, y, x + xstep, y + ystep) x += xstep y += ystep @@ -147,6 +148,7 @@ class SwfController < ApplicationController d += sprintf("%04b", l - 2) d += '1' # GeneralLine d += sprintf("%0#{l}b%0#{l}b", dx, dy) + d end # ----------------------------------------------------------------------- diff --git a/app/controllers/trace_controller.rb b/app/controllers/trace_controller.rb index 251c92a68..1b3f6355c 100644 --- a/app/controllers/trace_controller.rb +++ b/app/controllers/trace_controller.rb @@ -372,7 +372,7 @@ class TraceController < ApplicationController # Rename the temporary file to the final name FileUtils.mv(filename, @trace.trace_name) - rescue Exception => ex + rescue StandardError # Remove the file as we have failed to update the database FileUtils.rm_f(filename) @@ -384,7 +384,7 @@ class TraceController < ApplicationController # Clear the inserted flag to make the import daemon load the trace @trace.inserted = false @trace.save! - rescue Exception => ex + rescue StandardError # Remove the file as we have failed to update the database FileUtils.rm_f(@trace.trace_name) diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index b5bf30645..51978307a 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -64,7 +64,7 @@ module UserHelper size = options[:size] || 100 hash = Digest::MD5.hexdigest(user.email.downcase) default_image_url = image_url("users/images/large.png") - url = "#{request.protocol}www.gravatar.com/avatar/#{hash}.jpg?s=#{size}&d=#{u(default_image_url)}" + "#{request.protocol}www.gravatar.com/avatar/#{hash}.jpg?s=#{size}&d=#{u(default_image_url)}" end def user_gravatar_tag(user, options = {}) diff --git a/app/models/acl.rb b/app/models/acl.rb index 0c8996828..8bb4ae4b9 100644 --- a/app/models/acl.rb +++ b/app/models/acl.rb @@ -1,9 +1,9 @@ class Acl < ActiveRecord::Base def self.match(address, domain = nil) if domain - condition = Acl.where("address >>= ? OR domain = ?", address, domain) + Acl.where("address >>= ? OR domain = ?", address, domain) else - condition = Acl.where("address >>= ?", address) + Acl.where("address >>= ?", address) end end diff --git a/app/models/client_application.rb b/app/models/client_application.rb index 6195dfc2e..b1d3d945e 100644 --- a/app/models/client_application.rb +++ b/app/models/client_application.rb @@ -31,7 +31,7 @@ class ClientApplication < ActiveRecord::Base return false unless OauthNonce.remember(signature.request.nonce, signature.request.timestamp) value = signature.verify value - rescue OAuth::Signature::UnknownSignatureMethod => e + rescue OAuth::Signature::UnknownSignatureMethod false end diff --git a/app/models/message.rb b/app/models/message.rb index 8ca1dc5bc..65c2d29ed 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -22,7 +22,7 @@ class Message < ActiveRecord::Base body = mail.decoded end - message = Message.new( + Message.new( :sender => from, :recipient => to, :sent_on => mail.date.new_offset(0), diff --git a/app/models/relation.rb b/app/models/relation.rb index c59ae47d1..8f2d8b241 100644 --- a/app/models/relation.rb +++ b/app/models/relation.rb @@ -129,7 +129,7 @@ class Relation < ActiveRecord::Base member_el['ref'] = member.member_id.to_s member_el['role'] = member.member_role el << member_el - end + end end add_tags_to_xml_node(el, relation_tags) diff --git a/app/models/user.rb b/app/models/user.rb index 12afbb24b..f6d7ee78d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -138,7 +138,6 @@ class User < ActiveRecord::Base def nearby(radius = NEARBY_RADIUS, num = NEARBY_USERS) if home_lon && home_lat gc = OSM::GreatCircle.new(home_lat, home_lon) - bounds = gc.bounds(radius) sql_for_distance = gc.sql_for_distance("home_lat", "home_lon") nearby = User.where("id != ? AND status IN (\'active\', \'confirmed\') AND data_public = ? AND #{sql_for_distance} <= ?", id, true, radius).order(sql_for_distance).limit(num) else @@ -212,8 +211,8 @@ class User < ActiveRecord::Base def spam_score changeset_score = changesets.size * 50 trace_score = traces.size * 50 - diary_entry_score = diary_entries.inject(0) { |s, e| s += e.body.spam_score } - diary_comment_score = diary_comments.inject(0) { |s, c| s += c.body.spam_score } + diary_entry_score = diary_entries.inject(0) { |s, e| s + e.body.spam_score } + diary_comment_score = diary_comments.inject(0) { |s, c| s + c.body.spam_score } score = description.spam_score / 4.0 score += diary_entries.where("created_at > ?", 1.day.ago).count * 10 diff --git a/app/models/way.rb b/app/models/way.rb index 96422a8f6..e61661a70 100644 --- a/app/models/way.rb +++ b/app/models/way.rb @@ -86,9 +86,9 @@ class Way < ActiveRecord::Base # You can't pull in all the tags too unless we put a sequence_id on the way_tags table and have a multipart key def self.find_eager(id) - way = Way.find(id, :include => { :way_nodes => :node }) + Way.find(id, :include => { :way_nodes => :node }) # If waytag had a multipart key that was real, you could do this: - # way = Way.find(id, :include => [:way_tags, {:way_nodes => :node}]) + # Way.find(id, :include => [:way_tags, {:way_nodes => :node}]) end # Find a way given it's ID, and in a single SQL call also grab its nodes and tags diff --git a/db/migrate/020_populate_node_tags_and_remove.rb b/db/migrate/020_populate_node_tags_and_remove.rb index 8315cb143..6aa0bc44b 100644 --- a/db/migrate/020_populate_node_tags_and_remove.rb +++ b/db/migrate/020_populate_node_tags_and_remove.rb @@ -48,6 +48,7 @@ class PopulateNodeTagsAndRemove < ActiveRecord::Migration if have_nodes execute "LOAD DATA INFILE '#{nodes}' INTO TABLE nodes #{csvopts} (id, latitude, longitude, user_id, visible, timestamp, tile, version)" execute "LOAD DATA INFILE '#{node_tags}' INTO TABLE node_tags #{csvopts} (id, version, k, v)" + execute "LOAD DATA INFILE '#{current_nodes}' INTO TABLE current_nodes #{csvopts} (id, latitude, longitude, user_id, visible, timestamp, tile)" execute "LOAD DATA INFILE '#{current_node_tags}' INTO TABLE current_node_tags #{csvopts} (id, k, v)" end diff --git a/lib/bounding_box.rb b/lib/bounding_box.rb index 282a6a840..e78434263 100644 --- a/lib/bounding_box.rb +++ b/lib/bounding_box.rb @@ -1,8 +1,6 @@ class BoundingBox attr_reader :min_lon, :min_lat, :max_lon, :max_lat - private - LON_LIMIT = 180.0 LAT_LIMIT = 90.0 SCALED_LON_LIMIT = LON_LIMIT * GeoRecord::SCALE diff --git a/lib/country.rb b/lib/country.rb index 41e36ccec..16eec1a36 100644 --- a/lib/country.rb +++ b/lib/country.rb @@ -13,8 +13,6 @@ class Country countries[code] end - private - def self.countries @@countries ||= load_countries end diff --git a/lib/daemons/gpx_import.rb b/lib/daemons/gpx_import.rb index c257c1bfc..3679aa448 100755 --- a/lib/daemons/gpx_import.rb +++ b/lib/daemons/gpx_import.rb @@ -26,7 +26,7 @@ loop do Notifier.gpx_failure(trace, '0 points parsed ok. Do they all have lat,lng,alt,timestamp?').deliver trace.destroy end - rescue Exception => ex + rescue StandardError => ex logger.info ex.to_s ex.backtrace.each { |l| logger.info l } Notifier.gpx_failure(trace, ex.to_s + "\n" + ex.backtrace.join("\n")).deliver @@ -45,7 +45,7 @@ loop do begin trace.destroy - rescue Exception => ex + rescue StandardError => ex logger.info ex.to_s ex.backtrace.each { |l| logger.info l } end diff --git a/lib/diff_reader.rb b/lib/diff_reader.rb index 89e34550f..be6fee7bb 100644 --- a/lib/diff_reader.rb +++ b/lib/diff_reader.rb @@ -238,7 +238,7 @@ class DiffReader if action_attributes["if-unused"] begin old.delete_with_history!(new, @changeset.user) - rescue OSM::APIAlreadyDeletedError, OSM::APIPreconditionFailedError => ex + rescue OSM::APIAlreadyDeletedError, OSM::APIPreconditionFailedError xml_result["new_id"] = old.id.to_s xml_result["new_version"] = old.version.to_s end diff --git a/lib/gpx.rb b/lib/gpx.rb index 0a7a88c10..ae9af0d46 100644 --- a/lib/gpx.rb +++ b/lib/gpx.rb @@ -63,22 +63,16 @@ module GPX highlightgc.stroke('#000000') highlightgc.fill('#000000') - images = [] - - frames.times do - image = Magick::Image.new(width, height) do |image| + images = frames.times.collect do + Magick::Image.new(width, height) do |image| image.background_color = 'white' image.format = 'GIF' end - - images << image end oldpx = 0.0 oldpy = 0.0 - first = true - m = 0 mm = 0 points do |p| @@ -131,9 +125,9 @@ module GPX gc.stroke('#000000') gc.fill('#000000') - image = Magick::Image.new(width, height) do |image| - image.background_color = 'white' - image.format = 'GIF' + image = Magick::Image.new(width, height) do |i| + i.background_color = 'white' + i.format = 'GIF' end oldpx = 0.0 diff --git a/lib/nominatim.rb b/lib/nominatim.rb index 10eac78e5..c5d60cebd 100644 --- a/lib/nominatim.rb +++ b/lib/nominatim.rb @@ -12,7 +12,7 @@ module Nominatim response = OSM::Timer.timeout(4) do REXML::Document.new(Net::HTTP.get(URI.parse(url))) end - rescue Exception + rescue StandardError response = nil end diff --git a/lib/osm.rb b/lib/osm.rb index 1d0896417..6a41a57b6 100644 --- a/lib/osm.rb +++ b/lib/osm.rb @@ -510,7 +510,7 @@ module OSM end return nil - rescue Exception + rescue StandardError return nil end diff --git a/lib/potlatch.rb b/lib/potlatch.rb index 29cc93430..80d4d7225 100644 --- a/lib/potlatch.rb +++ b/lib/potlatch.rb @@ -71,6 +71,7 @@ module Potlatch d += encodestring("null") d += [-1].pack("N") d += encodevalue(n) + d end # Pack variables as AMF @@ -154,7 +155,7 @@ module Potlatch bodies.times do # Read each body name = AMF.getstring(@request) # | get message name index = AMF.getstring(@request) # | get index in response sequence - bytes = AMF.getlong(@request) # | get total size in bytes + AMF.getlong(@request) # | get total size in bytes args = AMF.getvalue(@request) # | get response (probably an array) result = @dispatch.call(name, *args) @@ -247,8 +248,11 @@ module Potlatch t = line.chomp if t =~ /^([\w:]+)\/(\w+)\s+(.+)$/ tag = $1; type = $2; values = $3 - if values == '-' then autotags[type][tag] = [] - else autotags[type][tag] = values.split(',').sort.reverse end + if values == '-' + autotags[type][tag] = [] + else + autotags[type][tag] = values.split(',').sort.reverse + end end end end diff --git a/script/locale/yaml2po b/script/locale/yaml2po index 22d3e78db..2753d69b7 100755 --- a/script/locale/yaml2po +++ b/script/locale/yaml2po @@ -16,7 +16,6 @@ LOCALE_DIR = File.dirname(__FILE__) + '/../../config/locales/' EN = YAML.load_file(LOCALE_DIR + 'en.yml') def iterate(hash, fhash = {}, path = '', outfile = $stdout) - postr = '' hash.each do |key, val| fhash[key] = {} unless fhash.key? key if val.is_a? Hash @@ -32,7 +31,6 @@ end def lang2po(lang, outfile = $stdout) puts lang - oth = {} infile = LOCALE_DIR + lang + '.yml' if File.exist? infile oth = YAML.load_file(infile) diff --git a/script/statistics b/script/statistics index 47a368a81..4165bb0d3 100755 --- a/script/statistics +++ b/script/statistics @@ -82,7 +82,7 @@ begin puts "" end -rescue Exception => e +rescue StandardError => e puts "

Exception: #{e}
#{e.backtrace.join('
')}

" end diff --git a/test/controllers/amf_controller_test.rb b/test/controllers/amf_controller_test.rb index 20002f7cc..def849f2a 100644 --- a/test/controllers/amf_controller_test.rb +++ b/test/controllers/amf_controller_test.rb @@ -446,7 +446,7 @@ class AmfControllerTest < ActionController::TestCase # Using similar method for the node controller test def test_putpoi_create_valid # This node has no tags - nd = Node.new + # create a node with random lat/lon lat = rand(100) - 50 + rand lon = rand(100) - 50 + rand @@ -485,7 +485,7 @@ class AmfControllerTest < ActionController::TestCase #### # This node has some tags - tnd = Node.new + # create a node with random lat/lon lat = rand(100) - 50 + rand lon = rand(100) - 50 + rand @@ -528,7 +528,7 @@ class AmfControllerTest < ActionController::TestCase # try creating a POI with rubbish in the tags def test_putpoi_create_with_control_chars # This node has no tags - nd = Node.new + # create a node with random lat/lon lat = rand(100) - 50 + rand lon = rand(100) - 50 + rand @@ -563,7 +563,7 @@ class AmfControllerTest < ActionController::TestCase # try creating a POI with rubbish in the tags def test_putpoi_create_with_invalid_utf8 # This node has no tags - nd = Node.new + # create a node with random lat/lon lat = rand(100) - 50 + rand lon = rand(100) - 50 + rand @@ -649,21 +649,21 @@ class AmfControllerTest < ActionController::TestCase req.read(2) # version # parse through any headers - headers = AMF.getint(req) # Read number of headers - headers.times do # Read each header - name = AMF.getstring(req) # | - req.getc # | skip boolean - value = AMF.getvalue(req) # | + headers = AMF.getint(req) # Read number of headers + headers.times do # Read each header + AMF.getstring(req) # | + req.getc # | skip boolean + AMF.getvalue(req) # | end # parse through responses results = {} - bodies = AMF.getint(req) # Read number of bodies - bodies.times do # Read each body - message = AMF.getstring(req) # | get message name - index = AMF.getstring(req) # | get index in response sequence - bytes = AMF.getlong(req) # | get total size in bytes - args = AMF.getvalue(req) # | get response (probably an array) + bodies = AMF.getint(req) # Read number of bodies + bodies.times do # Read each body + message = AMF.getstring(req) # | get message name + AMF.getstring(req) # | get index in response sequence + AMF.getlong(req) # | get total size in bytes + args = AMF.getvalue(req) # | get response (probably an array) results[message] = args end @amf_result = results diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb index 07b06e675..b65926d93 100644 --- a/test/controllers/changeset_controller_test.rb +++ b/test/controllers/changeset_controller_test.rb @@ -521,7 +521,6 @@ EOF put :create end assert_response :success - changeset_id = @response.body.to_i end end @@ -778,6 +777,7 @@ EOF # check that objects are unmodified assert_nodes_are_equal(node, Node.find(1)) assert_ways_are_equal(way, Way.find(1)) + assert_relations_are_equal(rel, Relation.find(1)) end ## diff --git a/test/controllers/node_controller_test.rb b/test/controllers/node_controller_test.rb index 8fb6d8e16..9e030ed39 100644 --- a/test/controllers/node_controller_test.rb +++ b/test/controllers/node_controller_test.rb @@ -194,14 +194,12 @@ class NodeControllerTest < ActionController::TestCase # in a way... content(nodes(:used_node_1).to_xml) delete :delete, :id => current_nodes(:used_node_1).id - assert_require_public_data - "shouldn't be able to delete a node used in a way (#{@response.body})" + assert_require_public_data "shouldn't be able to delete a node used in a way (#{@response.body})" # in a relation... content(nodes(:node_used_by_relationship).to_xml) delete :delete, :id => current_nodes(:node_used_by_relationship).id - assert_require_public_data - "shouldn't be able to delete a node used in a relation (#{@response.body})" + assert_require_public_data "shouldn't be able to delete a node used in a relation (#{@response.body})" ## now set auth for the public data user basic_authorization(users(:public_user).email, "test") diff --git a/test/controllers/old_node_controller_test.rb b/test/controllers/old_node_controller_test.rb index 6e90773fa..1b131b340 100644 --- a/test/controllers/old_node_controller_test.rb +++ b/test/controllers/old_node_controller_test.rb @@ -35,7 +35,6 @@ class OldNodeControllerTest < ActionController::TestCase def test_version ## First try this with a non-public user basic_authorization(users(:normal_user).email, "test") - changeset_id = changesets(:normal_user_first_change).id # setup a simple XML node xml_doc = current_nodes(:visible_node).to_xml @@ -85,7 +84,6 @@ class OldNodeControllerTest < ActionController::TestCase ## Now do it with the public user basic_authorization(users(:public_user).email, "test") - changeset_id = changesets(:public_user_first_change).id # setup a simple XML node xml_doc = current_nodes(:node_with_versions).to_xml diff --git a/test/controllers/relation_controller_test.rb b/test/controllers/relation_controller_test.rb index 2650d706c..c8ca07a38 100644 --- a/test/controllers/relation_controller_test.rb +++ b/test/controllers/relation_controller_test.rb @@ -681,7 +681,7 @@ OSM content doc put :update, :id => relation_id assert_response :success, "can't update relation: #{@response.body}" - new_version = @response.body.to_i + assert_equal 2, @response.body.to_i # get it back again and check the ordering again get :read, :id => relation_id diff --git a/test/controllers/way_controller_test.rb b/test/controllers/way_controller_test.rb index 3ae7aa36e..33d9b4579 100644 --- a/test/controllers/way_controller_test.rb +++ b/test/controllers/way_controller_test.rb @@ -124,11 +124,9 @@ class WayControllerTest < ActionController::TestCase "" + "" put :create - # hope for success + # hope for failure assert_response :forbidden, - "way upload did not return success status" - # read id of created way and search for it - wayid = @response.body + "way upload did not return forbidden status" ## Now use a public user nid1 = current_nodes(:used_node_1).id diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb index 3a1264628..04517c2b5 100644 --- a/test/helpers/application_helper_test.rb +++ b/test/helpers/application_helper_test.rb @@ -7,10 +7,6 @@ class ApplicationHelperTest < ActionView::TestCase I18n.locale = "en" end - def setup - I18n.locale = "en" - end - def test_linkify %w(http://example.com/test ftp://example.com/test https://example.com/test).each do |link| text = "Test #{link} is made into a link" diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index e45ff5b2d..3b79c433d 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -171,7 +171,6 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_openid_failure new_email = "newtester-openid2@osm.org" display_name = "new_tester-openid2" - password = "testtest2" assert_difference('User.count', 0) do assert_difference('ActionMailer::Base.deliveries.size', 0) do post "/user/new", @@ -190,7 +189,6 @@ class UserCreationTest < ActionDispatch::IntegrationTest def test_user_create_openid_redirect new_email = "redirect_tester_openid@osm.org" display_name = "redirect_tester_openid" - password = "" # nothing special about this page, just need a protected page to redirect back to. referer = "/traces/mine" assert_difference('User.count') do diff --git a/test/integration/user_login_test.rb b/test/integration/user_login_test.rb index 10c0f63b8..83322c76c 100644 --- a/test/integration/user_login_test.rb +++ b/test/integration/user_login_test.rb @@ -292,8 +292,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest post '/login', 'openid_url' => "http://localhost:1123/john.doe?openid.success=true", :referer => "/history" assert_response :redirect - res = openid_request(@response.redirect_url) - res2 = post '/login', res + post '/login', openid_request(@response.redirect_url) assert_response :redirect follow_redirect! @@ -311,8 +310,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest post '/login', 'openid_url' => "http://localhost:1123/john.doe", :referer => "/diary" assert_response :redirect - res = openid_request(@response.redirect_url) - post '/login', res + post '/login', openid_request(@response.redirect_url) assert_response :redirect follow_redirect! @@ -361,7 +359,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest assert_response :redirect res = openid_request(@response.redirect_url) - res2 = post '/login', res + post '/login', res assert_response :redirect follow_redirect! diff --git a/test/lib/bounding_box_test.rb b/test/lib/bounding_box_test.rb index eff48655c..bd2d6fec6 100644 --- a/test/lib/bounding_box_test.rb +++ b/test/lib/bounding_box_test.rb @@ -144,7 +144,6 @@ class BoundingBoxTest < ActiveSupport::TestCase end def test_expand_max_lat_with_margin - bbox = @bbox_expand @expand_max_lat_array.each_with_index do |array_string, index| check_expand(@bbox_expand_minus_two, array_string, 1, @expand_max_lat_margin_response[index]) end diff --git a/test/lib/i18n_test.rb b/test/lib/i18n_test.rb index 700966928..e8fcb687e 100644 --- a/test/lib/i18n_test.rb +++ b/test/lib/i18n_test.rb @@ -3,7 +3,7 @@ require 'test_helper' class I18nTest < ActiveSupport::TestCase I18n.available_locales.each do |locale| define_method("test_#{locale.to_s.underscore}".to_sym) do - plural_keys = plural_keys(locale) + # plural_keys = plural_keys(locale) translation_keys.each do |key| variables = [] @@ -32,7 +32,7 @@ class I18nTest < ActiveSupport::TestCase if value.is_a?(Hash) value.each do |subkey, subvalue| - # assert plural_keys.include?(subkey), "#{key}.#{subkey} is not a valid plural key" + # assert plural_keys.include?(subkey), "#{key}.#{subkey} is not a valid plural key" unless subvalue.nil? subvalue.scan(/%\{(\w+)\}/) do diff --git a/test/models/changeset_test.rb b/test/models/changeset_test.rb index f1ed1134e..dba1a3b43 100644 --- a/test/models/changeset_test.rb +++ b/test/models/changeset_test.rb @@ -40,7 +40,7 @@ class ChangesetTest < ActiveSupport::TestCase message_update = assert_raise(OSM::APIBadXMLError) do Changeset.from_xml(nokv, false) end - assert_match /tag is missing key/, message_create.message + assert_match /tag is missing key/, message_update.message end def test_from_xml_no_v diff --git a/test/models/message_test.rb b/test/models/message_test.rb index 05f3d9034..5cd171211 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -25,7 +25,7 @@ class MessageTest < ActiveSupport::TestCase def test_validating_msgs message = messages(:unread_message) assert message.valid? - massage = messages(:read_message) + message = messages(:read_message) assert message.valid? end @@ -79,8 +79,6 @@ class MessageTest < ActiveSupport::TestCase rescue ArgumentError => ex assert_equal ex.to_s, "invalid byte sequence in UTF-8" - rescue ActiveRecord::RecordInvalid - # because we only test invalid sequences it is OK to barf on them end end end diff --git a/test/models/note_test.rb b/test/models/note_test.rb index 1c67a3aa0..1f0cd74e5 100644 --- a/test/models/note_test.rb +++ b/test/models/note_test.rb @@ -30,7 +30,7 @@ class NoteTest < ActiveSupport::TestCase assert_not_nil note.closed_at end - def test_close + def test_reopen note = notes(:closed_note_with_comment) assert_equal "closed", note.status assert_not_nil note.closed_at diff --git a/test/test_helper.rb b/test/test_helper.rb index 718f7d643..f59bed648 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -71,6 +71,20 @@ class ActiveSupport::TestCase end end + ## + # for some reason assert_equal a, b fails when the relations are + # actually equal, so this method manually checks the fields... + def assert_relations_are_equal(a, b) + assert_not_nil a, "first relation is not allowed to be nil" + assert_not_nil b, "second relation #{a.id} is not allowed to be nil" + assert_equal a.id, b.id, "relation IDs" + assert_equal a.changeset_id, b.changeset_id, "changeset ID on relation #{a.id}" + assert_equal a.visible, b.visible, "visible on relation #{a.id}, #{a.visible.inspect} != #{b.visible.inspect}" + assert_equal a.version, b.version, "version on relation #{a.id}" + assert_equal a.tags, b.tags, "tags on relation #{a.id}" + assert_equal a.members, b.members, "member references on relation #{a.id}" + end + ## # for some reason assert_equal a, b fails when the ways are actually # equal, so this method manually checks the fields... @@ -130,20 +144,21 @@ class ActiveSupport::TestCase # Set things up for OpenID testing def openid_setup - rots_response = Net::HTTP.get_response(URI.parse("http://localhost:1123/")) + Net::HTTP.get_response(URI.parse("http://localhost:1123/")) rescue # It isn't, so start a new instance. rots = IO.popen("#{Rails.root}/vendor/gems/rots-0.2.1/bin/rots --silent") # Wait for up to 30 seconds for the server to start and respond before continuing - for i in (1..30) + 1.upto(30).each do begin sleep 1 - rots_response = Net::HTTP.get_response(URI.parse("http://localhost:1123/")) + Net::HTTP.get_response(URI.parse("http://localhost:1123/")) # If the rescue block doesn't fire, ROTS is up and running and we can continue break - rescue + rescue # If the connection failed, do nothing and repeat the loop + next end end