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

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index e983c51c64fed85fe13a3ed4c69bd605da32e884..d70618e766295c2b5cb72271929d60db0bac136a 100644 (file)
--- a/.rubocop_todo.yml
+++ b/.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:

diff --git a/test/abilities/api_capability_test.rb b/test/abilities/api_capability_test.rb
index 8a98f29e0e6274c6c871e57bbc9b697a1aa6666a..dccde57580db834fb2a7d81b3ecb34e073ffae0b 100644 (file)
--- a/test/abilities/api_capability_test.rb
+++ b/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|

diff --git a/test/controllers/api/nodes_controller_test.rb b/test/controllers/api/nodes_controller_test.rb
index 645408dd433d04236e76a5048582916ef5ce34a0..339dd2af8ae4f87022666d51634f0539d6ab9acc 100644 (file)
--- a/test/controllers/api/nodes_controller_test.rb
+++ b/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)

diff --git a/test/controllers/api/old_nodes_controller_test.rb b/test/controllers/api/old_nodes_controller_test.rb
index cc7fae416247474926a6351bf92b58b035013b60..e85bc86f85044592d895fc2a6186db818464a7ae 100644 (file)
--- a/test/controllers/api/old_nodes_controller_test.rb
+++ b/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.

diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb
index 559f19bd6d3a6989b2c2111b05f747677addadc1..de24c087de1b8bf3babfb3f4d6e564637c7ca12a 100644 (file)
--- a/test/controllers/api/relations_controller_test.rb
+++ b/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

diff --git a/test/controllers/api/ways_controller_test.rb b/test/controllers/api/ways_controller_test.rb
index 74ef9f524f5f5fa8db1326c8d8885d92f8a56d6d..0cf30e4fffb176fb1d34b931fcab7934f4cf1a37 100644 (file)
--- a/test/controllers/api/ways_controller_test.rb
+++ b/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)

diff --git a/test/helpers/browse_helper_test.rb b/test/helpers/browse_helper_test.rb
index 8e46f6ff3ca28fbdf2c9b3a199fa864ab12672d4..bc597596f5e46e532e48fb77ceb688ce59d63d6c 100644 (file)
--- a/test/helpers/browse_helper_test.rb
+++ b/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

diff --git a/test/integration/client_applications_test.rb b/test/integration/client_applications_test.rb
index 4c3e9df470197c1a50b2550c1d0645f3f06d519d..5a00a563108ccca1da7a8fec0393686c52e2399c 100644 (file)
--- a/test/integration/client_applications_test.rb
+++ b/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)

diff --git a/test/integration/short_links_test.rb b/test/integration/short_links_test.rb
index c03ebedab4033de6d262a1786dcaa9aa7f2162cc..8f949fdaeddf501fdc167941160322303e62f7e1 100644 (file)
--- a/test/integration/short_links_test.rb
+++ b/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)

diff --git a/test/integration/user_blocks_test.rb b/test/integration/user_blocks_test.rb
index ce67e74ba5455efef6efce6e91e869a8c747678b..ecadaad534f8a0d732114d860a73d1110a4fd2c5 100644 (file)
--- a/test/integration/user_blocks_test.rb
+++ b/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

diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb
index 9c934dc072282f8c8ac590435d34d546d652662c..8b6b3ef3cd2fb19fda066c19ddeb89e5abcc1862 100644 (file)
--- a/test/integration/user_creation_test.rb
+++ b/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