Fix tests on Python 2, and fix tests with DNSPython.

docs/changes/1012.bugfix

@@ -9,4 +9,5 @@ use modern Cython idioms.
 
 A few minor errors and discrepancies were fixed as well, such as
 ``gethostbyaddr(''localhost')`` working on Python 3 and failing on
-Python 2.
+Python 2. The DNSpython resolver now raises the expected TypeError in
+more cases instead of an AttributeError.

src/gevent/resolver/_addresses.py

@@ -142,9 +142,9 @@ def _ipv6_inet_aton(text,
 
 
 def _is_addr(host, parse=_ipv4_inet_aton):
-    if not host:
+    if not host or not isinstance(host, hostname_types):
         return False
-    assert isinstance(host, hostname_types), repr(host)
+
     try:
         parse(host)
     except AddressSyntaxError:
@@ -158,7 +158,7 @@ is_ipv4_addr = _is_addr
 
 def is_ipv6_addr(host):
     # Return True if host is a valid IPv6 address
-    if host:
+    if host and isinstance(host, hostname_types):
         s = '%' if isinstance(host, str) else b'%'
         host = host.split(s, 1)[0]
     return _is_addr(host, _ipv6_inet_aton)

src/gevent/resolver/ares.py

@@ -318,7 +318,7 @@ class Resolver(AbstractResolver):
                     raise
 
     def _getnameinfo(self, sockaddr, flags):
-        if not isinstance(flags, int):
+        if not isinstance(flags, integer_types):
             raise TypeError('an integer is required')
         if not isinstance(sockaddr, tuple):
             raise TypeError('getnameinfo() argument 1 must be a tuple')
@@ -331,7 +331,7 @@ class Resolver(AbstractResolver):
             raise TypeError('sockaddr[0] must be a string, not %s' % type(address).__name__)
 
         port = sockaddr[1]
-        if not isinstance(port, int):
+        if not isinstance(port, integer_types):
             raise TypeError('port must be an integer, not %s' % type(port))
 
         if len(sockaddr) > 2:
@@ -340,7 +340,7 @@ class Resolver(AbstractResolver):
             if flowinfo > 0xfffff:
                 raise OverflowError("getnameinfo(): flowinfo must be 0-1048575.")
 
-        result = self._getaddrinfo(address, str(sockaddr[1]),
+        result = self._getaddrinfo(address, port,
                                    family=AF_UNSPEC, socktype=SOCK_DGRAM, fill_in_type_proto=False)
         if len(result) != 1:
             raise error('sockaddr resolved to multiple addresses')

src/gevent/resolver/dnspython.py

@@ -393,13 +393,17 @@ class Resolver(AbstractResolver):
         aliases = self._resolver.hosts_resolver.getaliases(hostname)
         net_resolver = self._resolver.network_resolver
         rdtype = _family_to_rdtype(family)
-        while True:
+        while 1:
             try:
                 ans = net_resolver.query(hostname, dns.rdatatype.CNAME, rdtype)
             except (dns.resolver.NoAnswer, dns.resolver.NXDOMAIN, dns.resolver.NoNameservers):
                 break
             except dTimeout:
                 break
+            except AttributeError as ex:
+                if hostname is None or isinstance(hostname, int):
+                    raise TypeError(ex)
+                raise
             else:
                 aliases.extend(str(rr.target) for rr in ans.rrset)
                 hostname = ans[0].target

src/gevent/tests/test__socket_dns.py

@@ -14,8 +14,7 @@ import traceback
 
 import gevent.socket as gevent_socket
 import gevent.testing as greentest
-#from gevent.testing.util import log
-#from gevent.testing.util import debug
+
 from gevent.testing import util
 from gevent.testing import six
 from gevent.testing.six import xrange
@@ -39,69 +38,8 @@ import gevent.testing.timing
 assert gevent_socket.gaierror is socket.gaierror
 assert gevent_socket.error is socket.error
 
-TRACE = not util.QUIET and os.getenv('GEVENT_DEBUG', '') == 'trace'
-RUN_ALL_HOST_TESTS = os.getenv('GEVENTTEST_RUN_ALL_ETC_HOST_TESTS', '')
-
-def trace(message, *args, **kwargs):
-    if TRACE:
-        util.debug(message, *args, **kwargs)
-
-def _run(function, *args):
-    # Things that the stdlib should never raise and neither should we;
-    # these indicate bugs in our code and we want to raise them.
-    REAL_ERRORS = (AttributeError, ValueError, NameError)
-    try:
-        result = function(*args)
-        assert not isinstance(result, BaseException), repr(result)
-        return result
-    except REAL_ERRORS:
-        raise
-    except Exception as ex:
-        if TRACE:
-            traceback.print_exc()
-        return ex
-
-
-def format_call(function, args):
-    args = repr(args)
-    if args.endswith(',)'):
-        args = args[:-2] + ')'
-    try:
-        module = function.__module__.replace('gevent._socketcommon', 'gevent')
-        name = function.__name__
-        return '%s:%s%s' % (module, name, args)
-    except AttributeError:
-        return function + args
-
 
-def trace_fresult(result, seconds):
-    if not TRACE:
-        return
-    if isinstance(result, Exception):
-        msg = '  -=>  raised %r' % (result, )
-    else:
-        msg = '  -=>  returned %r' % (result, )
-    time_ms = ' %.2fms' % (seconds * 1000.0, )
-    space = 80 - len(msg) - len(time_ms)
-    if space > 0:
-        space = ' ' * space
-    else:
-        space = ''
-    util.debug(msg + space + time_ms)
-
-
-def run(function, *args):
-    trace(format_call(function, args))
-    delta = time()
-    result = _run(function, *args)
-    delta = time() - delta
-    trace_fresult(result, delta)
-    return result, delta
-
-
-def trace_call(result, runtime, function, *args):
-    util.debug(format_call(function, args))
-    trace_fresult(result, runtime)
+RUN_ALL_HOST_TESTS = os.getenv('GEVENTTEST_RUN_ALL_ETC_HOST_TESTS', '')
 
 
 def compare_relaxed(a, b):
@@ -207,13 +145,13 @@ def add(klass, hostname, name=None,
     test1.__name__ = 'test_%s_getaddrinfo' % name
     _setattr(klass, test1.__name__, test1)
 
-    def test2(self):
+    def test_gethostbyname(self):
         x = hostname() if call else hostname
         ipaddr = self._test('gethostbyname', x)
         if not isinstance(ipaddr, Exception):
             self._test('gethostbyaddr', ipaddr)
-    test2.__name__ = 'test_%s_gethostbyname' % name
-    _setattr(klass, test2.__name__, test2)
+    test_gethostbyname.__name__ = 'test_%s_gethostbyname' % name
+    _setattr(klass, test_gethostbyname.__name__, test_gethostbyname)
 
     def test3(self):
         x = hostname() if call else hostname
@@ -238,8 +176,71 @@ class TestCase(greentest.TestCase):
     maxDiff = None
     __timeout__ = 30
     switch_expected = None
+
+    TRACE = not util.QUIET and os.getenv('GEVENT_DEBUG', '') == 'trace'
     verbose_dns = TRACE
 
+    def trace(self, message, *args, **kwargs):
+        if self.TRACE:
+            util.debug(message, *args, **kwargs)
+
+    # Things that the stdlib should never raise and neither should we;
+    # these indicate bugs in our code and we want to raise them.
+    REAL_ERRORS = (AttributeError, ValueError, NameError)
+
+    def __run_resolver(self, function, args):
+        try:
+            result = function(*args)
+            assert not isinstance(result, BaseException), repr(result)
+            return result
+        except self.REAL_ERRORS:
+            raise
+        except Exception as ex:
+            if self.TRACE:
+                traceback.print_exc()
+            return ex
+
+    def __trace_call(self, result, runtime, function, *args):
+        util.debug(self.__format_call(function, args))
+        self.__trace_fresult(result, runtime)
+
+    def __format_call(self, function, args):
+        args = repr(args)
+        if args.endswith(',)'):
+            args = args[:-2] + ')'
+        try:
+            module = function.__module__.replace('gevent._socketcommon', 'gevent')
+            name = function.__name__
+            return '%s:%s%s' % (module, name, args)
+        except AttributeError:
+            return function + args
+
+    def __trace_fresult(self, result, seconds):
+        if isinstance(result, Exception):
+            msg = '  -=>  raised %r' % (result, )
+        else:
+            msg = '  -=>  returned %r' % (result, )
+        time_ms = ' %.2fms' % (seconds * 1000.0, )
+        space = 80 - len(msg) - len(time_ms)
+        if space > 0:
+            space = ' ' * space
+        else:
+            space = ''
+        util.debug(msg + space + time_ms)
+
+    if not TRACE:
+        def run_resolver(self, function, func_args):
+            now = time()
+            return self.__run_resolver(function, func_args), time() - now
+    else:
+        def run_resolver(self, function, func_args):
+            self.trace(self.__format_call(function, func_args))
+            delta = time()
+            result = self.__run_resolver(function, func_args)
+            delta = time() - delta
+            self.__trace_fresult(result, delta)
+            return result, delta
+
     def setUp(self):
         super(TestCase, self).setUp()
         if not self.verbose_dns:
@@ -263,19 +264,17 @@ class TestCase(greentest.TestCase):
             return type(result1) is not type(result2)
         return repr(result1) != repr(result2)
 
-    def _test(self, func, *args):
-        gevent_func = getattr(gevent_socket, func)
-        real_func = monkey.get_original('socket', func)
-        real_result, time_real = run(real_func, *args)
-        gevent_result, time_gevent = run(gevent_func, *args)
-        if util.QUIET and self.should_log_results(real_result, gevent_result):
-            util.log('')
-            trace_call(real_result, time_real, real_func, *args)
-            trace_call(gevent_result, time_gevent, gevent_func, *args)
-        self.assertEqualResults(real_result, gevent_result, func)
+    def _test(self, func_name, *args):
+        gevent_func = getattr(gevent_socket, func_name)
+        real_func = monkey.get_original('socket', func_name)
 
-        if self.verbose_dns and time_gevent > time_real + 0.01 and time_gevent > 0.02:
-            msg = 'gevent:%s%s took %dms versus %dms stdlib' % (func, args, time_gevent * 1000.0, time_real * 1000.0)
+        tester = getattr(self, '_run_test_' + func_name, self._run_test_generic)
+        result = tester(func_name, real_func, gevent_func, args)
+        _real_result, time_real, gevent_result, time_gevent = result
+
+        if self.verbose_dns and time_gevent > time_real + 0.02 and time_gevent > 0.03:
+            msg = 'gevent:%s%s took %dms versus %dms stdlib' % (
+                func_name, args, time_gevent * 1000.0, time_real * 1000.0)
 
             if time_gevent > time_real + 1:
                 word = 'VERY'
@@ -286,6 +285,16 @@ class TestCase(greentest.TestCase):
 
         return gevent_result
 
+    def _run_test_generic(self, func_name, real_func, gevent_func, func_args):
+        real_result, time_real = self.run_resolver(real_func, func_args)
+        gevent_result, time_gevent = self.run_resolver(gevent_func, func_args)
+        if util.QUIET and self.should_log_results(real_result, gevent_result):
+            util.log('')
+            self.__trace_call(real_result, time_real, real_func, func_args)
+            self.__trace_call(gevent_result, time_gevent, gevent_func, func_args)
+        self.assertEqualResults(real_result, gevent_result, func_name)
+        return real_result, time_real, gevent_result, time_gevent
+
     def _normalize_result(self, result, func_name):
         norm_name = '_normalize_result_' + func_name
         if hasattr(self, norm_name):
@@ -551,15 +560,30 @@ TestEtcHosts.populate_tests()
 
 
 class TestGeventOrg(TestCase):
+    # For this test to work correctly, it needs to resolve to
+    # an address with a single A record; round-robin DNS and multiple A records
+    # may mess it up (subsequent requests---and we always make two---may return
+    # unequal results). We used to use gevent.org, but that now has multiple A records;
+    # trying www.gevent.org which is a CNAME to readthedocs.org then worked, but it became
+    # an alias for python-gevent.readthedocs.org, which is an alias for readthedocs.io,
+    # and which also has multiple addresses. So we run the resolver twice to try to get
+    # the different answers, if needed.
+    HOSTNAME = 'www.gevent.org'
+
+
+    if RESOLVER_NOT_SYSTEM:
+        def _normalize_result_gethostbyname(self, result):
+            if result == '104.17.33.82':
+                result = '104.17.32.82'
+            return result
+
+        def _normalize_result_gethostbyname_ex(self, result):
+            result = super(TestGeventOrg, self)._normalize_result_gethostbyname_ex(result)
+            if result[0] == 'python-gevent.readthedocs.org':
+                result = ('readthedocs.io', ) + result[1:]
+            return result
 
-    HOSTNAME = 'python-gevent.readthedocs.org'
 
-# For this test to work correctly, it needs to resolve to
-# an address with a single A record; round-robin DNS and multiple A records
-# may mess it up (subsequent requests---and we always make two---may return
-# unequal results). We used to use gevent.org, but that now has multiple A records;
-# trying www.gevent.org which is a CNAME to readthedocs.org then worked, but it became
-# an alias for python-gevent.readthedocs.org, which is an alias for readthedocs.io.
 add(TestGeventOrg, TestGeventOrg.HOSTNAME)
 
 
@@ -571,21 +595,11 @@ class TestFamily(TestCase):
             cls._result = socket.getaddrinfo(TestGeventOrg.HOSTNAME, None)
         return cls._result
 
-    @unittest.skip(
-        "In April 2020, the system resolvers started returning INET6 answers on macOS and Travis "
-        "whereas gevent only returns INET (presumably the RTD configuration changed). "
-    )
     def test_inet(self):
-        self.assertEqualResults(
-            self.getresult(),
-            gevent_socket.getaddrinfo(TestGeventOrg.HOSTNAME, None, socket.AF_INET),
-            'getaddrinfo')
+        self._test('getaddrinfo', TestGeventOrg.HOSTNAME, None, socket.AF_INET)
 
     def test_unspec(self):
-        self.assertEqualResults(
-            self.getresult(),
-            gevent_socket.getaddrinfo(TestGeventOrg.HOSTNAME, None, socket.AF_UNSPEC),
-            'getaddrinfo')
+        self._test('getaddrinfo', TestGeventOrg.HOSTNAME, None, socket.AF_UNSPEC)
 
     def test_badvalue(self):
         self._test('getaddrinfo', TestGeventOrg.HOSTNAME, None, 255)
@@ -646,7 +660,10 @@ class Test_getaddrinfo(TestCase):
         self.assertIs(af, socket.AF_INET)
 
 class TestInternational(TestCase):
-    pass
+    if PY2:
+        # We expect these to raise UnicodeEncodeError, which is a
+        # subclass of ValueError
+        REAL_ERRORS = set(TestCase.REAL_ERRORS) - {ValueError,}
 
 # dns python can actually resolve these: it uses
 # the 2008 version of idna encoding, whereas on Python 2,