[lldb-dap][test] Refactor runInTerminal Tests.

[da-viper]

Replace isTestSupported function with skipIfBuildType annotation.

Test that uses the IsTestSupported function are no longer run, as the size of lldb-dap binary is now more than 1mb.

Update the broken test.

Fixes #108621

We could probably check if the test now passes on linux arm since it was disabled because it timed out. I experienced the timeout after replacing the IsTestSupported with skipIfBuildType.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Ebuka Ezike (da-viper)

Changes

Replace isTestSupported function with skipIfBuildType annotation.

Test that uses the IsTestSupported function are no longer run, as the size of lldb-dap binary is now more than 1mb.

Update the broken test.

Fixes #108621

We could probably check if the test now passes on linux arm since it was disabled because it timed out. I experienced the timeout after replacing the IsTestSupported with skipIfBuildType.

---

Full diff: https://github.com/llvm/llvm-project/pull/144954.diff

4 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+8-4)
• (modified) lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py (+46-41)
• (modified) lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py (+21-50)
• (modified) lldb/test/API/tools/lldb-dap/runInTerminal/main.c (+3-4)

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 6d32491eaa5e9..0fe36cd4bc71f 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
@@ -179,9 +179,13 @@ def encode_content(cls, s: str) -> bytes:
     @classmethod
     def validate_response(cls, command, response):
         if command["command"] != response["command"]:
-            raise ValueError("command mismatch in response")
+            raise ValueError(
+                f"command mismatch in response {command['command']} != {response['command']}"
+            )
         if command["seq"] != response["request_seq"]:
-            raise ValueError("seq mismatch in response")
+            raise ValueError(
+                f"seq mismatch in response {command['seq']} != {response['request_seq']}"
+            )
 
     def _read_packet_thread(self):
         done = False
@@ -404,8 +408,8 @@ def send_recv(self, command):
                 self.reverse_requests.append(response_or_request)
                 if response_or_request["command"] == "runInTerminal":
                     subprocess.Popen(
-                        response_or_request["arguments"]["args"],
-                        env=response_or_request["arguments"]["env"],
+                        response_or_request["arguments"].get("args"),
+                        env=response_or_request["arguments"].get("env", {}),
                     )
                     self.send_packet(
                         {
diff --git a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py
index e23d34bd99308..3ba7deb285de9 100644
--- a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py
+++ b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py
@@ -2,23 +2,35 @@
 Test lldb-dap RestartRequest.
 """
 
-import os
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import line_number
+from typing import Dict, Any, List
+
 import lldbdap_testcase
+from lldbsuite.test.decorators import skipIfWindows, skipIf, skipIfBuildType
+from lldbsuite.test.lldbtest import line_number
 
 
+@skipIfBuildType(["debug"])
 class TestDAP_restart_runInTerminal(lldbdap_testcase.DAPTestCaseBase):
-    def isTestSupported(self):
-        try:
-            # We skip this test for debug builds because it takes too long
-            # parsing lldb's own debug info. Release builds are fine.
-            # Checking the size of the lldb-dap binary seems to be a decent
-            # proxy for a quick detection. It should be far less than 1 MB in
-            # Release builds.
-            return os.path.getsize(os.environ["LLDBDAP_EXEC"]) < 1000000
-        except:
-            return False
+    def verify_stopped_on_entry(self, stopped_events: List[Dict[str, Any]]):
+        seen_stopped_event = 0
+        for stopped_event in stopped_events:
+            body = stopped_event.get("body")
+            if body is None:
+                continue
+
+            reason = body.get("reason")
+            if reason is None:
+                continue
+
+            self.assertNotEqual(
+                reason,
+                "breakpoint",
+                'verify stop after restart isn\'t "main" breakpoint',
+            )
+            if reason == "entry":
+                seen_stopped_event += 1
+
+        self.assertEqual(seen_stopped_event, 1, "expect only one stopped entry event.")
 
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=["arm$"])  # Always times out on buildbot
@@ -27,8 +39,6 @@ def test_basic_functionality(self):
         Test basic restarting functionality when the process is running in
         a terminal.
         """
-        if not self.isTestSupported():
-            return
         line_A = line_number("main.c", "// breakpoint A")
         line_B = line_number("main.c", "// breakpoint B")
 
@@ -60,33 +70,31 @@ def test_basic_functionality(self):
             "i != 0 after hitting breakpoint A on restart",
         )
 
+        # Check breakpoint B
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([bp_B])
+        self.assertEqual(
+            int(self.dap_server.get_local_variable_value("i")),
+            1234,
+            "i != 1234 after hitting breakpoint B",
+        )
+        self.continue_to_exit()
+
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=["arm$"])  # Always times out on buildbot
     def test_stopOnEntry(self):
         """
         Check that stopOnEntry works correctly when using runInTerminal.
         """
-        if not self.isTestSupported():
-            return
-        line_A = line_number("main.c", "// breakpoint A")
-        line_B = line_number("main.c", "// breakpoint B")
-
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program, runInTerminal=True, stopOnEntry=True)
         [bp_main] = self.set_function_breakpoints(["main"])
 
-        # When using stopOnEntry, configurationDone doesn't result in a running
-        # process, we should immediately get a stopped event instead.
+        self.dap_server.request_continue()  # sends configuration done
         stopped_events = self.dap_server.wait_for_stopped()
         # We should be stopped at the entry point.
-        for stopped_event in stopped_events:
-            if "body" in stopped_event:
-                body = stopped_event["body"]
-                if "reason" in body:
-                    reason = body["reason"]
-                    self.assertNotEqual(
-                        reason, "breakpoint", "verify stop isn't a breakpoint"
-                    )
+        self.assertGreaterEqual(len(stopped_events), 0, "expect stopped events")
+        self.verify_stopped_on_entry(stopped_events)
 
         # Then, if we continue, we should hit the breakpoint at main.
         self.dap_server.request_continue()
@@ -95,14 +103,11 @@ def test_stopOnEntry(self):
         # Restart and check that we still get a stopped event before reaching
         # main.
         self.dap_server.request_restart()
-        stopped_events = self.dap_server.wait_for_stopped()
-        for stopped_event in stopped_events:
-            if "body" in stopped_event:
-                body = stopped_event["body"]
-                if "reason" in body:
-                    reason = body["reason"]
-                    self.assertNotEqual(
-                        reason,
-                        "breakpoint",
-                        'verify stop after restart isn\'t "main" breakpoint',
-                    )
+        stopped_events = self.dap_server.wait_for_stopped(timeout=20)
+        self.verify_stopped_on_entry(stopped_events)
+
+        # continue to main
+        self.dap_server.request_continue()
+        self.verify_breakpoint_hit([bp_main])
+
+        self.continue_to_exit()
diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
index 65c931210d400..3d07cd8b20e28 100644
--- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
+++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
@@ -2,52 +2,33 @@
 Test lldb-dap runInTerminal reverse request
 """
 
-
-import dap_server
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import skipIfBuildType, skipIfWindows, skipIf, no_match
+from lldbsuite.test.lldbtest import line_number
 import lldbdap_testcase
-import time
 import os
 import subprocess
-import shutil
 import json
-from threading import Thread
 
 
+@skipIfBuildType(["debug"])
 class TestDAP_runInTerminal(lldbdap_testcase.DAPTestCaseBase):
-    def readPidMessage(self, fifo_file):
+    def read_pid_message(self, fifo_file):
         with open(fifo_file, "r") as file:
             self.assertIn("pid", file.readline())
 
-    def sendDidAttachMessage(self, fifo_file):
+    @staticmethod
+    def send_did_attach_message(fifo_file):
         with open(fifo_file, "w") as file:
             file.write(json.dumps({"kind": "didAttach"}) + "\n")
 
-    def readErrorMessage(self, fifo_file):
+    @staticmethod
+    def read_error_message(fifo_file):
         with open(fifo_file, "r") as file:
             return file.readline()
 
-    def isTestSupported(self):
-        # For some strange reason, this test fails on python3.6
-        if not (sys.version_info.major == 3 and sys.version_info.minor >= 7):
-            return False
-        try:
-            # We skip this test for debug builds because it takes too long parsing lldb's own
-            # debug info. Release builds are fine.
-            # Checking the size of the lldb-dap binary seems to be a decent proxy for a quick
-            # detection. It should be far less than 1 MB in Release builds.
-            if os.path.getsize(os.environ["LLDBDAP_EXEC"]) < 1000000:
-                return True
-        except:
-            return False
-
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_runInTerminal(self):
-        if not self.isTestSupported():
-            return
         """
             Tests the "runInTerminal" reverse request. It makes sure that the IDE can
             launch the inferior with the correct environment variables and arguments.
@@ -77,7 +58,7 @@ def test_runInTerminal(self):
 
         # We verify we actually stopped inside the loop
         counter = int(self.dap_server.get_local_variable_value("counter"))
-        self.assertGreater(counter, 0)
+        self.assertEqual(counter, 1)
 
         # We verify we were able to set the launch arguments
         argc = int(self.dap_server.get_local_variable_value("argc"))
@@ -90,10 +71,10 @@ def test_runInTerminal(self):
         env = self.dap_server.request_evaluate("foo")["body"]["result"]
         self.assertIn("bar", env)
 
+        self.continue_to_exit()
+
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_runInTerminalWithObjectEnv(self):
-        if not self.isTestSupported():
-            return
         """
             Tests the "runInTerminal" reverse request. It makes sure that the IDE can
             launch the inferior with the correct environment variables using an object.
@@ -113,11 +94,11 @@ def test_runInTerminalWithObjectEnv(self):
         self.assertIn("FOO", request_envs)
         self.assertEqual("BAR", request_envs["FOO"])
 
+        self.continue_to_exit()
+
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_runInTerminalInvalidTarget(self):
-        if not self.isTestSupported():
-            return
         self.build_and_create_debug_adapter()
         response = self.launch(
             "INVALIDPROGRAM",
@@ -135,8 +116,6 @@ def test_runInTerminalInvalidTarget(self):
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_missingArgInRunInTerminalLauncher(self):
-        if not self.isTestSupported():
-            return
         proc = subprocess.run(
             [self.lldbDAPExec, "--launch-target", "INVALIDPROGRAM"],
             capture_output=True,
@@ -150,8 +129,6 @@ def test_missingArgInRunInTerminalLauncher(self):
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self):
-        if not self.isTestSupported():
-            return
         comm_file = os.path.join(self.getBuildDir(), "comm-file")
         os.mkfifo(comm_file)
 
@@ -167,9 +144,9 @@ def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self):
             stderr=subprocess.PIPE,
         )
 
-        self.readPidMessage(comm_file)
-        self.sendDidAttachMessage(comm_file)
-        self.assertIn("No such file or directory", self.readErrorMessage(comm_file))
+        self.read_pid_message(comm_file)
+        self.send_did_attach_message(comm_file)
+        self.assertIn("No such file or directory", self.read_error_message(comm_file))
 
         _, stderr = proc.communicate()
         self.assertIn("No such file or directory", stderr)
@@ -177,8 +154,6 @@ def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self):
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self):
-        if not self.isTestSupported():
-            return
         comm_file = os.path.join(self.getBuildDir(), "comm-file")
         os.mkfifo(comm_file)
 
@@ -195,8 +170,8 @@ def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self):
             stdout=subprocess.PIPE,
         )
 
-        self.readPidMessage(comm_file)
-        self.sendDidAttachMessage(comm_file)
+        self.read_pid_message(comm_file)
+        self.send_did_attach_message(comm_file)
 
         stdout, _ = proc.communicate()
         self.assertIn("foo", stdout)
@@ -204,8 +179,6 @@ def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self):
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self):
-        if not self.isTestSupported():
-            return
         comm_file = os.path.join(self.getBuildDir(), "comm-file")
         os.mkfifo(comm_file)
 
@@ -216,8 +189,8 @@ def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self):
             env={**os.environ, "FOO": "BAR"},
         )
 
-        self.readPidMessage(comm_file)
-        self.sendDidAttachMessage(comm_file)
+        self.read_pid_message(comm_file)
+        self.send_did_attach_message(comm_file)
 
         stdout, _ = proc.communicate()
         self.assertIn("FOO=BAR", stdout)
@@ -225,8 +198,6 @@ def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self):
     @skipIfWindows
     @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
     def test_NonAttachedRunInTerminalLauncher(self):
-        if not self.isTestSupported():
-            return
         comm_file = os.path.join(self.getBuildDir(), "comm-file")
         os.mkfifo(comm_file)
 
@@ -244,7 +215,7 @@ def test_NonAttachedRunInTerminalLauncher(self):
             env={**os.environ, "LLDB_DAP_RIT_TIMEOUT_IN_MS": "1000"},
         )
 
-        self.readPidMessage(comm_file)
+        self.read_pid_message(comm_file)
 
         _, stderr = proc.communicate()
         self.assertIn("Timed out trying to get messages from the debug adapter", stderr)
diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/main.c b/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
index 676bd830e657b..0cc25d374d08a 100644
--- a/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
+++ b/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
@@ -4,8 +4,7 @@
 
 int main(int argc, char *argv[]) {
   const char *foo = getenv("FOO");
-  for (int counter = 1;; counter++) {
-    sleep(1); // breakpoint
-  }
-  return 0;
+  int counter = 1;
+
+  return 0; // breakpoint
 }

[DavidSpickett]

We could probably check if the test now passes on linux arm since it was disabled because it timed out. I experienced the timeout after replacing the IsTestSupported with skipIfBuildType.

I ran them on Arm and Arm64 and they are passing.

They could always time out again on the bot machine under heavy load, so enable them in a separate change so reverting it is easier.