Exclude test_suite() from pytest collection (fixes #481)

tests/test_regression.py

@@ -11,136 +11,212 @@
 # FOR A PARTICULAR PURPOSE.
 #
 ##############################################################################
-"""Tests for waitress.channel maintenance logic"""
+"""Tests for waitress.channel maintenance logic
 
-import doctest
+Regression test for HTTPChannel.maintenance: channels that have been
+"inactive" for a configured time get closed. The bug was that
+last_activity is set at creation time but never updated during async
+channel activity (reads and writes), so any channel older than the
+configured timeout would be marked for closing when maintenance ran,
+regardless of activity.
+
+This used to be a single doctest (see git history), written against a
+Python 2-only API (tuple-unpacking ``bind`` parameters, print
+statements) that could not run under Python 3 at all, and that pytest
+additionally warned about collecting via its ``test_suite()`` wrapper
+(see GH #481). It is rewritten below as ordinary pytest functions
+against the current API.
+"""
+
+import socket
+import time
+
+from waitress.server import create_server
+
+dummy_app = object()
+
+
+class FakeListenSocket(socket.socket):
+    """Stand-in for the listening socket create_server() binds to. Only
+    used because _start=False and _sock is supplied still exercises the
+    normal socket setup path (set_reuse_addr, etc.) -- nothing here is
+    ever actually connected to."""
+
+    family = socket.AF_INET
+    type = socket.SOCK_STREAM
+    proto = 0
+
+    def __init__(self):
+        self.bound = None
+        self.opts = []
+
+    def bind(self, addr):
+        self.bound = addr
+
+    def listen(self, num):
+        self.listened = num
+
+    def getsockname(self):
+        return self.bound
+
+    def setsockopt(self, *arg):
+        self.opts.append(arg)
+
+    def getsockopt(self, *arg):
+        return 1
+
+    def setblocking(self, *_):
+        pass
+
+    def fileno(self):
+        return 10
+
+    def getpeername(self):
+        return "127.0.0.1"
+
+    def close(self):
+        pass
 
 
 class FakeSocket:  # pragma: no cover
-    data = ""
-    setblocking = lambda *_: None
-    close = lambda *_: None
+    """Minimal socket stand-in, just enough to construct a real
+    HTTPChannel and drive it through handle_read()/handle_write()."""
 
     def __init__(self, no):
         self.no = no
+        self.to_recv = b""
+        self.sent = b""
 
     def fileno(self):
         return self.no
 
     def getpeername(self):
         return ("localhost", self.no)
 
-    def send(self, data):
-        self.data += data
-        return len(data)
-
-    def recv(self, data):
-        return "data"
+    def getsockopt(self, level, optname):
+        return 2048
 
+    def setblocking(self, *_):
+        pass
 
-def zombies_test():
-    """Regression test for HTTPChannel.maintenance method
+    def close(self):
+        pass
 
-    Bug: This method checks for channels that have been "inactive" for a
-    configured time. The bug was that last_activity is set at creation time
-    but never updated during async channel activity (reads and writes), so
-    any channel older than the configured timeout will be closed when a new
-    channel is created, regardless of activity.
+    def send(self, data):
+        self.sent += data
+        return len(data)
 
-    >>> import time
-    >>> import waitress.adjustments
-    >>> config = waitress.adjustments.Adjustments()
+    def recv(self, buffer_size):
+        result = self.to_recv[:buffer_size]
+        self.to_recv = self.to_recv[buffer_size:]
+        return result
 
-    >>> from waitress.server import HTTPServer
-    >>> class TestServer(HTTPServer):
-    ...     def bind(self, (ip, port)):
-    ...         print "Listening on %s:%d" % (ip or '*', port)
-    >>> sb = TestServer('127.0.0.1', 80, start=False, verbose=True)
-    Listening on 127.0.0.1:80
 
-    First we confirm the correct behavior, where a channel with no activity
-    for the timeout duration gets closed.
+class DummyTaskDispatcher:
+    def __init__(self):
+        self.tasks = []
 
-    >>> from waitress.channel import HTTPChannel
-    >>> socket = FakeSocket(42)
-    >>> channel = HTTPChannel(sb, socket, ('localhost', 42))
+    def add_task(self, task):
+        self.tasks.append(task)
 
-    >>> channel.connected
-    True
+    def shutdown(self):
+        self.was_shutdown = True
 
-    >>> channel.last_activity -= int(config.channel_timeout) + 1
 
-    >>> channel.next_channel_cleanup[0] = channel.creation_time - int(
-    ...     config.cleanup_interval) - 1
+def _make_server(map, channel_timeout=120, cleanup_interval=30):
+    """A real TcpWSGIServer, not started, backed by a FakeSocket so no
+    actual socket is bound."""
+    return create_server(
+        dummy_app,
+        host="127.0.0.1",
+        port=0,
+        map=map,
+        _sock=FakeListenSocket(),
+        _dispatcher=DummyTaskDispatcher(),
+        _start=False,
+        channel_timeout=channel_timeout,
+        cleanup_interval=cleanup_interval,
+    )
 
-    >>> socket2 = FakeSocket(7)
-    >>> channel2 = HTTPChannel(sb, socket2, ('localhost', 7))
 
-    >>> channel.connected
-    False
+def _make_channel(server, no, map):
+    from waitress.channel import HTTPChannel
 
-    Write Activity
-    --------------
+    sock = FakeSocket(no)
+    channel = HTTPChannel(server, sock, ("localhost", no), adj=server.adj, map=map)
+    server.active_channels[no] = channel
 
-    Now we make sure that if there is activity the channel doesn't get closed
-    incorrectly.
+    return channel, sock
 
-    >>> channel2.connected
-    True
 
-    >>> channel2.last_activity -= int(config.channel_timeout) + 1
+def test_maintenance_closes_inactive_channel():
+    """A channel with no activity for the timeout duration gets marked
+    for closing by maintenance."""
+    map = {}
+    server = _make_server(map, channel_timeout=1)
+    channel, _ = _make_channel(server, 42, map)
 
-    >>> channel2.handle_write()
+    assert channel.will_close is False
 
-    >>> channel2.next_channel_cleanup[0] = channel2.creation_time - int(
-    ...     config.cleanup_interval) - 1
+    channel.last_activity -= server.adj.channel_timeout + 1
+    server.maintenance(time.time())
 
-    >>> socket3 = FakeSocket(3)
-    >>> channel3 = HTTPChannel(sb, socket3, ('localhost', 3))
+    assert channel.will_close is True
 
-    >>> channel2.connected
-    True
 
-    Read Activity
-    --------------
+def test_maintenance_write_activity_prevents_close():
+    """A channel that has had write activity since it went "idle" must
+    not be marked for closing, even though it's older than the
+    timeout."""
+    map = {}
+    server = _make_server(map, channel_timeout=1)
+    channel, sock = _make_channel(server, 7, map)
 
-    We should test to see that read activity will update a channel as well.
+    channel.last_activity -= server.adj.channel_timeout + 1
 
-    >>> channel3.connected
-    True
+    # Give it something to flush, then flush it -- this is what bumps
+    # last_activity on the write path.
+    channel.total_outbufs_len = 1
+    channel.outbufs[0].append(b"data")
+    channel.handle_write()
 
-    >>> channel3.last_activity -= int(config.channel_timeout) + 1
+    server.maintenance(time.time())
 
-    >>> import waitress.parser
-    >>> channel3.parser_class = (
-    ...    waitress.parser.HTTPRequestParser)
-    >>> channel3.handle_read()
+    assert channel.will_close is False
 
-    >>> channel3.next_channel_cleanup[0] = channel3.creation_time - int(
-    ...     config.cleanup_interval) - 1
 
-    >>> socket4 = FakeSocket(4)
-    >>> channel4 = HTTPChannel(sb, socket4, ('localhost', 4))
+def test_maintenance_read_activity_prevents_close():
+    """A channel that has had read activity since it went "idle" must
+    not be marked for closing, even though it's older than the
+    timeout."""
+    map = {}
+    server = _make_server(map, channel_timeout=1)
+    channel, sock = _make_channel(server, 3, map)
 
-    >>> channel3.connected
-    True
+    channel.last_activity -= server.adj.channel_timeout + 1
 
-    Main loop window
-    ----------------
+    # A single byte of inbound data is enough to bump last_activity via
+    # handle_read(), without needing a complete HTTP request.
+    sock.to_recv = b"G"
+    channel.handle_read()
 
-    There is also a corner case we'll do a shallow test for where a
-    channel can be closed waiting for the main loop.
+    server.maintenance(time.time())
 
-    >>> channel4.last_activity -= 1
+    assert channel.will_close is False
 
-    >>> last_active = channel4.last_activity
 
-    >>> channel4.set_async()
+def test_maintenance_leaves_channel_with_pending_requests_alone():
+    """A channel currently servicing a request must never be marked
+    for closing by maintenance, regardless of last_activity -- matching
+    the "main loop window" case from the original regression: activity
+    can still be in flight even if the timestamp itself is stale."""
+    map = {}
+    server = _make_server(map, channel_timeout=1)
+    channel, _ = _make_channel(server, 4, map)
 
-    >>> channel4.last_activity != last_active
-    True
-    """
+    channel.last_activity -= server.adj.channel_timeout + 1
+    channel.requests = [object()]
 
+    server.maintenance(time.time())
 
-def test_suite():
-    return doctest.DocTestSuite()
+    assert channel.will_close is False