[lldb-dap] Reland refactor of DebugCommunication.

Originally commited in 362b9d7 and then reverted in cb63b75.

This reland a subset of the changes to dap_server.py/DebugCommunication and addresses the python3.10 comptability issue.

This includes less type annotations since those were the reason for the failures on that specific version of python.

I've done additionall testing on python3.8, python3.10 and python3.13 to further validate these changes.

Author: ashgti

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -1,6 +1,6 @@
 import os
 import time
-from typing import Optional
+from typing import Optional, Callable, Any, List, Union
 import uuid
 
 import dap_server
@@ -120,11 +120,19 @@ def wait_for_breakpoints_to_resolve(
             f"Expected to resolve all breakpoints. Unresolved breakpoint ids: {unresolved_breakpoints}",
         )
 
-    def waitUntil(self, condition_callback):
-        for _ in range(20):
-            if condition_callback():
+    def wait_until(
+        self,
+        predicate: Callable[[], bool],
+        delay: float = 0.5,
+        timeout: float = DEFAULT_TIMEOUT,
+    ) -> bool:
+        """Repeatedly run the predicate until either the predicate returns True
+        or a timeout has occurred."""
+        deadline = time.monotonic() + timeout
+        while deadline > time.monotonic():
+            if predicate():
                 return True
-            time.sleep(0.5)
+            time.sleep(delay)
         return False
 
     def assertCapabilityIsSet(self, key: str, msg: Optional[str] = None) -> None:
@@ -137,13 +145,16 @@ def assertCapabilityIsNotSet(self, key: str, msg: Optional[str] = None) -> None:
         if key in self.dap_server.capabilities:
             self.assertEqual(self.dap_server.capabilities[key], False, msg)
 
-    def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT):
+    def verify_breakpoint_hit(
+        self, breakpoint_ids: List[Union[int, str]], timeout: float = DEFAULT_TIMEOUT
+    ):
         """Wait for the process we are debugging to stop, and verify we hit
         any breakpoint location in the "breakpoint_ids" array.
         "breakpoint_ids" should be a list of breakpoint ID strings
         (["1", "2"]). The return value from self.set_source_breakpoints()
         or self.set_function_breakpoints() can be passed to this function"""
         stopped_events = self.dap_server.wait_for_stopped(timeout)
+        normalized_bp_ids = [str(b) for b in breakpoint_ids]
         for stopped_event in stopped_events:
             if "body" in stopped_event:
                 body = stopped_event["body"]
@@ -154,22 +165,16 @@ def verify_breakpoint_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT):
                     and body["reason"] != "instruction breakpoint"
                 ):
                     continue
-                if "description" not in body:
+                if "hitBreakpointIds" not in body:
                     continue
-                # Descriptions for breakpoints will be in the form
-                # "breakpoint 1.1", so look for any description that matches
-                # ("breakpoint 1.") in the description field as verification
-                # that one of the breakpoint locations was hit. DAP doesn't
-                # allow breakpoints to have multiple locations, but LLDB does.
-                # So when looking at the description we just want to make sure
-                # the right breakpoint matches and not worry about the actual
-                # location.
-                description = body["description"]
-                for breakpoint_id in breakpoint_ids:
-                    match_desc = f"breakpoint {breakpoint_id}."
-                    if match_desc in description:
+                hit_breakpoint_ids = body["hitBreakpointIds"]
+                for bp in hit_breakpoint_ids:
+                    if str(bp) in normalized_bp_ids:
                         return
-        self.assertTrue(False, f"breakpoint not hit, stopped_events={stopped_events}")
+        self.assertTrue(
+            False,
+            f"breakpoint not hit, wanted breakpoint_ids {breakpoint_ids} in stopped_events {stopped_events}",
+        )
 
     def verify_all_breakpoints_hit(self, breakpoint_ids, timeout=DEFAULT_TIMEOUT):
         """Wait for the process we are debugging to stop, and verify we hit
@@ -213,7 +218,7 @@ def verify_stop_exception_info(self, expected_description, timeout=DEFAULT_TIMEO
                     return True
         return False
 
-    def verify_commands(self, flavor, output, commands):
+    def verify_commands(self, flavor: str, output: str, commands: list[str]):
         self.assertTrue(output and len(output) > 0, "expect console output")
         lines = output.splitlines()
         prefix = "(lldb) "
@@ -226,10 +231,11 @@ def verify_commands(self, flavor, output, commands):
                     found = True
                     break
             self.assertTrue(
-                found, "verify '%s' found in console output for '%s'" % (cmd, flavor)
+                found,
+                f"Command '{flavor}' - '{cmd}' not found in output: {output}",
             )
 
-    def get_dict_value(self, d, key_path):
+    def get_dict_value(self, d: dict, key_path: list[str]) -> Any:
         """Verify each key in the key_path array is in contained in each
         dictionary within "d". Assert if any key isn't in the
         corresponding dictionary. This is handy for grabbing values from VS
@@ -298,28 +304,34 @@ def get_source_and_line(self, threadId=None, frameIndex=0):
                         return (source["path"], stackFrame["line"])
         return ("", 0)
 
-    def get_stdout(self, timeout=0.0):
-        return self.dap_server.get_output("stdout", timeout=timeout)
+    def get_stdout(self):
+        return self.dap_server.get_output("stdout")
 
-    def get_console(self, timeout=0.0):
-        return self.dap_server.get_output("console", timeout=timeout)
+    def get_console(self):
+        return self.dap_server.get_output("console")
 
-    def get_important(self, timeout=0.0):
-        return self.dap_server.get_output("important", timeout=timeout)
+    def get_important(self):
+        return self.dap_server.get_output("important")
 
-    def collect_stdout(self, timeout_secs, pattern=None):
+    def collect_stdout(
+        self, timeout: float = DEFAULT_TIMEOUT, pattern: Optional[str] = None
+    ) -> str:
         return self.dap_server.collect_output(
-            "stdout", timeout_secs=timeout_secs, pattern=pattern
+            "stdout", timeout=timeout, pattern=pattern
         )
 
-    def collect_console(self, timeout_secs, pattern=None):
+    def collect_console(
+        self, timeout: float = DEFAULT_TIMEOUT, pattern: Optional[str] = None
+    ) -> str:
         return self.dap_server.collect_output(
-            "console", timeout_secs=timeout_secs, pattern=pattern
+            "console", timeout=timeout, pattern=pattern
         )
 
-    def collect_important(self, timeout_secs, pattern=None):
+    def collect_important(
+        self, timeout: float = DEFAULT_TIMEOUT, pattern: Optional[str] = None
+    ) -> str:
         return self.dap_server.collect_output(
-            "important", timeout_secs=timeout_secs, pattern=pattern
+            "important", timeout=timeout, pattern=pattern
         )
 
     def get_local_as_int(self, name, threadId=None):

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

@@ -153,7 +153,7 @@ def test_commands(self):
         breakpoint_ids = self.set_function_breakpoints(functions)
         self.assertEqual(len(breakpoint_ids), len(functions), "expect one breakpoint")
         self.continue_to_breakpoints(breakpoint_ids)
-        output = self.collect_console(timeout_secs=10, pattern=stopCommands[-1])
+        output = self.collect_console(timeout=10, pattern=stopCommands[-1])
         self.verify_commands("stopCommands", output, stopCommands)
 
         # Continue after launch and hit the "pause()" call and stop the target.
@@ -163,7 +163,7 @@ def test_commands(self):
         time.sleep(0.5)
         self.dap_server.request_pause()
         self.dap_server.wait_for_stopped()
-        output = self.collect_console(timeout_secs=10, pattern=stopCommands[-1])
+        output = self.collect_console(timeout=10, pattern=stopCommands[-1])
         self.verify_commands("stopCommands", output, stopCommands)
 
         # Continue until the program exits
@@ -172,7 +172,7 @@ def test_commands(self):
         # "exitCommands" that were run after the second breakpoint was hit
         # and the "terminateCommands" due to the debugging session ending
         output = self.collect_console(
-            timeout_secs=10.0,
+            timeout=10.0,
             pattern=terminateCommands[0],
         )
         self.verify_commands("exitCommands", output, exitCommands)
@@ -223,7 +223,7 @@ def test_terminate_commands(self):
         # "terminateCommands"
         self.dap_server.request_disconnect(terminateDebuggee=True)
         output = self.collect_console(
-            timeout_secs=1.0,
+            timeout=1.0,
             pattern=terminateCommands[0],
         )
         self.verify_commands("terminateCommands", output, terminateCommands)

lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py

@@ -10,16 +10,14 @@
 
 
 class TestDAP_cancel(lldbdap_testcase.DAPTestCaseBase):
-    def send_async_req(self, command: str, arguments={}) -> int:
-        seq = self.dap_server.sequence
-        self.dap_server.send_packet(
+    def send_async_req(self, command: str, arguments: dict = {}) -> int:
+        return self.dap_server.send_packet(
             {
                 "type": "request",
                 "command": command,
                 "arguments": arguments,
             }
         )
-        return seq
 
     def async_blocking_request(self, duration: float) -> int:
         """
@@ -54,18 +52,18 @@ def test_pending_request(self):
         pending_seq = self.async_blocking_request(duration=self.DEFAULT_TIMEOUT / 2)
         cancel_seq = self.async_cancel(requestId=pending_seq)
 
-        blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
+        blocking_resp = self.dap_server.receive_response(blocking_seq)
         self.assertEqual(blocking_resp["request_seq"], blocking_seq)
         self.assertEqual(blocking_resp["command"], "evaluate")
         self.assertEqual(blocking_resp["success"], True)
 
-        pending_resp = self.dap_server.recv_packet(filter_type=["response"])
+        pending_resp = self.dap_server.receive_response(pending_seq)
         self.assertEqual(pending_resp["request_seq"], pending_seq)
         self.assertEqual(pending_resp["command"], "evaluate")
         self.assertEqual(pending_resp["success"], False)
         self.assertEqual(pending_resp["message"], "cancelled")
 
-        cancel_resp = self.dap_server.recv_packet(filter_type=["response"])
+        cancel_resp = self.dap_server.receive_response(cancel_seq)
         self.assertEqual(cancel_resp["request_seq"], cancel_seq)
         self.assertEqual(cancel_resp["command"], "cancel")
         self.assertEqual(cancel_resp["success"], True)
@@ -80,19 +78,16 @@ def test_inflight_request(self):
 
         blocking_seq = self.async_blocking_request(duration=self.DEFAULT_TIMEOUT / 2)
         # Wait for the sleep to start to cancel the inflight request.
-        self.collect_console(
-            timeout_secs=self.DEFAULT_TIMEOUT,
-            pattern="starting sleep",
-        )
+        self.collect_console(pattern="starting sleep")
         cancel_seq = self.async_cancel(requestId=blocking_seq)
 
-        blocking_resp = self.dap_server.recv_packet(filter_type=["response"])
+        blocking_resp = self.dap_server.receive_response(blocking_seq)
         self.assertEqual(blocking_resp["request_seq"], blocking_seq)
         self.assertEqual(blocking_resp["command"], "evaluate")
         self.assertEqual(blocking_resp["success"], False)
         self.assertEqual(blocking_resp["message"], "cancelled")
 
-        cancel_resp = self.dap_server.recv_packet(filter_type=["response"])
+        cancel_resp = self.dap_server.receive_response(cancel_seq)
         self.assertEqual(cancel_resp["request_seq"], cancel_seq)
         self.assertEqual(cancel_resp["command"], "cancel")
         self.assertEqual(cancel_resp["success"], True)

lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py

@@ -1,8 +1,8 @@
-import os
+"""
+Test lldb-dap command hooks
+"""
 
-import dap_server
 import lldbdap_testcase
-from lldbsuite.test import lldbtest, lldbutil
 from lldbsuite.test.decorators import *
 
 
@@ -23,7 +23,7 @@ def test_command_directive_quiet_on_success(self):
             exitCommands=["?" + command_quiet, command_not_quiet],
         )
         full_output = self.collect_console(
-            timeout_secs=1.0,
+            timeout=1.0,
             pattern=command_not_quiet,
         )
         self.assertNotIn(command_quiet, full_output)
@@ -51,7 +51,7 @@ def do_test_abort_on_error(
             expectFailure=True,
         )
         full_output = self.collect_console(
-            timeout_secs=1.0,
+            timeout=1.0,
             pattern=command_abort_on_error,
         )
         self.assertNotIn(command_quiet, full_output)
@@ -81,9 +81,6 @@ def test_command_directive_abort_on_error_attach_commands(self):
             expectFailure=True,
         )
         self.assertFalse(resp["success"], "expected 'attach' failure")
-        full_output = self.collect_console(
-            timeout_secs=1.0,
-            pattern=command_abort_on_error,
-        )
+        full_output = self.collect_console(pattern=command_abort_on_error)
         self.assertNotIn(command_quiet, full_output)
         self.assertIn(command_abort_on_error, full_output)

lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py

@@ -139,9 +139,7 @@ def test_exit_status_message_sigterm(self):
         process.wait()
 
         # Get the console output
-        console_output = self.collect_console(
-            timeout_secs=10.0, pattern="exited with status"
-        )
+        console_output = self.collect_console(pattern="exited with status")
 
         # Verify the exit status message is printed.
         self.assertRegex(
@@ -156,9 +154,7 @@ def test_exit_status_message_ok(self):
         self.continue_to_exit()
 
         # Get the console output
-        console_output = self.collect_console(
-            timeout_secs=10.0, pattern="exited with status"
-        )
+        console_output = self.collect_console(pattern="exited with status")
 
         # Verify the exit status message is printed.
         self.assertIn(
@@ -177,9 +173,7 @@ def test_diagnositcs(self):
             f"target create --core  {core}", context="repl"
         )
 
-        diagnostics = self.collect_important(
-            timeout_secs=self.DEFAULT_TIMEOUT, pattern="minidump file"
-        )
+        diagnostics = self.collect_important(pattern="minidump file")
 
         self.assertIn(
             "warning: unable to retrieve process ID from minidump file",