Merge remote-tracking branch 'openstreetmap/pull/1350'

diff --git a/Gemfile b/Gemfile
index 2cbb1cbef67ed982bf00398e47c3557d778c3643..7d87708f16859503cb536b6d1b36a934c8eb0517 100644 (file)
--- 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 a693e857c1fde0f9dbadf7bea65c650dd45340b1..7edc94fc86dd8a07034abb4427d2dc91e3c7c658 100644 (file)
--- 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 cd3a2156c82b5bfa89f46567776f233d0c48ac41..370f0f30009c2a09cafa3ba7ea5847a21636c372 100644 (file)
--- 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 70eacb3aa9bbb6868b57e8528658fa7201e97874..afa50a35696a2acfee56caceca4ab64b1dbdb08b 100644 (file)
--- 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 409a93b4e26b61b52ea56e99f18b18713b6173f1..e549ec90a07c9b5d4502dddd81814c7a8bfaac69 100644 (file)
--- 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 88a3fdc94a8d5040febd86ee2ff039940e4f92a5..0fbaa105d84b6e679ac18be0833a0138960a6d66 100644 (file)
--- 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: |
     <?xml version="1.0" encoding="UTF-8"?>
     <geodata>
@@ -15,8 +15,8 @@
       </standard>
     </geodata>
 
-/?geoit=XML&postal=k1a+0b1:
-  code: 200 
+/?geoit=XML&postal=k1a%200b1:
+  code: 200
   body: |
     <?xml version="1.0" encoding="UTF-8"?>
     <geodata>
@@ -31,9 +31,9 @@
         <confidence>0.9</confidence>
       </standard>
     </geodata>
-  
-/?geoit=XML&postal=Q0Q+0Q0:
-  code: 200 
+
+/?geoit=XML&postal=Q0Q%200Q0:
+  code: 200
   body: |
     <?xml version="1.0" encoding="UTF-8"?>
     <geodata>

diff --git a/test/http/gravatar.yml b/test/http/gravatar.yml
deleted file mode 100644 (file)
index c954bc8..0000000
--- 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 41467721f414d17b0d231445b0b7a87c816ddcc5..accaebf33aae157a4f966b80cf98c5a2bc5f6055 100644 (file)
--- 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: |
     <?xml version="1.0" encoding="UTF-8" ?>
     <searchresults timestamp='Sun, 01 Mar 15 20:02:29 +0000' attribution='Data © OpenStreetMap contributors, ODbL 1.0. http://www.openstreetmap.org/copyright' querystring='Hoddesdon' viewbox='-0.559,51.766,0.836,51.217' polygon='false' exclude_place_ids='110741' more_url='http://nominatim.openstreetmap.org/search?format=xml&amp;exclude_place_ids=110741&amp;viewbox=-0.559%2C51.766%2C0.836%2C51.217&amp;q=Hoddesdon'>
       <place place_id='110741' osm_type='node' osm_id='18007599' place_rank='18' boundingbox="51.7216709,51.8016709,-0.0512898,0.0287102" lat='51.7616709' lon='-0.0112898' display_name='Hoddesdon, Hertfordshire, East of England, England, United Kingdom' class='place' type='town' importance='0.50547792382382' icon='http://nominatim.openstreetmap.org/images/mapicons/poi_place_town.p.20.png'/>
     </searchresults>
-  
-/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: |
     <?xml version="1.0" encoding="UTF-8" ?>
@@ -15,7 +15,7 @@
       <place place_id='127984131' osm_type='relation' osm_id='2677978' place_rank='16' boundingbox="51.6808751,51.7806237,-0.114204,0.0145267" lat='51.73083995' lon='-0.0579457295222991' display_name='Broxbourne, Hertfordshire, East of England, England, United Kingdom' class='boundary' type='administrative' importance='0.46' icon='http://nominatim.openstreetmap.org/images/mapicons/poi_boundary_administrative.p.20.png'><extratags><tag key="place" value="village"/></extratags></place>
       <place place_id='109724' osm_type='node' osm_id='17044599' place_rank='30' boundingbox="51.7418469,51.7518469,-0.0156773,-0.0056773" lat='51.7468469' lon='-0.0106773' display_name='Broxbourne, Stafford Drive, Broxbourne, Hertfordshire, East of England, England, United Kingdom' class='railway' type='station' importance='0.111' icon='http://nominatim.openstreetmap.org/images/mapicons/transport_train_station2.p.20.png'><extratags></extratags></place>
     </searchresults>
-  
+
 /reverse?accept-language=&lat=51.7632&lon=-0.0076&zoom=15:
   code: 200
   body: |
@@ -32,7 +32,7 @@
         <country_code>gb</country_code>
       </addressparts>
     </reversegeocode>
-  
+
 /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 e9e3f45c41d101925a6b5ff2a891e486ec8a62a6..28074d089b093cc52d847d21e922c4570287218f 100644 (file)
--- 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 00a9ae06d50a0e40b6722966f36206a44fb7092c..e3ca535206749b860cb7e1d4890aef9580b5629d 100644 (file)
--- 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 43810db997885e1815ab20d2d668869649bc322c..ee1f89afa23cc4918e5cde3023c99f179643ce52 100644 (file)
--- 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 d999b5fb5cd36db29c91dd95e6c4559266081f40..cdf4fcfe9cffafb1d924a7286dc34a6d7851a556 100644 (file)
--- 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 5732430255d8b1703f1e1bfb35e13ca33475607f..2613c402cea5988611b970423c8a111ed738d99b 100644 (file)
--- 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 a664feeb0aa13bca045bb7d438aa4dea4a0ad021..d4d7450fb545eeb994203917741cb70091d623b8 100644 (file)
--- 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 95b1df307fec59d3e5876470734ba4cf2a4cb30b..5f52bb135413deaa37bdd1b386e0e8227a3a5014 100644 (file)
--- 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