fixup: Merge the MockGDBServer packetLog into the existing MockGDBServerResponder's packetLog

Author: dsandersllvm

lldb/packages/Python/lldbsuite/test/gdbclientutils.py

@@ -7,7 +7,7 @@
 import traceback
 from enum import Enum
 from lldbsuite.support import seven
-from typing import Optional, List, Tuple, Literal, Union, Sequence
+from typing import Optional, List, Tuple, Union, Sequence
 
 
 def checksum(message):
@@ -77,6 +77,35 @@ def hex_decode_bytes(hex_bytes):
     return out
 
 
+class PacketDirection(Enum):
+    RECV = "recv"
+    SEND = "send"
+
+
+class PacketLog:
+    def __init__(self):
+        self._packets: list[tuple[PacketDirection, str]] = []
+
+    def add_sent(self, packet: str):
+        self._packets.append((PacketDirection.SEND, packet))
+
+    def add_received(self, packet: str):
+        self._packets.append((PacketDirection.RECV, packet))
+
+    def get_sent(self):
+        return [
+            pkt for direction, pkt in self._packets if direction == PacketDirection.SEND
+        ]
+
+    def get_received(self):
+        return [
+            pkt for direction, pkt in self._packets if direction == PacketDirection.RECV
+        ]
+
+    def __iter__(self):
+        return iter(self._packets)
+
+
 class MockGDBServerResponder:
     """
     A base class for handling client packets and issuing server responses for
@@ -100,17 +129,21 @@ class SpecialResponse(Enum):
     type Response = Union[str, SpecialResponse]
 
     def __init__(self):
-        self.packetLog: List[str] = []
+        self.packetLog = PacketLog()
 
     def respond(self, packet: str) -> Sequence[Response]:
         """
         Return the unframed packet data that the server should issue in response
         to the given packet received from the client.
         """
-        self.packetLog.append(packet)
+        self.packetLog.add_received(packet)
         response = self._respond_impl(packet)
         if not isinstance(response, list):
             response = [response]
+        for part in response:
+            if isinstance(part, self.SpecialResponse):
+                continue
+            self.packetLog.add_sent(part)
         return response
 
     def _respond_impl(self, packet) -> Union[Response, List[Response]]:
@@ -529,12 +562,10 @@ class MockGDBServer:
     _receivedData = None
     _receivedDataOffset = None
     _shouldSendAck = True
-    packetLog: list[tuple[Literal["recv"] | Literal["send"], str | bytes]]
 
     def __init__(self, socket):
         self._socket = socket
         self.responder = MockGDBServerResponder()
-        self.packetLog = []
 
     def start(self):
         # Start a thread that waits for a client connection.
@@ -661,13 +692,11 @@ def _parsePacket(self):
         # can start on the next packet the next time around
         self._receivedData = data[i:]
         self._receivedDataOffset = 0
-        self.packetLog.append(("recv", packet))
         return packet
 
     def _sendPacket(self, packet: str):
         assert self._socket is not None
         framed_packet = seven.bitcast_to_bytes(frame_packet(packet))
-        self.packetLog.append(("send", framed_packet))
         self._socket.sendall(framed_packet)
 
     def _handlePacket(self, packet):

lldb/packages/Python/lldbsuite/test/lldbgdbclient.py

@@ -60,7 +60,7 @@ def connect(self, target, plugin="gdb-remote"):
         self.assertTrue(process, PROCESS_IS_VALID)
         return process
 
-    def assertPacketLogReceived(self, packets, log=None):
+    def assertPacketLogReceived(self, packets, log: PacketLog = None):
         """
         Assert that the mock server's packet log received the given packets.
 
@@ -72,18 +72,20 @@ def assertPacketLogReceived(self, packets, log=None):
         require that they are ordered in the log as they ordered in the arg.
         """
         if log is None:
-            log = self.server.responder.packetLog
+            received = self.server.responder.packetLog.get_received()
+        else:
+            received = log.get_received()
         i = 0
         j = 0
 
-        while i < len(packets) and j < len(log):
-            if log[j] == packets[i]:
+        while i < len(packets) and j < len(received):
+            if received[j] == packets[i]:
                 i += 1
             j += 1
         if i < len(packets):
             self.fail(
                 "Did not receive: %s\nLast 10 packets:\n\t%s"
-                % (packets[i], "\n\t".join(log))
+                % (packets[i], "\n\t".join(received))
             )
 
 

lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py

@@ -142,29 +142,24 @@ def test_read_registers_using_g_packets(self):
         # But there certainly should be no p packets after the g packet.
 
         self.read_registers(process)
-        print(f"\nPACKET LOG:\n{self.server.responder.packetLog}\n")
+        received = self.server.responder.packetLog.get_received()
+        print(f"\nPACKET LOG:\n{received}\n")
         g_pos = 0
         try:
-            g_pos = self.server.responder.packetLog.index("g")
+            g_pos = received.index("g")
         except err:
             self.fail("'g' packet not found after fetching registers")
 
         try:
-            second_g = self.server.responder.packetLog.index("g", g_pos)
+            second_g = received.index("g", g_pos + 1)
             self.fail("Found more than one 'g' packet")
         except:
             pass
 
         # Make sure there aren't any `p` packets after the `g` packet:
         self.assertEqual(
             0,
-            len(
-                [
-                    p
-                    for p in self.server.responder.packetLog[g_pos:]
-                    if p.startswith("p")
-                ]
-            ),
+            len([p for p in received[g_pos:] if p.startswith("p")]),
         )
 
     def test_read_registers_using_p_packets(self):
@@ -177,10 +172,9 @@ def test_read_registers_using_p_packets(self):
         process = self.connect(target)
 
         self.read_registers(process)
-        self.assertNotIn("g", self.server.responder.packetLog)
-        self.assertGreater(
-            len([p for p in self.server.responder.packetLog if p.startswith("p")]), 0
-        )
+        received = self.server.responder.packetLog.get_received()
+        self.assertNotIn("g", received)
+        self.assertGreater(len([p for p in received if p.startswith("p")]), 0)
 
     def test_write_registers_using_P_packets(self):
         """Test writing registers using 'P' packets (default behavior)"""
@@ -189,12 +183,9 @@ def test_write_registers_using_P_packets(self):
         process = self.connect(target)
 
         self.write_registers(process)
-        self.assertEqual(
-            0, len([p for p in self.server.responder.packetLog if p.startswith("G")])
-        )
-        self.assertGreater(
-            len([p for p in self.server.responder.packetLog if p.startswith("P")]), 0
-        )
+        received = self.server.responder.packetLog.get_received()
+        self.assertEqual(0, len([p for p in received if p.startswith("G")]))
+        self.assertGreater(len([p for p in received if p.startswith("P")]), 0)
 
     def test_write_registers_using_G_packets(self):
         """Test writing registers using 'G' packets"""
@@ -209,12 +200,9 @@ def readRegister(self, register):
         process = self.connect(target)
 
         self.write_registers(process)
-        self.assertEqual(
-            0, len([p for p in self.server.responder.packetLog if p.startswith("P")])
-        )
-        self.assertGreater(
-            len([p for p in self.server.responder.packetLog if p.startswith("G")]), 0
-        )
+        received = self.server.responder.packetLog.get_received()
+        self.assertEqual(0, len([p for p in received if p.startswith("P")]))
+        self.assertGreater(len([p for p in received if p.startswith("G")]), 0)
 
     def read_registers(self, process):
         self.for_each_gpr(