Fix rubocop lint issues

.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

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

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

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'

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

app/controllers/swf_controller.rb

@@ -102,10 +102,10 @@ class SwfController < ApplicationController
   # Line-drawing
 
   def startShape
-    s = 0.chr										# No fill styles
+    s = 0.chr						# No fill styles
-    s += 2.chr										# Two line styles
+    s += 2.chr						# Two line styles
-    s += packUI16(0) + 0.chr + 255.chr + 255.chr		# Width 5, RGB #00FFFF
+    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 += 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
 
   # -----------------------------------------------------------------------

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)
 

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 = {})

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
 

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
 

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),

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)

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_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_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

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

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
 

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

lib/country.rb

@@ -13,8 +13,6 @@ class Country
     countries[code]
   end
 
-  private
-
   def self.countries
     @@countries ||= load_countries
   end

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

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

lib/gpx.rb

@@ -63,22 +63,16 @@ module GPX
       highlightgc.stroke('#000000')
       highlightgc.fill('#000000')
 
-      images = []
+      images = frames.times.collect do
-
+        Magick::Image.new(width, height) do |image|
-      frames.times do
-        image = 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 = Magick::Image.new(width, height) do |i|
-        image.background_color = 'white'
+        i.background_color = 'white'
-        image.format = 'GIF'
+        i.format = 'GIF'
       end
 
       oldpx = 0.0

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
 

lib/osm.rb

@@ -510,7 +510,7 @@ module OSM
     end
 
     return nil
-  rescue Exception
+  rescue StandardError
     return nil
   end
 

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] = []
+            if values == '-'
-            else autotags[type][tag] = values.split(',').sort.reverse end
+              autotags[type][tag] = []
+            else
+              autotags[type][tag] = values.split(',').sort.reverse
+            end
           end
         end
       end

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)

script/statistics

@@ -82,7 +82,7 @@ begin
 
     puts "</table>"
   end
-rescue Exception => e
+rescue StandardError => e
   puts "<p><em>Exception: #{e}</em><br />#{e.backtrace.join('<br />')}</p>"
 end
 

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 = AMF.getint(req)		# Read number of headers
-    headers.times do						# Read each header
+    headers.times do			# Read each header
-      name = AMF.getstring(req)				#  |
+      AMF.getstring(req)		#  |
-      req.getc				   				#  | skip boolean
+      req.getc				#  | skip boolean
-      value = AMF.getvalue(req)				#  |
+      AMF.getvalue(req)			#  |
     end
 
     # parse through responses
     results = {}
-    bodies = AMF.getint(req)					# Read number of bodies
+    bodies = AMF.getint(req)		# Read number of bodies
-    bodies.times do							# Read each body
+    bodies.times do			# Read each body
-      message = AMF.getstring(req)			#  | get message name
+      message = AMF.getstring(req)	#  | get message name
-      index = AMF.getstring(req)				#  | get index in response sequence
+      AMF.getstring(req)		#  | get index in response sequence
-      bytes = AMF.getlong(req)				#  | get total size in bytes
+      AMF.getlong(req)			#  | get total size in bytes
-      args = AMF.getvalue(req)				#  | get response (probably an array)
+      args = AMF.getvalue(req)		#  | get response (probably an array)
       results[message] = args
     end
     @amf_result = results

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
 
   ##

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
+    assert_require_public_data "shouldn't be able to delete a node used in a way (#{@response.body})"
-    "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
+    assert_require_public_data "shouldn't be able to delete a node used in a relation (#{@response.body})"
-    "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")

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

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

test/controllers/way_controller_test.rb

@@ -124,11 +124,9 @@ class WayControllerTest < ActionController::TestCase
       "<nd ref='#{nid1}'/><nd ref='#{nid2}'/>" +
       "<tag k='test' v='yes' /></way></osm>"
     put :create
-    # hope for success
+    # hope for failure
     assert_response :forbidden,
-                    "way upload did not return success status"
+                    "way upload did not return forbidden status"
-    # read id of created way and search for it
-    wayid = @response.body
 
     ## Now use a public user
     nid1 = current_nodes(:used_node_1).id

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"

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

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)
+    post '/login', openid_request(@response.redirect_url)
-    res2 = post '/login', res
 
     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', openid_request(@response.redirect_url)
-    post '/login', res
 
     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!

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

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

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

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

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

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