Merge remote-tracking branch 'upstream/pull/2918' into master

.rubocop_todo.yml

@@ -84,21 +84,6 @@ Metrics/PerceivedComplexity:
 Minitest/MultipleAssertions:
   Max: 81
 
-# Offense count: 26
-# Cop supports --auto-correct.
-Minitest/TestMethodName:
-  Exclude:
-    - 'test/abilities/api_capability_test.rb'
-    - 'test/controllers/api/nodes_controller_test.rb'
-    - 'test/controllers/api/old_nodes_controller_test.rb'
-    - 'test/controllers/api/relations_controller_test.rb'
-    - 'test/controllers/api/ways_controller_test.rb'
-    - 'test/helpers/browse_helper_test.rb'
-    - 'test/integration/client_applications_test.rb'
-    - 'test/integration/short_links_test.rb'
-    - 'test/integration/user_blocks_test.rb'
-    - 'test/integration/user_creation_test.rb'
-
 # Offense count: 6
 Naming/AccessorMethodName:
   Exclude:

test/abilities/api_capability_test.rb

@@ -3,6 +3,8 @@
 require "test_helper"
 
 class ApiCapabilityTest < ActiveSupport::TestCase
+  private
+
   def tokens(*toks)
     AccessToken.new do |token|
       toks.each do |t|

test/controllers/api/nodes_controller_test.rb

@@ -559,6 +559,8 @@ module Api
       assert_includes apinode.tags, "\#{@user.inspect}"
     end
 
+    private
+
     ##
     # update the changeset_id of a node element
     def update_changeset(xml, changeset_id)

test/controllers/api/old_nodes_controller_test.rb

@@ -161,13 +161,6 @@ module Api
       check_not_found_id_version(24356, create(:node).version)
     end
 
-    def check_not_found_id_version(id, version)
-      get node_version_path(:id => id, :version => version)
-      assert_response :not_found
-    rescue ActionController::UrlGenerationError => e
-      assert_match(/No route matches/, e.to_s)
-    end
-
     ##
     # Test that getting the current version is identical to picking
     # that version with the version URI call.
@@ -419,6 +412,13 @@ module Api
       assert_nodes_are_equal current_node, old_node
     end
 
+    def check_not_found_id_version(id, version)
+      get node_version_path(:id => id, :version => version)
+      assert_response :not_found
+    rescue ActionController::UrlGenerationError => e
+      assert_match(/No route matches/, e.to_s)
+    end
+
     ##
     # returns a 16 character long string with some nasty characters in it.
     # this ought to stress-test the tag handling as well as the versioning.

test/controllers/api/relations_controller_test.rb

@@ -151,25 +151,6 @@ module Api
                                   [relation_with_relation, second_relation])
     end
 
-    def check_relations_for_element(path, type, id, expected_relations)
-      # check the "relations for relation" mode
-      get path
-      assert_response :success
-
-      # count one osm element
-      assert_select "osm[version='#{Settings.api_version}'][generator='OpenStreetMap server']", 1
-
-      # we should have only the expected number of relations
-      assert_select "osm>relation", expected_relations.size
-
-      # and each of them should contain the element we originally searched for
-      expected_relations.each do |relation|
-        # The relation should appear once, but the element could appear multiple times
-        assert_select "osm>relation[id='#{relation.id}']", 1
-        assert_select "osm>relation[id='#{relation.id}']>member[type='#{type}'][ref='#{id}']"
-      end
-    end
-
     def test_full
       # check the "full" mode
       get relation_full_path(:id => 999999)
@@ -926,9 +907,26 @@ module Api
       end
     end
 
-    # ============================================================
-    # utility functions
-    # ============================================================
+    private
+
+    def check_relations_for_element(path, type, id, expected_relations)
+      # check the "relations for relation" mode
+      get path
+      assert_response :success
+
+      # count one osm element
+      assert_select "osm[version='#{Settings.api_version}'][generator='OpenStreetMap server']", 1
+
+      # we should have only the expected number of relations
+      assert_select "osm>relation", expected_relations.size
+
+      # and each of them should contain the element we originally searched for
+      expected_relations.each do |relation|
+        # The relation should appear once, but the element could appear multiple times
+        assert_select "osm>relation[id='#{relation.id}']", 1
+        assert_select "osm>relation[id='#{relation.id}']>member[type='#{type}'][ref='#{id}']"
+      end
+    end
 
     ##
     # checks that the XML document and the string arguments have

test/controllers/api/ways_controller_test.rb

@@ -754,6 +754,8 @@ module Api
       end
     end
 
+    private
+
     ##
     # update the changeset_id of a way element
     def update_changeset(xml, changeset_id)

test/helpers/browse_helper_test.rb

@@ -132,6 +132,8 @@ class BrowseHelperTest < ActionView::TestCase
     assert_includes tags, %w[shop gift]
   end
 
+  private
+
   def add_old_tags_selection(old_node)
     { "building" => "yes",
       "shop" => "gift",
@@ -152,8 +154,6 @@ class BrowseHelperTest < ActionView::TestCase
     end
   end
 
-  private
-
   def preferred_languages
     Locale.list(I18n.locale)
   end

test/integration/client_applications_test.rb

@@ -76,6 +76,8 @@ class ClientApplicationsTest < ActionDispatch::IntegrationTest
     # tests, as its too tied into the HTTP headers and stuff that it signs.
   end
 
+  private
+
   ##
   # utility method to make the HTML screening easier to read.
   def assert_in_heading(&block)

test/integration/short_links_test.rb

@@ -9,6 +9,8 @@ class ShortLinksTest < ActionDispatch::IntegrationTest
     assert_short_link_redirect(ShortLink.encode(-0.107846, 51.50771, 18))
   end
 
+  private
+
   ##
   # utility method to test short links
   def assert_short_link_redirect(short_link)

test/integration/user_blocks_test.rb

@@ -1,17 +1,13 @@
 require "test_helper"
 
 class UserBlocksTest < ActionDispatch::IntegrationTest
-  def auth_header(user, pass)
-    { "HTTP_AUTHORIZATION" => format("Basic %<auth>s", :auth => Base64.encode64("#{user}:#{pass}")) }
-  end
-
   def test_api_blocked
     blocked_user = create(:user)
 
     get "/api/#{Settings.api_version}/user/details"
     assert_response :unauthorized
 
-    get "/api/#{Settings.api_version}/user/details", :headers => auth_header(blocked_user.display_name, "test")
+    get "/api/#{Settings.api_version}/user/details", :headers => basic_authorization_header(blocked_user.display_name, "test")
     assert_response :success
 
     # now block the user
@@ -21,7 +17,7 @@ class UserBlocksTest < ActionDispatch::IntegrationTest
       :reason => "testing",
       :ends_at => Time.now.getutc + 5.minutes
     )
-    get "/api/#{Settings.api_version}/user/details", :headers => auth_header(blocked_user.display_name, "test")
+    get "/api/#{Settings.api_version}/user/details", :headers => basic_authorization_header(blocked_user.display_name, "test")
     assert_response :forbidden
   end
 
@@ -35,7 +31,7 @@ class UserBlocksTest < ActionDispatch::IntegrationTest
       :reason => "testing",
       :ends_at => Time.now.getutc + 5.minutes
     )
-    get "/api/#{Settings.api_version}/user/details", :headers => auth_header(blocked_user.display_name, "test")
+    get "/api/#{Settings.api_version}/user/details", :headers => basic_authorization_header(blocked_user.display_name, "test")
     assert_response :forbidden
 
     # revoke the ban
@@ -54,7 +50,7 @@ class UserBlocksTest < ActionDispatch::IntegrationTest
     reset!
 
     # access the API again. this time it should work
-    get "/api/#{Settings.api_version}/user/details", :headers => auth_header(blocked_user.display_name, "test")
+    get "/api/#{Settings.api_version}/user/details", :headers => basic_authorization_header(blocked_user.display_name, "test")
     assert_response :success
   end
 end

test/integration/user_creation_test.rb

@@ -157,7 +157,7 @@ class UserCreationTest < ActionDispatch::IntegrationTest
   end
 
   # Check that the user can successfully recover their password
-  def lost_password_recovery_success
+  def test_lost_password_recovery_success
     # Open the lost password form
     # Submit the lost password form
     # Check the e-mail