<p><b>@tomhughes</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1350#pullrequestreview-6369623">test/controllers/site_controller_test.rb</a>:</p>
<pre style='color:#555'>> @@ -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")
</pre>
<p>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 <code>test_helper</code> maybe?</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1350#pullrequestreview-6369623">test/controllers/user_controller_test.rb</a>:</p>
<pre style='color:#555'>> @@ -3,6 +3,10 @@
 class UserControllerTest < ActionController::TestCase
   api_fixtures
 
+  setup do
</pre>
<p>We've always used <code>def setup</code> 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.</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1350#pullrequestreview-6369623">test/http/nominatim.yml</a>:</p>
<pre style='color:#555'>> @@ -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:
</pre>
<p>So was this actually failing and hitting the live nominatim before if the stubbed URL was no longer correct?</p>

<hr>

<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1350#pullrequestreview-6369623">test/test_helper.rb</a>:</p>
<pre style='color:#555'>> @@ -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"])
</pre>
<p>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?</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1350#pullrequestreview-6369623">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABWnLSvnhzy446284LiabmFMITrmB6YEks5q5PBngaJpZM4KkVyV">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABWnLfYeutIV7ie6t2lf2sDo2Rn2M0auks5q5PBngaJpZM4KkVyV.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/openstreetmap/openstreetmap-website/pull/1350#pullrequestreview-6369623"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/openstreetmap/openstreetmap-website","title":"openstreetmap/openstreetmap-website","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/openstreetmap/openstreetmap-website"}},"updates":{"snippets":[{"icon":"PERSON","message":"@tomhughes commented on #1350"}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/openstreetmap-website/pull/1350#pullrequestreview-6369623"}}}</script>