[Lldb-commits] [lldb] [lldb-dap] Improving tests logging to understand CI failures. (PR #139311)

[John Harrison via lldb-commits]

https://github.com/ashgti created https://github.com/llvm/llvm-project/pull/139311

To improve logging this adjusts two properties of the existing tests:

* Forwards stderr from lldb-dap to the process in case errors are reported to stderr.
* Adjusts `DebugAdapterServer.terminate` to close stdin and wait for the process to exit instead of sending SIGTERM. Additionally, if we end up with a non-zero exit status we now raise an error to note the unexpected exit status.

With these changes, I did find one test case in `TestDAP_console.test_diagnositcs` that was not waiting to ensure the expected event had arrived by the time it performed an assert.

>From abb7aad60e314fadb235e370f431112fa078e023 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Fri, 9 May 2025 11:58:35 -0700
Subject: [PATCH] [lldb-dap] Improving tests logging to understand CI failures.

To improve logging this adjusts two properties of the existing tests:

* Forwards stderr from lldb-dap to the process in case errors are reported to stderr.
* Adjusts `DebugAdapterServer.terminate` to close stdin and wait for the process to exit instead of sending SIGTERM. Additionally, if we end up with a non-zero exit status we now raise an error to note the unexpected exit status.

With these changes, I did find one test case in `TestDAP_console.test_diagnositcs` that was not waiting to ensure the expected event had arrived by the time it performed an assert.
---
 .../test/tools/lldb-dap/dap_server.py         | 36 +++++++++++++++----
 .../tools/lldb-dap/console/TestDAP_console.py |  5 +--
 lldb/test/API/tools/lldb-dap/io/TestDAP_io.py |  4 ---
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index e10342b72f4f0..292209f8ab042 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -8,6 +8,7 @@
 import socket
 import string
 import subprocess
+import signal
 import sys
 import threading
 import time
@@ -1269,7 +1270,7 @@ def launch(cls, /, executable, env=None, log_file=None, connection=None):
             args,
             stdin=subprocess.PIPE,
             stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
+            stderr=sys.stderr,
             env=adapter_env,
         )
 
@@ -1302,14 +1303,37 @@ def get_pid(self):
     def terminate(self):
         super(DebugAdapterServer, self).terminate()
         if self.process is not None:
-            self.process.terminate()
+            process = self.process
+            self.process = None
             try:
-                self.process.wait(timeout=20)
+                # When we close stdin it should signal the lldb-dap that no 
+                # new messages will arrive and it should shutdown on its own.
+                process.stdin.close()
+                process.wait(timeout=20)
             except subprocess.TimeoutExpired:
-                self.process.kill()
-                self.process.wait()
-            self.process = None
+                process.kill()
+                process.wait()
+            if process.returncode != 0:
+                raise DebugAdapterProcessError(process.returncode)
+
 
+class DebugAdapterError(Exception): pass
+
+class DebugAdapterProcessError(DebugAdapterError):
+    """Raised when the lldb-dap process exits with a non-zero exit status.
+    """
+
+    def __init__(self, returncode):
+        self.returncode = returncode
+
+    def __str__(self):
+        if self.returncode and self.returncode < 0:
+            try:
+                return f"lldb-dap died with {signal.Signals(-self.returncode).name}."
+            except ValueError:
+                return f"lldb-dap died with unknown signal {-self.returncode}."
+        else:
+            return f"lldb-dap returned non-zero exit status {self.returncode}."
 
 def attach_options_specified(options):
     if options.pid is not None:
diff --git a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
index 8642e317f9b3a..f6809c0cdcb60 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -176,9 +176,10 @@ def test_diagnositcs(self):
             f"target create --core  {core}", context="repl"
         )
 
-        output = self.get_important()
+        diag_message = self.collect_important(timeout_secs=self.timeoutval, pattern="minidump file")
+
         self.assertIn(
             "warning: unable to retrieve process ID from minidump file",
-            output,
+            diag_message,
             "diagnostic found in important output",
         )
diff --git a/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py b/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
index f05f876e57b49..b72b98de412b4 100644
--- a/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
+++ b/lldb/test/API/tools/lldb-dap/io/TestDAP_io.py
@@ -22,13 +22,9 @@ def cleanup():
                 process.terminate()
                 process.wait()
             stdout_data = process.stdout.read().decode()
-            stderr_data = process.stderr.read().decode()
             print("========= STDOUT =========", file=sys.stderr)
             print(stdout_data, file=sys.stderr)
             print("========= END =========", file=sys.stderr)
-            print("========= STDERR =========", file=sys.stderr)
-            print(stderr_data, file=sys.stderr)
-            print("========= END =========", file=sys.stderr)
             print("========= DEBUG ADAPTER PROTOCOL LOGS =========", file=sys.stderr)
             with open(log_file_path, "r") as file:
                 print(file.read(), file=sys.stderr)