From: Tom Hughes Date: Wed, 28 Oct 2020 11:29:57 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/2918' into master X-Git-Tag: live~1867 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/93e49daee21b64f0af0ba7fa3035904843a028ea?hp=048ab33934bb34abe323dcbc331512ef7f89328e Merge remote-tracking branch 'upstream/pull/2918' into master --- diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e983c51c6..d70618e76 100644 --- 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 8a98f29e0..dccde5758 100644 --- 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 645408dd4..339dd2af8 100644 --- 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 cc7fae416..e85bc86f8 100644 --- 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 559f19bd6..de24c087d 100644 --- 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 74ef9f524..0cf30e4ff 100644 --- 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 8e46f6ff3..bc597596f 100644 --- 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 4c3e9df47..5a00a5631 100644 --- 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 c03ebedab..8f949fdae 100644 --- 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 ce67e74ba..ecadaad53 100644 --- 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 %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 9c934dc07..8b6b3ef3c 100644 --- 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