[openstreetmap/openstreetmap-website] Use Webmock to intercept http requests in tests (#1350)

Tom Hughes notifications at github.com
Sun Oct 30 19:30:47 UTC 2016


tomhughes commented on this pull request.



> @@ -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_request(:get, "http://api.hostip.info/country.php?ip=0.0.0.0")

Given the number of tests which need this (I think I counted seven) should we maybe just install this one globally? Is there even a place to do that? Something in `test_helper` maybe?

> @@ -3,6 +3,10 @@
 class UserControllerTest < ActionController::TestCase
   api_fixtures
 
+  setup do

We've always used `def setup` in the past. Is this way the typical way to do it now? Basically I'd prefer to standardise one way or the other and I'd just like to figure out which is more normal so we can standardise on that.

> @@ -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:

So was this actually failing and hitting the live nominatim before if the stubbed URL was no longer correct?

> @@ -160,23 +161,18 @@ def assert_no_missing_translations(msg = "")
     ##
     # execute a block with a given set of HTTP responses stubbed
     def with_http_stubs(stubs_file)
-      http_client_save = OSM.http_client
-
-      begin
-        stubs = YAML.load_file(File.expand_path("../http/#{stubs_file}.yml", __FILE__))
+      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"])

If we're turning the URL into a regex do we need to add anchors to the start and end so we don't accidentally get a substring match? I realise it's pretty unlikely but anchoring it makes it more correct I think?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/1350#pullrequestreview-6369623
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20161030/c39115a6/attachment.html>


More information about the rails-dev mailing list