socket/dns test cleanups for CI.

Use test.support.find_unused_port() instead of rolling our own; it handles windows better.
Limit the DNS6 tests to known good configurations.
Hopefully fix a bunch of flakiness in test__refcount's use of sockets.

Fixes #1389

src/gevent/testing/params.py

@@ -18,17 +18,14 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
 
+from . import support
 
 from .sysinfo import PY3
 from .sysinfo import PYPY
 from .sysinfo import WIN
 from .sysinfo import LIBUV
-from .sysinfo import OSX
 
-from .sysinfo import RUNNING_ON_TRAVIS
-from .sysinfo import RUNNING_ON_APPVEYOR
 from .sysinfo import EXPECT_POOR_TIMER_RESOLUTION
-from .sysinfo import RESOLVER_ARES
 
 
 # Travis is slow and overloaded; Appveyor used to be faster, but
@@ -47,33 +44,18 @@ else:
 
 LARGE_TIMEOUT = max(LOCAL_TIMEOUT, CI_TIMEOUT)
 
-DEFAULT_LOCAL_HOST_ADDR = 'localhost'
-DEFAULT_LOCAL_HOST_ADDR6 = DEFAULT_LOCAL_HOST_ADDR
-DEFAULT_BIND_ADDR = ''
+# Previously we set this manually to 'localhost'
+# and then had some conditions where we changed it to
+# 127.0.0.1 (e.g., on Windows or OSX or travis), but Python's test.support says
+# # Don't use "localhost", since resolving it uses the DNS under recent
+# # Windows versions (see issue #18792).
+# and sets it unconditionally to 127.0.0.1.
+DEFAULT_LOCAL_HOST_ADDR = support.HOST
+DEFAULT_LOCAL_HOST_ADDR6 = support.HOSTv6
+# Not all TCP stacks support dual binding where ''
+# binds to both v4 and v6.
+DEFAULT_BIND_ADDR = support.HOST
 
-if RUNNING_ON_TRAVIS or OSX:
-    # As of November 2017 (probably Sept or Oct), after a
-    # Travis upgrade, using "localhost" no longer works,
-    # producing 'OSError: [Errno 99] Cannot assign
-    # requested address'. This is apparently something to do with
-    # docker containers. Sigh.
-
-    # OSX 10.14.3 is also happier using explicit addresses
-    DEFAULT_LOCAL_HOST_ADDR = '127.0.0.1'
-    DEFAULT_LOCAL_HOST_ADDR6 = '::1'
-    # Likewise, binding to '' appears to work, but it cannot be
-    # connected to with the same error.
-    DEFAULT_BIND_ADDR = '127.0.0.1'
-
-if RUNNING_ON_APPVEYOR:
-    DEFAULT_BIND_ADDR = '127.0.0.1'
-    DEFAULT_LOCAL_HOST_ADDR = '127.0.0.1'
-
-
-if RESOLVER_ARES and OSX:
-    # Ares likes to raise "malformed domain name" on '', at least
-    # on OS X
-    DEFAULT_BIND_ADDR = '127.0.0.1'
 
 DEFAULT_CONNECT = DEFAULT_LOCAL_HOST_ADDR
 DEFAULT_BIND_ADDR_TUPLE = (DEFAULT_BIND_ADDR, 0)

src/gevent/testing/support.py

+# Copyright (c) 2018 gevent community
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+
+# A re-export of the support module from Python's test package, with some
+# compatibility shims.
+
+# Note that this imports a lot of modules.
+
+# pylint:disable=wildcard-import,unused-wildcard-import
+
+try:
+    # Renamed from test_support in Python 3,
+    # *and* in 2.7.14 (but with a BWC module)
+    from test.support import *
+except ImportError:
+    from test.test_support import *
+
+try:
+    HOSTv6
+except NameError:
+    HOSTv6 = '::1'

src/gevent/tests/test__refcount.py

@@ -23,6 +23,7 @@
 are not leaked by the hub.
 """
 from __future__ import print_function
+
 from _socket import socket as c_socket
 import sys
 if sys.version_info[0] >= 3:
@@ -40,11 +41,14 @@ else:
 import _socket
 _socket.socket = Socket
 
-import gevent.testing as greentest
+
 from gevent import monkey; monkey.patch_all()
-from gevent.testing import flaky
 
-from pprint import pformat
+import gevent.testing as greentest
+from gevent.testing import support
+from gevent.testing import params
+
+
 try:
     from thread import start_new_thread
 except ImportError:
@@ -60,101 +64,116 @@ SOCKET_TIMEOUT = 0.1
 if greentest.RUNNING_ON_CI:
     SOCKET_TIMEOUT *= 2
 
-def init_server():
-    s = socket.socket()
-    s.settimeout(SOCKET_TIMEOUT)
-    s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-    s.bind(('127.0.0.1', 0))
-    s.listen(5)
-    return s
 
+class Server(object):
+
+    listening = False
+    client_data = None
+    server_port = None
 
-def handle_request(s, raise_on_timeout):
+    def __init__(self, raise_on_timeout):
+        self.raise_on_timeout = raise_on_timeout
+        self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         try:
-        conn, _ = s.accept()
+            self.server_port = support.bind_port(self.socket, params.DEFAULT_BIND_ADDR)
+        except:
+            self.close()
+            raise
+
+    def close(self):
+        self.socket.close()
+        self.socket = None
+
+    def handle_request(self):
+        try:
+            self.socket.settimeout(SOCKET_TIMEOUT)
+
+            self.socket.listen(5)
+
+            self.listening = True
+
+            try:
+                conn, _ = self.socket.accept()
             except socket.timeout:
-        if raise_on_timeout:
+                if self.raise_on_timeout:
                     raise
                 return
-    #print('handle_request - accepted')
-    res = conn.recv(100)
-    assert res == b'hello', repr(res)
-    #print('handle_request - recvd %r' % res)
-    res = conn.send(b'bye')
-    #print('handle_request - sent %r' % res)
-    #print('handle_request - conn refcount: %s' % sys.getrefcount(conn))
+
+            try:
+                self.client_data = conn.recv(100)
+                conn.send(b'bye')
+            finally:
                 conn.close()
+        finally:
+            self.close()
 
 
-def make_request(port):
-    #print('make_request')
-    s = socket.socket()
-    s.connect(('127.0.0.1', port))
-    #print('make_request - connected')
-    res = s.send(b'hello')
-    #print('make_request - sent %s' % res)
-    res = s.recv(100)
-    assert res == b'bye', repr(res)
-    #print('make_request - recvd %r' % res)
-    s.close()
+class Client(object):
 
+    server_data = None
 
-def run_interaction(run_client):
-    s = init_server()
-    start_new_thread(handle_request, (s, run_client))
-    if run_client:
-        port = s.getsockname()[1]
-        start_new_thread(make_request, (port, ))
-    sleep(0.1 + SOCKET_TIMEOUT)
-    #print(sys.getrefcount(s._sock))
-    #s.close()
-    w = weakref.ref(s._sock)
-    s.close()
-    if greentest.WIN:
-        # The background thread may not have even had a chance to run
-        # yet, sleep again to be sure it does. Otherwise there could be
-        # strong refs to the socket still around.
+    def __init__(self, server_port):
+        self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        self.server_port = server_port
+
+
+    def close(self):
+        self.socket.close()
+        self.socket = None
+
+    def make_request(self):
         try:
-            sleep(0.1 + SOCKET_TIMEOUT)
-        except Exception: # pylint:disable=broad-except
-            pass
-
-    return w
-
-
-def run_and_check(run_client):
-    w = run_interaction(run_client=run_client)
-    if greentest.PYPY:
-        # PyPy doesn't use a strict ref counting and must
-        # run a gc, but the object should be gone
-        gc.collect()
-    if w():
-        print(pformat(gc.get_referrers(w())))
-        for x in gc.get_referrers(w()):
-            print(pformat(x))
-            for y in gc.get_referrers(x):
-                print('-', pformat(y))
-        raise AssertionError('server should be dead by now')
+            self.socket.connect((params.DEFAULT_CONNECT, self.server_port))
+            self.socket.send(b'hello')
+            self.server_data = self.socket.recv(100)
+        finally:
+            self.close()
 
 
-@greentest.skipOnCI("Often fail with timeouts or force closed connections; not sure why.")
-@greentest.skipIf(
-    greentest.RUN_LEAKCHECKS and greentest.PY3,
-    "Often fail with force closed connections; not sure why. "
-)
 class Test(greentest.TestCase):
-
     __timeout__ = greentest.LARGE_TIMEOUT
 
-    @flaky.reraises_flaky_timeout(socket.timeout)
+    def run_interaction(self, run_client):
+        server = Server(raise_on_timeout=run_client)
+        wref_to_hidden_server_socket = weakref.ref(server.socket._sock)
+        client = None
+        start_new_thread(server.handle_request)
+        if run_client:
+            client = Client(server.server_port)
+            start_new_thread(client.make_request)
+
+        # Wait until we do our business
+        while server.socket is not None:
+            sleep(0.01)
+
+        # If we have a client, then we should have data
+        if run_client:
+            self.assertEqual(server.client_data, b'hello')
+            self.assertEqual(client.server_data, b'bye')
+
+        return wref_to_hidden_server_socket
+
+
+    def run_and_check(self, run_client):
+        wref_to_hidden_server_socket = self.run_interaction(run_client=run_client)
+        greentest.gc_collect_if_needed()
+        if wref_to_hidden_server_socket():
+            from pprint import pformat
+            print(pformat(gc.get_referrers(wref_to_hidden_server_socket())))
+            for x in gc.get_referrers(wref_to_hidden_server_socket()):
+                print(pformat(x))
+                for y in gc.get_referrers(x):
+                    print('-', pformat(y))
+            self.fail('server socket should be dead by now')
+
     def test_clean_exit(self):
-        run_and_check(True)
-        run_and_check(True)
+        self.run_and_check(True)
+        self.run_and_check(True)
+
 
-    @flaky.reraises_flaky_timeout(socket.timeout)
     def test_timeout_exit(self):
-        run_and_check(False)
-        run_and_check(False)
+        self.run_and_check(False)
+        self.run_and_check(False)
 
 
 if __name__ == '__main__':

src/gevent/tests/test__socket.py

+# This line can be commented out so that most tests run with the
+# system socket for comparison.
 from gevent import monkey; monkey.patch_all()
 
 import sys
@@ -9,8 +11,10 @@ import time
 import unittest
 import gevent.testing as greentest
 from functools import wraps
+
 from gevent.testing import six
 from gevent.testing import LARGE_TIMEOUT
+from gevent.testing import support
 
 # we use threading on purpose so that we can test both regular and gevent sockets with the same code
 from threading import Thread as _Thread
@@ -294,24 +298,30 @@ class TestTCP(greentest.TestCase):
     def test_connect_ex_nonblocking_bad_connection(self):
         # Issue 841
         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        try:
             s.setblocking(False)
-        ret = s.connect_ex((greentest.DEFAULT_LOCAL_HOST_ADDR, get_port()))
+            ret = s.connect_ex((greentest.DEFAULT_LOCAL_HOST_ADDR, support.find_unused_port()))
             self.assertIsInstance(ret, errno_types)
+        finally:
             s.close()
 
     def test_connect_ex_gaierror(self):
         # Issue 841
         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        try:
             with self.assertRaises(socket.gaierror):
-            s.connect_ex(('foo.bar.fizzbuzz', get_port()))
+                s.connect_ex(('foo.bar.fizzbuzz', support.find_unused_port()))
+        finally:
             s.close()
 
     def test_connect_ex_nonblocking_overflow(self):
         # Issue 841
         s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+        try:
             s.setblocking(False)
             with self.assertRaises(OverflowError):
                 s.connect_ex((greentest.DEFAULT_LOCAL_HOST_ADDR, 65539))
+        finally:
             s.close()
 
     @unittest.skipUnless(hasattr(socket, 'SOCK_CLOEXEC'),
@@ -340,25 +350,34 @@ class TestTCP(greentest.TestCase):
 
         acceptor.join()
 
-def get_port():
-    tempsock = socket.socket()
-    tempsock.bind(('', 0))
-    port = tempsock.getsockname()[1]
-    tempsock.close()
-    return port
-
 
 class TestCreateConnection(greentest.TestCase):
 
     __timeout__ = LARGE_TIMEOUT
 
-    def test_refuses(self):
-        with self.assertRaises(socket.error) as cm:
-            socket.create_connection((greentest.DEFAULT_BIND_ADDR, get_port()),
+    def test_refuses(self, **conn_args):
+        connect_port = support.find_unused_port()
+        with self.assertRaisesRegex(
+                socket.error,
+                # We really expect "connection refused". It's unclear
+                # where/why we would get '[errno -2] name or service not known'
+                # but it seems some systems generate that.
+                # https://github.com/gevent/gevent/issues/1389
+                'refused|not known'
+        ):
+            socket.create_connection(
+                (greentest.DEFAULT_BIND_ADDR, connect_port),
                 timeout=30,
-                                     source_address=('', get_port()))
-        ex = cm.exception
-        self.assertIn('refused', str(ex).lower())
+                **conn_args
+            )
+
+    def test_refuses_from_port(self):
+        source_port = support.find_unused_port()
+        # Usually we don't want to bind/connect to '', but
+        # using it as the source is required if we don't want to hang,
+        # at least on some systems (OS X)
+        self.test_refuses(source_address=('', source_port))
+
 
     @greentest.ignores_leakcheck
     def test_base_exception(self):

src/gevent/tests/test__socket_dns6.py

@@ -2,38 +2,51 @@
 # -*- coding: utf-8 -*-
 from __future__ import print_function, absolute_import, division
 
-import gevent.testing as greentest
 import socket
+import unittest
+
+import gevent.testing as greentest
 from gevent.tests.test__socket_dns import TestCase, add
 
+from gevent.testing.sysinfo import OSX
 from gevent.testing.sysinfo import RESOLVER_NOT_SYSTEM
 from gevent.testing.sysinfo import RESOLVER_DNSPYTHON
 
-if not greentest.RUNNING_ON_CI and not RESOLVER_DNSPYTHON:
 
 
-    # We can't control the DNS servers we use there
-    # for the system. This works best with the google DNS servers
-    # The getnameinfo test can fail on CI.
+# We can't control the DNS servers on CI (or in general...)
+# for the system. This works best with the google DNS servers
+# The getnameinfo test can fail on CI.
 
-    # Previously only Test6_ds failed, but as of Jan 2018, Test6
-    # and Test6_google begin to fail:
+# Previously only Test6_ds failed, but as of Jan 2018, Test6
+# and Test6_google begin to fail:
 
-      # First differing element 0:
-    # 'vm2.test-ipv6.com'
-    # 'ip119.gigo.com'
+# First differing element 0:
+# 'vm2.test-ipv6.com'
+# 'ip119.gigo.com'
 
-    # - ('vm2.test-ipv6.com', [], ['2001:470:1:18::125'])
-    # ?   ---------  ^^                             ^^
+# - ('vm2.test-ipv6.com', [], ['2001:470:1:18::125'])
+# ?   ---------  ^^                             ^^
 
-    # + ('ip119.gigo.com', [], ['2001:470:1:18::119'])
-    # ?     ^^^^^^^^                             ^^
+# + ('ip119.gigo.com', [], ['2001:470:1:18::119'])
+# ?     ^^^^^^^^                             ^^
 
-    class Test6(TestCase):
+# These are known to work on jamadden's OS X machine using the google
+# resolvers (but not with DNSPython; things don't *quite* match)...so
+# by default we skip the tests everywhere else.
+
+class Test6(TestCase):
 
     # host that only has AAAA record
     host = 'aaaa.test-ipv6.com'
 
+    if not OSX or RESOLVER_DNSPYTHON:
+        def _test(self, *args): # pylint:disable=arguments-differ
+            raise unittest.SkipTest(
+                "Only known to work on jamadden's machine. "
+                "Please help investigate and make DNS tests more robust."
+            )
+
     def test_empty(self):
         self._test('getaddrinfo', self.host, 'http')
 
@@ -47,7 +60,7 @@ if not greentest.RUNNING_ON_CI and not RESOLVER_DNSPYTHON:
         self._test('getaddrinfo', self.host, None, socket.AF_UNSPEC)
 
 
-    class Test6_google(Test6):
+class Test6_google(Test6):
     host = 'ipv6.google.com'
 
     def _normalize_result_getnameinfo(self, result):
@@ -57,12 +70,12 @@ if not greentest.RUNNING_ON_CI and not RESOLVER_DNSPYTHON:
             return ()
         return result
 
-    add(Test6, Test6.host)
-    add(Test6_google, Test6_google.host)
+add(Test6, Test6.host)
+add(Test6_google, Test6_google.host)
 
 
 
-    class Test6_ds(Test6):
+class Test6_ds(Test6):
     # host that has both A and AAAA records
     host = 'ds.test-ipv6.com'
 
@@ -74,7 +87,7 @@ if not greentest.RUNNING_ON_CI and not RESOLVER_DNSPYTHON:
 
     _normalize_result_gethostbyname = _normalize_result_gethostbyaddr
 
-    add(Test6_ds, Test6_ds.host)
+add(Test6_ds, Test6_ds.host)
 
 
 if __name__ == '__main__':