Add tests for early and outgoing packet listeners.

minecraft/networking/connection.py

@@ -315,8 +315,13 @@ class Connection(object):
 
     def disconnect(self):
         """ Terminate the existing server connection, if there is one. """
-        if self.networking_thread is not None:
         with self._write_lock:  # pylint: disable=not-context-manager
+            if self.socket is not None:
+                # Flush any packets remaining in the queue.
+                while self._pop_packet():
+                    pass
+
+            if self.networking_thread is not None:
                 self.networking_thread.interrupt = True
 
         if self.socket is not None:

tests/fake_server.py

@@ -23,7 +23,7 @@ import uuid
 VERSIONS = sorted(SUPPORTED_MINECRAFT_VERSIONS.items(), key=lambda i: i[1])
 VERSIONS = [v for (v, p) in VERSIONS]
 
-THREAD_TIMEOUT_S = 5
+THREAD_TIMEOUT_S = 2
 
 
 class FakeClientDisconnect(Exception):
@@ -79,7 +79,6 @@ class FakeClientHandler(object):
         self.write_packet(clientbound.play.JoinGamePacket(
             entity_id=0, game_mode=0, dimension=0, difficulty=2, max_players=1,
             level_type='default', reduced_debug_info=False))
-        raise FakeServerDisconnect
 
     def handle_play_packet(self, packet):
         # Called upon each packet received after handle_play_start() returns.
@@ -247,8 +246,8 @@ class FakeServer(object):
 
         The server listens on a local TCP socket and accepts client connections
         in serial, in a single-threaded manner. It responds to status queries,
-        performs handshake and login, and, by default, immediately cleanly
-        disconnects the client after they join the game.
+        performs handshake and login, and, by default, echoes any chat messages
+        back to the client until it disconnects.1~
 
         The behaviour of the server can be customised by writing subclasses of
         FakeClientHandler, overriding its public methods of the form
@@ -383,7 +382,8 @@ class _FakeServerTest(unittest.TestCase):
             client_handler_type = self.client_handler_type
 
         server = FakeServer(minecraft_version=server_version,
-                            compression_threshold=compression_threshold)
+                            compression_threshold=compression_threshold,
+                            client_handler_type=client_handler_type)
         addr = "localhost"
         port = server.listen_socket.getsockname()[1]
 
@@ -432,7 +432,9 @@ class _FakeServerTest(unittest.TestCase):
                         errors.append({
                             'msg': 'Thread "%s" timed out.' % thread.name})
         except:
-            errors.insert(0, sys.exc_info())
+            errors.insert(0, {
+                'msg': 'Exception in main thread',
+                'exc_info': sys.exc_info()})
         else:
             timeout = True
             for lock, [exc_info], thread_name in (
@@ -448,7 +450,7 @@ class _FakeServerTest(unittest.TestCase):
                             'exc_info': exc_info})
                     timeout = False
             if timeout:
-                errors.insert(0, {'msg': 'Timed out.'})
+                errors.insert(0, {'msg': 'Test timed out.'})
 
         if len(errors) > 1:
             for error in errors:

tests/test_connection.py

@@ -1,11 +1,19 @@
-from . import fake_server
 from minecraft import SUPPORTED_MINECRAFT_VERSIONS
+from minecraft.networking.packets import clientbound, serverbound
+from minecraft.networking.connection import IgnorePacket
+
+from . import fake_server
 
 
 class ConnectTest(fake_server._FakeServerTest):
     def test_connect(self):
         self._test_connect()
 
+    class client_handler_type(fake_server.FakeClientHandler):
+        def handle_play_start(self):
+            super(ConnectTest.client_handler_type, self).handle_play_start()
+            raise fake_server.FakeServerDisconnect
+
 
 class PingTest(ConnectTest):
     def _start_client(self, client):
@@ -24,11 +32,13 @@ class ConnectCompressionHighTest(ConnectTest):
 
 
 class AllowedVersionsTest(fake_server._FakeServerTest):
-    VERSIONS = sorted(SUPPORTED_MINECRAFT_VERSIONS.items(), key=lambda p: p[1])
-    VERSIONS = dict((VERSIONS[0], VERSIONS[len(VERSIONS)//2], VERSIONS[-1]))
+    versions = sorted(SUPPORTED_MINECRAFT_VERSIONS.items(), key=lambda p: p[1])
+    versions = dict((versions[0], versions[len(versions)//2], versions[-1]))
+
+    client_handler_type = ConnectTest.client_handler_type
 
     def test_with_version_names(self):
-        for version, proto in AllowedVersionsTest.VERSIONS.items():
+        for version, proto in AllowedVersionsTest.versions.items():
             client_versions = {
                 v for (v, p) in SUPPORTED_MINECRAFT_VERSIONS.items()
                 if p <= proto}
@@ -36,9 +46,139 @@ class AllowedVersionsTest(fake_server._FakeServerTest):
                 server_version=version, client_versions=client_versions)
 
     def test_with_protocol_numbers(self):
-        for version, proto in AllowedVersionsTest.VERSIONS.items():
+        for version, proto in AllowedVersionsTest.versions.items():
             client_versions = {
                 p for (v, p) in SUPPORTED_MINECRAFT_VERSIONS.items()
                 if p <= proto}
             self._test_connect(
                 server_version=version, client_versions=client_versions)
+
+
+class EarlyPacketListenerTest(ConnectTest):
+    """ Early packet listeners should be called before ordinary ones, even when
+        the early packet listener is registered afterwards.
+    """
+    def _start_client(self, client):
+        def handle_join(packet):
+            assert early_handle_join.called, \
+                   'Ordinary listener called before early listener.'
+            handle_join.called = True
+        handle_join.called = False
+        client.register_packet_listener(
+            handle_join, clientbound.play.JoinGamePacket)
+
+        def early_handle_join(packet):
+            early_handle_join.called = True
+        client.register_packet_listener(
+            early_handle_join, clientbound.play.JoinGamePacket, early=True)
+        early_handle_join.called = False
+
+        def handle_disconnect(packet):
+            assert early_handle_join.called, 'Early listener not called.'
+            assert handle_join.called, 'Ordinary listener not called.'
+            raise fake_server.FakeServerTestSuccess
+        client.register_packet_listener(
+            handle_disconnect, clientbound.play.DisconnectPacket)
+
+        client.connect()
+
+
+class IgnorePacketTest(ConnectTest):
+    """ Raising 'minecraft.networking.connection.IgnorePacket' from within a
+        packet listener should prevent any subsequent packet listeners from
+        being called, and, if the listener is early, should prevent the default
+        behaviour from being triggered.
+    """
+
+    def _start_client(self, client):
+        keep_alive_ids_incoming = []
+        keep_alive_ids_outgoing = []
+
+        def handle_keep_alive_1(packet):
+            keep_alive_ids_incoming.append(packet.keep_alive_id)
+            if packet.keep_alive_id == 1:
+                raise IgnorePacket
+        client.register_packet_listener(
+            handle_keep_alive_1, clientbound.play.KeepAlivePacket, early=True)
+
+        def handle_keep_alive_2(packet):
+            keep_alive_ids_incoming.append(packet.keep_alive_id)
+            assert packet.keep_alive_id > 1
+            if packet.keep_alive_id == 2:
+                raise IgnorePacket
+        client.register_packet_listener(
+            handle_keep_alive_2, clientbound.play.KeepAlivePacket)
+
+        def handle_keep_alive_3(packet):
+            keep_alive_ids_incoming.append(packet.keep_alive_id)
+            assert packet.keep_alive_id == 3
+        client.register_packet_listener(
+            handle_keep_alive_3, clientbound.play.KeepAlivePacket)
+
+        def handle_outgoing_keep_alive_2(packet):
+            keep_alive_ids_outgoing.append(packet.keep_alive_id)
+            assert 2 <= packet.keep_alive_id <= 3
+            if packet.keep_alive_id == 2:
+                raise IgnorePacket
+        client.register_packet_listener(
+            handle_outgoing_keep_alive_2, serverbound.play.KeepAlivePacket,
+            outgoing=True, early=True)
+
+        def handle_outgoing_keep_alive_3(packet):
+            keep_alive_ids_outgoing.append(packet.keep_alive_id)
+            assert packet.keep_alive_id == 3
+            raise IgnorePacket
+        client.register_packet_listener(
+            handle_outgoing_keep_alive_3, serverbound.play.KeepAlivePacket,
+            outgoing=True)
+
+        def handle_outgoing_keep_alive_none(packet):
+            keep_alive_ids_outgoing.append(packet.keep_alive_id)
+            assert False
+        client.register_packet_listener(
+            handle_outgoing_keep_alive_none, serverbound.play.KeepAlivePacket,
+            outgoing=True)
+
+        def handle_disconnect(packet):
+            assert keep_alive_ids_incoming == [1, 2, 2, 3, 3, 3], \
+                'Incoming keep-alive IDs %r != %r' % \
+                (keep_alive_ids_incoming, [1, 2, 2, 3, 3, 3])
+            assert keep_alive_ids_outgoing == [2, 3, 3], \
+                'Outgoing keep-alive IDs %r != %r' % \
+                (keep_alive_ids_incoming, [2, 3, 3])
+        client.register_packet_listener(
+            handle_disconnect, clientbound.play.DisconnectPacket)
+
+        client.connect()
+
+    class client_handler_type(fake_server.FakeClientHandler):
+        __slots__ = '_keep_alive_ids_returned'
+
+        def __init__(self, *args, **kwds):
+            super(IgnorePacketTest.client_handler_type, self).__init__(
+                *args, **kwds)
+            self._keep_alive_ids_returned = []
+
+        def handle_play_start(self):
+            super(IgnorePacketTest.client_handler_type, self)\
+                .handle_play_start()
+            self.write_packet(clientbound.play.KeepAlivePacket(
+                keep_alive_id=1))
+            self.write_packet(clientbound.play.KeepAlivePacket(
+                keep_alive_id=2))
+            self.write_packet(clientbound.play.KeepAlivePacket(
+                keep_alive_id=3))
+            self.write_packet(clientbound.play.DisconnectPacket(
+                json_data='{"text":"Test complete."}'))
+
+        def handle_play_packet(self, packet):
+            super(IgnorePacketTest.client_handler_type, self) \
+                .handle_play_packet(packet)
+            if isinstance(packet, serverbound.play.KeepAlivePacket):
+                self._keep_alive_ids_returned.append(packet.keep_alive_id)
+
+        def handle_play_client_disconnect(self):
+            assert self._keep_alive_ids_returned == [3], \
+                   'Returned keep-alive IDs %r != %r' % \
+                   (self._keep_alive_ids_returned, [3])
+            raise fake_server.FakeServerTestSuccess