Remove network dependency in GeoIP tests

[Markus Holtermann]

Occasionally some GIS tests fail due to connectivity or availability issues when trying to get live data. Those requests should be mocked away. One of the tests failing is gis_tests.test_geoip2.GeoIPTest.test05_unicode_response:

ERROR [15.017s]: test05_unicode_response (gis_tests.test_geoip2.GeoIPTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jenkins/workspace/pull-requests-trusty/database/postgis/label/trusty-pr/python/python2.7/tests/gis_tests/test_geoip2.py", line 127, in test05_unicode_response
    d = g.city("nigde.edu.tr")
  File "/home/jenkins/workspace/pull-requests-trusty/database/postgis/label/trusty-pr/python/python2.7/django/contrib/gis/geoip2/base.py", line 172, in city
    enc_query = self._check_query(query, city=True)
  File "/home/jenkins/workspace/pull-requests-trusty/database/postgis/label/trusty-pr/python/python2.7/django/contrib/gis/geoip2/base.py", line 162, in _check_query
    query = socket.gethostbyname(query)
gaierror: [Errno -2] Name or service not known

[Tim Graham]

Attached an example of how to fix the test in question, but needs to be applied to the other tests in that file as well as test_geoip.py. Should be fairly straightforward.

[Anton Baklanov]

Let me try this.

[Berker Peksag]

Instead of mocking socket.gethostbyname()(the IP can be changed), you can skip the test if there is a transient failure. CPython uses the following helper: ​https://hg.python.org/cpython/file/default/Lib/test/support/__init__.py#l1294 The API could be better, of course.

[Tim Graham]

Thanks for the suggestion Berker, but in this case the IP of the website isn't important for the purposes of the test (other than it must be in the tested country/city), so actually I think the mocking approach is better because we don't have to worry if the website does relocate their server outside of the given test country/city (of course the IP could be reassigned geographically too, but at that point we just have to update the test).

[Anton Baklanov]

Hi all!

Had to wait until a weekend to move forward on this. Here is what I've got for now:

• Mocking works fine for geoip2 since domains are resolved within python code - ​https://github.com/django/django/blob/master/django/contrib/gis/geoip2/base.py#L162

(​initial branch is there)

• With geoip it is not that simple. Domain resolution is done within maxmind C library - ​https://github.com/maxmind/geoip-api-c/blob/733b36e88138a6d603e411976c0a18972f8e2045/libGeoIP/GeoIP.c#L1681 and django just uses it as is via ctypes - ​https://github.com/django/django/blob/master/django/contrib/gis/geoip/prototypes.py#L122

• Other tests seem to work fine without network (had 146 skips though).

Unfortunately Berker's hint on catching network errors during the test (using context manager for example) is not going to work in this case because the actual network error is being consumed inside the C library and all we have is just None as a result of a geoip query.

I can see the following options here:

• Introduce some sort of a skip decorator or just a routine inside a testsuite for checking if there is a proper network connection (and possibly skipping tests that require internet). This seems a bit tricky to me. How do we check connectivity? Which resources should we query? Besides, for this particular case we don't need a full internet connection at all, just DNS (local caching server will do fine).

• Add hacks to geoip tests (3 tests require such 'fixing') so they will try to do socket.gethostbyname for domains under test in the beginning or setUp. In case gethostbyname fails - tests will proceed with IP checks only, skipping fqdn checks (probably issuing a warning, so it's easy to catch if the issue persists).

• Leave it as is. geoip is deprecated after all.

@timgraham, @berkerpeksag - could you advise a proper way to go here?

Meanwhile, I'm going to get some sleep. Perhaps there are other solutions hidden out there which I can't see now.

[Tim Graham]

I wouldn't worry about the deprecated geoip tests if it's a big pain. In fact, I only recently enabled them on Jenkins. If they are problematic in the future we can just disable them.

[Anton Baklanov]

​https://github.com/django/django/pull/5343

[Tim Graham <timograham@…>]

In d0ed01c:

Fixed #25407 -- Removed network dependency in GeoIP tests.