From: Tom Hughes Date: Wed, 9 Nov 2016 20:32:54 +0000 (+0000) Subject: Merge remote-tracking branch 'openstreetmap/pull/1350' X-Git-Tag: live~3719 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/2f722fc281434ef59e19c504bff9e748e9d3f4b4?hp=2308e69582e1a55d83c5ce89f6c850d56ad196aa Merge remote-tracking branch 'openstreetmap/pull/1350' --- diff --git a/Gemfile b/Gemfile index 2cbb1cbef..7d87708f1 100644 --- a/Gemfile +++ b/Gemfile @@ -101,6 +101,7 @@ group :test do gem "rubocop" gem "timecop" gem "minitest", "~> 5.1", :platforms => [:ruby_19, :ruby_20] + gem "webmock" end # Needed in development as well so rake can see konacha tasks diff --git a/Gemfile.lock b/Gemfile.lock index a693e857c..7edc94fc8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -75,6 +75,8 @@ GEM term-ansicolor (~> 1.3) thor (~> 0.19.1) tins (>= 1.6.0, < 2) + crack (0.4.3) + safe_yaml (~> 1.0.0) crass (1.0.2) dalli (2.7.6) deadlock_retry (1.2.0) @@ -94,6 +96,7 @@ GEM geoip (1.6.2) globalid (0.3.7) activesupport (>= 4.1.0) + hashdiff (0.3.0) hashie (3.4.6) htmlentities (4.3.4) http_accept_language (2.0.5) @@ -257,6 +260,7 @@ GEM unicode-display_width (~> 1.0, >= 1.0.1) ruby-openid (2.7.0) ruby-progressbar (1.8.1) + safe_yaml (1.0.4) sanitize (4.4.0) crass (~> 1.0.2) nokogiri (>= 1.4.4) @@ -298,6 +302,10 @@ GEM validates_email_format_of (1.6.3) i18n vendorer (0.1.16) + webmock (1.24.2) + addressable (>= 2.3.6) + crack (>= 0.3.2) + hashdiff websocket-driver (0.6.4) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.2) @@ -359,6 +367,7 @@ DEPENDENCIES uglifier (>= 1.3.0) validates_email_format_of (>= 1.5.1) vendorer + webmock BUNDLED WITH - 1.10.6 + 1.11.2 diff --git a/lib/osm.rb b/lib/osm.rb index cd3a2156c..370f0f300 100644 --- a/lib/osm.rb +++ b/lib/osm.rb @@ -569,11 +569,6 @@ module OSM @http_client ||= Faraday.new end - # Set the HTTP client to use - def self.http_client=(client) - @http_client = client - end - # Return the GeoIP database handle def self.geoip_database @geoip_database ||= GeoIP.new(GEOIP_DATABASE) if defined?(GEOIP_DATABASE) diff --git a/test/controllers/site_controller_test.rb b/test/controllers/site_controller_test.rb index 70eacb3aa..afa50a356 100644 --- a/test/controllers/site_controller_test.rb +++ b/test/controllers/site_controller_test.rb @@ -8,6 +8,8 @@ class SiteControllerTest < ActionController::TestCase def setup Object.const_set("ID_KEY", client_applications(:oauth_web_app).key) Object.const_set("POTLATCH2_KEY", client_applications(:oauth_web_app).key) + + stub_signup_requests end ## diff --git a/test/controllers/user_controller_test.rb b/test/controllers/user_controller_test.rb index 409a93b4e..e549ec90a 100644 --- a/test/controllers/user_controller_test.rb +++ b/test/controllers/user_controller_test.rb @@ -3,6 +3,10 @@ require "test_helper" class UserControllerTest < ActionController::TestCase api_fixtures + def setup + stub_signup_requests + end + ## # test all routes which lead to this controller def test_routes @@ -392,6 +396,7 @@ class UserControllerTest < ActionController::TestCase def test_confirm_success_no_token_no_referer user = users(:inactive_user) + stub_gravatar_request(user.email) confirm_string = user.tokens.create.token @request.cookies["_osm_session"] = user.display_name @@ -402,6 +407,7 @@ class UserControllerTest < ActionController::TestCase def test_confirm_success_good_token_no_referer user = users(:inactive_user) + stub_gravatar_request(user.email) confirm_string = user.tokens.create.token token = user.tokens.create.token @@ -412,6 +418,7 @@ class UserControllerTest < ActionController::TestCase def test_confirm_success_bad_token_no_referer user = users(:inactive_user) + stub_gravatar_request(user.email) confirm_string = user.tokens.create.token token = users(:normal_user).tokens.create.token @@ -423,6 +430,7 @@ class UserControllerTest < ActionController::TestCase def test_confirm_success_no_token_with_referer user = users(:inactive_user) + stub_gravatar_request(user.email) confirm_string = user.tokens.create(:referer => diary_new_path).token @request.cookies["_osm_session"] = user.display_name @@ -433,6 +441,7 @@ class UserControllerTest < ActionController::TestCase def test_confirm_success_good_token_with_referer user = users(:inactive_user) + stub_gravatar_request(user.email) confirm_string = user.tokens.create(:referer => diary_new_path).token token = user.tokens.create.token @@ -443,6 +452,7 @@ class UserControllerTest < ActionController::TestCase def test_confirm_success_bad_token_with_referer user = users(:inactive_user) + stub_gravatar_request(user.email) confirm_string = user.tokens.create(:referer => diary_new_path).token token = users(:normal_user).tokens.create.token @@ -521,6 +531,7 @@ class UserControllerTest < ActionController::TestCase def test_confirm_email_success user = users(:second_public_user) + stub_gravatar_request(user.new_email) confirm_string = user.tokens.create.token post :confirm_email, :confirm_string => confirm_string @@ -551,35 +562,33 @@ class UserControllerTest < ActionController::TestCase # this happens when the email is actually changed # which is triggered by the confirmation mail def test_gravatar_auto_enable - with_http_stubs "gravatar" do - # switch to email that has a gravatar - user = users(:first_gravatar_user) - confirm_string = user.tokens.create.token - # precondition gravatar should be turned off - assert !user.image_use_gravatar - post :confirm_email, :confirm_string => confirm_string - assert_response :redirect - assert_redirected_to :action => :account, :display_name => user.display_name - assert_match /Confirmed your change of email address/, flash[:notice] - # gravatar use should now be enabled - assert User.find(users(:first_gravatar_user).id).image_use_gravatar - end + # switch to email that has a gravatar + user = users(:first_gravatar_user) + stub_gravatar_request(user.new_email, 200) + confirm_string = user.tokens.create.token + # precondition gravatar should be turned off + assert !user.image_use_gravatar + post :confirm_email, :confirm_string => confirm_string + assert_response :redirect + assert_redirected_to :action => :account, :display_name => user.display_name + assert_match /Confirmed your change of email address/, flash[:notice] + # gravatar use should now be enabled + assert User.find(users(:first_gravatar_user).id).image_use_gravatar end def test_gravatar_auto_disable - with_http_stubs "gravatar" do - # switch to email without a gravatar - user = users(:second_gravatar_user) - confirm_string = user.tokens.create.token - # precondition gravatar should be turned on - assert user.image_use_gravatar - post :confirm_email, :confirm_string => confirm_string - assert_response :redirect - assert_redirected_to :action => :account, :display_name => user.display_name - assert_match /Confirmed your change of email address/, flash[:notice] - # gravatar use should now be disabled - assert !User.find(users(:second_gravatar_user).id).image_use_gravatar - end + # switch to email without a gravatar + user = users(:second_gravatar_user) + stub_gravatar_request(user.new_email, 404) + confirm_string = user.tokens.create.token + # precondition gravatar should be turned on + assert user.image_use_gravatar + post :confirm_email, :confirm_string => confirm_string + assert_response :redirect + assert_redirected_to :action => :account, :display_name => user.display_name + assert_match /Confirmed your change of email address/, flash[:notice] + # gravatar use should now be disabled + assert !User.find(users(:second_gravatar_user).id).image_use_gravatar end def test_terms_new_user diff --git a/test/http/geocoder_ca.yml b/test/http/geocoder_ca.yml index 88a3fdc94..0fbaa105d 100644 --- a/test/http/geocoder_ca.yml +++ b/test/http/geocoder_ca.yml @@ -1,5 +1,5 @@ -/?geoit=XML&postal=A1B+2C3: - code: 200 +/?geoit=XML&postal=A1B%202C3: + code: 200 body: | @@ -15,8 +15,8 @@ -/?geoit=XML&postal=k1a+0b1: - code: 200 +/?geoit=XML&postal=k1a%200b1: + code: 200 body: | @@ -31,9 +31,9 @@ 0.9 - -/?geoit=XML&postal=Q0Q+0Q0: - code: 200 + +/?geoit=XML&postal=Q0Q%200Q0: + code: 200 body: | diff --git a/test/http/gravatar.yml b/test/http/gravatar.yml deleted file mode 100644 index c954bc822..000000000 --- a/test/http/gravatar.yml +++ /dev/null @@ -1,7 +0,0 @@ -/avatar/842bc90353fac655450e62223e4e105d?d=404 : - code: 404 - body: Ignored, test for new_g2@openstreetmap.org - -/avatar/d2e95ef0ac6933916bf42ff1ee4eca4b?d=404 : - code: 200 - body: Ignored, test for new_g1@openstreetmap.org diff --git a/test/http/nominatim.yml b/test/http/nominatim.yml index 41467721f..accaebf33 100644 --- a/test/http/nominatim.yml +++ b/test/http/nominatim.yml @@ -1,12 +1,12 @@ -/search?accept-language=&format=xml&q=Hoddesdon&viewbox=-0.559%2C51.766%2C0.836%2C51.217: +/search?accept-language=&extratags=1&format=xml&q=Hoddesdon&viewbox=-0.559,51.766,0.836,51.217: code: 200 body: | - -/search?accept-language=&format=xml&extratags=1&q=Broxbourne&viewbox=-0.559%2C51.766%2C0.836%2C51.217: + +/search?accept-language=&extratags=1&format=xml&q=Broxbourne&viewbox=-0.559,51.766,0.836,51.217: code: 200 body: | @@ -15,7 +15,7 @@ - + /reverse?accept-language=&lat=51.7632&lon=-0.0076&zoom=15: code: 200 body: | @@ -32,7 +32,7 @@ gb - + /reverse?accept-language=&lat=51.7632&lon=-0.0076&zoom=17: code: 200 body: | diff --git a/test/http/npemap.yml b/test/http/npemap.yml index e9e3f45c4..28074d089 100644 --- a/test/http/npemap.yml +++ b/test/http/npemap.yml @@ -1,10 +1,10 @@ -/cgi/geocoder.fcgi?format=text&postcode=CV4+7AL: +/cgi/geocoder.fcgi?format=text&postcode=CV4%207AL: code: 200 body: | # Easting,Northing,Matched Postcode,Latitude,Longitude 429926,276058,'CV4 7AL',52.381748701968,-1.56176420939232 -/cgi/geocoder.fcgi?format=text&postcode=XX9+9XX: +/cgi/geocoder.fcgi?format=text&postcode=XX9%209XX: code: 200 body: | Error: Postcode area 'XX' not found, postcode probably invalid diff --git a/test/integration/oauth_test.rb b/test/integration/oauth_test.rb index 00a9ae06d..e3ca53520 100644 --- a/test/integration/oauth_test.rb +++ b/test/integration/oauth_test.rb @@ -6,6 +6,10 @@ class OAuthTest < ActionDispatch::IntegrationTest include OAuth::Helper + setup do + stub_signup_requests + end + def test_oauth10_web_app client = client_applications(:oauth_web_app) diff --git a/test/integration/page_locale_test.rb b/test/integration/page_locale_test.rb index 43810db99..ee1f89afa 100644 --- a/test/integration/page_locale_test.rb +++ b/test/integration/page_locale_test.rb @@ -5,6 +5,7 @@ class PageLocaleTest < ActionDispatch::IntegrationTest def setup I18n.locale = "en" + stub_signup_requests end def teardown diff --git a/test/integration/user_creation_test.rb b/test/integration/user_creation_test.rb index d999b5fb5..cdf4fcfe9 100644 --- a/test/integration/user_creation_test.rb +++ b/test/integration/user_creation_test.rb @@ -7,6 +7,8 @@ class UserCreationTest < ActionDispatch::IntegrationTest I18n.locale = "en" OmniAuth.config.test_mode = true + + stub_request(:get, /.*gravatar.com.*d=404/).to_return(:status => 404) end def teardown diff --git a/test/integration/user_roles_test.rb b/test/integration/user_roles_test.rb index 573243025..2613c402c 100644 --- a/test/integration/user_roles_test.rb +++ b/test/integration/user_roles_test.rb @@ -3,6 +3,10 @@ require "test_helper" class UserRolesTest < ActionDispatch::IntegrationTest fixtures :users, :user_roles + setup do + stub_signup_requests + end + test "grant" do check_fail(:grant, :public_user, :moderator) check_fail(:grant, :moderator_user, :moderator) diff --git a/test/integration/user_terms_seen_test.rb b/test/integration/user_terms_seen_test.rb index a664feeb0..d4d7450fb 100644 --- a/test/integration/user_terms_seen_test.rb +++ b/test/integration/user_terms_seen_test.rb @@ -3,6 +3,10 @@ require "test_helper" class UserTermsSeenTest < ActionDispatch::IntegrationTest fixtures :users + setup do + stub_signup_requests + end + def test_api_blocked with_terms_seen(true) do user = users(:terms_not_seen_user) diff --git a/test/test_helper.rb b/test/test_helper.rb index 95b1df307..5f52bb135 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -4,6 +4,7 @@ Coveralls.wear!("rails") ENV["RAILS_ENV"] = "test" require File.expand_path("../../config/environment", __FILE__) require "rails/test_help" +require "webmock/minitest" load "composite_primary_keys/fixtures.rb" module ActiveSupport @@ -160,23 +161,24 @@ module ActiveSupport ## # execute a block with a given set of HTTP responses stubbed def with_http_stubs(stubs_file) - http_client_save = OSM.http_client + stubs = YAML.load_file(File.expand_path("../http/#{stubs_file}.yml", __FILE__)) + stubs.each do |url, response| + stub_request(:get, Regexp.new(Regexp.quote(url))).to_return(:status => response["code"], :body => response["body"]) + end - begin - stubs = YAML.load_file(File.expand_path("../http/#{stubs_file}.yml", __FILE__)) + yield + end - OSM.http_client = Faraday.new do |builder| - builder.adapter :test do |stub| - stubs.each do |url, response| - stub.get(url) { |_env| [response["code"], {}, response["body"]] } - end - end - end + def stub_gravatar_request(email, status = 200, body = nil) + hash = Digest::MD5.hexdigest(email.downcase) + url = "https://www.gravatar.com/avatar/#{hash}?d=404" + stub_request(:get, url).and_return(:status => status, :body => body) + end - yield - ensure - OSM.http_client = http_client_save - end + def stub_signup_requests + # Controller tests and integration tests use different IPs + stub_request(:get, "http://api.hostip.info/country.php?ip=0.0.0.0") + stub_request(:get, "http://api.hostip.info/country.php?ip=127.0.0.1") end end end