[lldb-dap] Test gardening, enabling tests and improving doc comments. #140777
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A bit of test gardenining. Enabling tests that were marked as skipped as part of llvm#137660. Fixed up some comments and doc comments and fixed some test helpers.
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesA bit of test gardenining. Enabling tests that were marked as skipped as part of #137660. Fixed up some comments and doc comments and fixed some test helpers. Patch is 38.97 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/140777.diff 36 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index 895f2a82547a9..5e14dfeda48a8 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -1,5 +1,6 @@
# System modules
from functools import wraps
+from typing import Optional
from packaging import version
import ctypes
import locale
@@ -1102,3 +1103,28 @@ def is_feature_enabled():
return "%s is not supported on this system." % feature
return skipTestIfFn(is_feature_enabled)
+
+
+def skipIfBinaryToLarge(path: Optional[str], maxSize: int):
+ """Skip the test if a binary is to large.
+
+ 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.
+ """
+
+ def check_binary_size():
+ if not path or not os.path.exists(path):
+ return "invalid path"
+
+ try:
+ size = os.path.getsize(path)
+ if size <= maxSize:
+ return None
+ return f"binary {path} (size = {size} is to larger than {maxSize}"
+ except:
+ return f"failed to read size of {path}"
+
+ return skipTestIfFn(check_binary_size)
diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py
index 7c2b540195d15..edb87a9314d78 100644
--- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py
+++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py
@@ -2,26 +2,16 @@
Test lldb-dap "port" configuration to "attach" request
"""
-import dap_server
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
from lldbsuite.test import lldbplatformutil
from lldbgdbserverutils import Pipe
import lldbdap_testcase
-import os
-import shutil
-import subprocess
-import tempfile
-import threading
-import sys
-import socket
+import lldb
-@skip("https://github.com/llvm/llvm-project/issues/138803")
+@skip(bugnumber="https://github.com/llvm/llvm-project/issues/138803")
class TestDAP_attachByPortNum(lldbdap_testcase.DAPTestCaseBase):
- default_timeout = 20
-
def set_and_hit_breakpoint(self, continueToExit=True):
self.dap_server.wait_for_stopped()
@@ -50,7 +40,7 @@ def get_debug_server_command_line_args(self):
def get_debug_server_pipe(self):
pipe = Pipe(self.getBuildDir())
self.addTearDownHook(lambda: pipe.close())
- pipe.finish_connection(self.default_timeout)
+ pipe.finish_connection(self.DEFAULT_TIMEOUT)
return pipe
@skipIfWindows
@@ -73,28 +63,33 @@ def test_by_port(self):
)
# Read the port number from the debug server pipe.
- port = pipe.read(10, self.default_timeout)
+ port = pipe.read(10, self.DEFAULT_TIMEOUT)
# Trim null byte, convert to int
port = int(port[:-1])
self.assertIsNotNone(
port, " Failed to read the port number from debug server pipe"
)
- self.attach(program=program, gdbRemotePort=port, sourceInitFile=True)
+ self.attach(
+ program=program,
+ gdbRemotePort=port,
+ sourceInitFile=True,
+ stopOnEntry=True,
+ )
self.set_and_hit_breakpoint(continueToExit=True)
- self.process.terminate()
@skipIfWindows
@skipIfNetBSD
- def test_by_port_and_pid(self):
+ def test_fails_if_both_port_and_pid_are_set(self):
"""
Tests attaching to a process by process ID and port number.
"""
program = self.build_and_create_debug_adapter_for_attach()
- # It is not necessary to launch "lldb-server" to obtain the actual port and pid for attaching.
- # However, when providing the port number and pid directly, "lldb-dap" throws an error message, which is expected.
- # So, used random pid and port numbers here.
+ # It is not necessary to launch "lldb-server" to obtain the actual port
+ # and pid for attaching. However, when providing the port number and pid
+ # directly, "lldb-dap" throws an error message, which is expected. So,
+ # used random pid and port numbers here.
pid = 1354
port = 1234
@@ -106,10 +101,9 @@ def test_by_port_and_pid(self):
sourceInitFile=True,
expectFailure=True,
)
- if not (response and response["success"]):
- self.assertFalse(
- response["success"], "The user can't specify both pid and port"
- )
+ self.assertFalse(
+ response["success"], "The user can't specify both pid and port"
+ )
@skipIfWindows
@skipIfNetBSD
@@ -123,11 +117,10 @@ def test_by_invalid_port(self):
response = self.attach(
program=program, gdbRemotePort=port, sourceInitFile=True, expectFailure=True
)
- if not (response and response["success"]):
- self.assertFalse(
- response["success"],
- "The user can't attach with invalid port (%s)" % port,
- )
+ self.assertFalse(
+ response["success"],
+ "The user can't attach with invalid port (%s)" % port,
+ )
@skipIfWindows
@skipIfNetBSD
@@ -147,9 +140,7 @@ def test_by_illegal_port(self):
response = self.attach(
program=program, gdbRemotePort=port, sourceInitFile=True, expectFailure=True
)
- if not (response and response["success"]):
- self.assertFalse(
- response["success"],
- "The user can't attach with illegal port (%s)" % port,
- )
- self.process.terminate()
+ self.assertFalse(
+ response["success"],
+ "The user can't attach with illegal port (%s)" % port,
+ )
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
index aae1251b17c93..26df2573555df 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setBreakpoints.py
@@ -12,7 +12,6 @@
import os
-@skip("Temporarily disable the breakpoint tests")
class TestDAP_setBreakpoints(lldbdap_testcase.DAPTestCaseBase):
def setUp(self):
lldbdap_testcase.DAPTestCaseBase.setUp(self)
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py
index 4dc8c5b3c7ded..92ac66cd44c5d 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setExceptionBreakpoints.py
@@ -10,7 +10,6 @@
import lldbdap_testcase
-@skip("Temporarily disable the breakpoint tests")
class TestDAP_setExceptionBreakpoints(lldbdap_testcase.DAPTestCaseBase):
@skipIfWindows
def test_functionality(self):
diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py
index baaca4d974d5d..946595f639edc 100644
--- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_setFunctionBreakpoints.py
@@ -10,7 +10,6 @@
import lldbdap_testcase
-@skip("Temporarily disable the breakpoint tests")
class TestDAP_setFunctionBreakpoints(lldbdap_testcase.DAPTestCaseBase):
@skipIfWindows
def test_set_and_clear(self):
diff --git a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
index 824ed8fe3bb97..a239664f9eef7 100644
--- a/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
+++ b/lldb/test/API/tools/lldb-dap/cancel/TestDAP_cancel.py
@@ -2,8 +2,6 @@
Test lldb-dap cancel request
"""
-import time
-
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
import lldbdap_testcase
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 7b4d1adbb2071..9f02d2f951d27 100644
--- a/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
+++ b/lldb/test/API/tools/lldb-dap/console/TestDAP_console.py
@@ -1,26 +1,23 @@
"""
-Test lldb-dap setBreakpoints request
+Test lldb-dap console output
"""
-import dap_server
import lldbdap_testcase
-from lldbsuite.test import lldbutil
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-def get_subprocess(root_process, process_name):
- queue = [root_process]
- while queue:
- process = queue.pop()
- if process.name() == process_name:
- return process
- queue.extend(process.children())
-
- self.assertTrue(False, "No subprocess with name %s found" % process_name)
+class TestDAP_console(lldbdap_testcase.DAPTestCaseBase):
+ def get_subprocess(self, root_process, process_name):
+ queue = [root_process]
+ while queue:
+ process = queue.pop()
+ if process.name() == process_name:
+ return process
+ queue.extend(process.children())
+ self.assertTrue(False, "No subprocess with name %s found" % process_name)
-class TestDAP_console(lldbdap_testcase.DAPTestCaseBase):
def check_lldb_command(
self, lldb_command, contains_string, assert_msg, command_escape_prefix="`"
):
@@ -134,7 +131,7 @@ def test_exit_status_message_sigterm(self):
file=sys.stderr,
)
return
- process = get_subprocess(psutil.Process(os.getpid()), process_name)
+ process = self.get_subprocess(psutil.Process(os.getpid()), process_name)
process.terminate()
process.wait()
diff --git a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py
index db43dbaf515cf..72d7171b7c5fc 100644
--- a/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py
+++ b/lldb/test/API/tools/lldb-dap/coreFile/TestDAP_coreFile.py
@@ -2,10 +2,8 @@
Test lldb-dap coreFile attaching
"""
-import dap_server
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
import lldbdap_testcase
import os
diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
index a542a318050dd..e3ead1095cba0 100644
--- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
+++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py
@@ -9,7 +9,7 @@
class TestDAP_setDataBreakpoints(lldbdap_testcase.DAPTestCaseBase):
def setUp(self):
- lldbdap_testcase.DAPTestCaseBase.setUp(self)
+ super().setUp()
self.accessTypes = ["read", "write", "readWrite"]
@skipIfWindows
diff --git a/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py b/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
index 9e8ef5b289f2e..a4759084fdb9d 100644
--- a/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
+++ b/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
@@ -3,10 +3,8 @@
"""
-import dap_server
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
import lldbdap_testcase
import os
diff --git a/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py b/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py
index 09e3f62f0eead..3c93ec193716f 100644
--- a/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py
+++ b/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py
@@ -3,7 +3,6 @@
"""
-import dap_server
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
from lldbsuite.test import lldbutil
diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
index 2166e88151986..923f8f511f1c5 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
+++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
@@ -3,10 +3,7 @@
"""
import re
-
import lldbdap_testcase
-import dap_server
-from lldbsuite.test import lldbutil
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
diff --git a/lldb/test/API/tools/lldb-dap/instruction-breakpoint/TestDAP_instruction_breakpoint.py b/lldb/test/API/tools/lldb-dap/instruction-breakpoint/TestDAP_instruction_breakpoint.py
index 53b7df9e54af2..3793b1e136ac6 100644
--- a/lldb/test/API/tools/lldb-dap/instruction-breakpoint/TestDAP_instruction_breakpoint.py
+++ b/lldb/test/API/tools/lldb-dap/instruction-breakpoint/TestDAP_instruction_breakpoint.py
@@ -1,32 +1,25 @@
-import dap_server
-import shutil
+"""
+Test lldb-dap setInstructionBreakpoints request
+"""
+
+import os
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
import lldbdap_testcase
-import os
-import lldb
class TestDAP_InstructionBreakpointTestCase(lldbdap_testcase.DAPTestCaseBase):
NO_DEBUG_INFO_TESTCASE = True
def setUp(self):
- lldbdap_testcase.DAPTestCaseBase.setUp(self)
+ super().setUp()
self.main_basename = "main-copy.cpp"
self.main_path = os.path.realpath(self.getBuildArtifact(self.main_basename))
@skipIfWindows
def test_instruction_breakpoint(self):
- self.build()
- self.instruction_breakpoint_test()
-
- def instruction_breakpoint_test(self):
"""Sample test to ensure SBFrame::Disassemble produces SOME output"""
- # Create a target by the debugger.
- target = self.createTestTarget()
-
main_line = line_number("main.cpp", "breakpoint 1")
program = self.getBuildArtifact("a.out")
@@ -49,15 +42,15 @@ def instruction_breakpoint_test(self):
)
other_breakpoint_id = breakpoint["id"]
- # Continue and then verifiy the breakpoint
+ # Continue and then verify the breakpoint
self.dap_server.request_continue()
self.verify_breakpoint_hit([other_breakpoint_id])
# now we check the stack trace making sure that we got mapped source paths
frames = self.dap_server.request_stackTrace()["body"]["stackFrames"]
- intstructionPointerReference = []
- setIntstructionBreakpoints = []
- intstructionPointerReference.append(frames[0]["instructionPointerReference"])
+ instructionPointerReference = []
+ setInstructionBreakpoints = []
+ instructionPointerReference.append(frames[0]["instructionPointerReference"])
self.assertEqual(
frames[0]["source"]["name"], self.main_basename, "incorrect source name"
)
@@ -69,21 +62,21 @@ def instruction_breakpoint_test(self):
instruction = self.disassemble(frameIndex=0)
self.assertEqual(
instruction["address"],
- intstructionPointerReference[0],
- "current breakpoint reference is not in the disaasembly view",
+ instructionPointerReference[0],
+ "current breakpoint reference is not in the disassembly view",
)
# Get next instruction address to set instruction breakpoint
disassembled_instruction_list = self.dap_server.disassembled_instructions
instruction_addr_list = list(disassembled_instruction_list.keys())
- index = instruction_addr_list.index(intstructionPointerReference[0])
+ index = instruction_addr_list.index(instructionPointerReference[0])
if len(instruction_addr_list) >= (index + 1):
next_inst_addr = instruction_addr_list[index + 1]
if len(next_inst_addr) > 2:
- setIntstructionBreakpoints.append(next_inst_addr)
+ setInstructionBreakpoints.append(next_inst_addr)
instruction_breakpoint_response = (
self.dap_server.request_setInstructionBreakpoints(
- setIntstructionBreakpoints
+ setInstructionBreakpoints
)
)
inst_breakpoints = instruction_breakpoint_response["body"][
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index 8805ce50e6a21..eb3caa9d85583 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -1,15 +1,12 @@
"""
-Test lldb-dap setBreakpoints request
+Test lldb-dap launch request
"""
-import dap_server
+import os
+import re
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
import lldbdap_testcase
-import time
-import os
-import re
# Many tests are skipped on Windows because get_stdout() returns None there.
# Despite the test program printing correctly. See
@@ -339,7 +336,7 @@ def test_commands(self):
launch.
"initCommands" are a list of LLDB commands that get executed
- before the targt is created.
+ before the target is created.
"preRunCommands" are a list of LLDB commands that get executed
after the target has been created and before the launch.
"stopCommands" are a list of LLDB commands that get executed each
@@ -425,7 +422,7 @@ def test_extra_launch_commands(self):
first_line = line_number(source, "// breakpoint 1")
second_line = line_number(source, "// breakpoint 2")
# Set target binary and 2 breakpoints
- # then we can varify the "launchCommands" get run
+ # then we can verify the "launchCommands" get run
# also we can verify that "stopCommands" get run as the
# breakpoints get hit
launchCommands = [
@@ -508,12 +505,12 @@ def test_failing_launch_commands(self):
# Get output from the console. This should contain both the
# "initCommands" and the "preRunCommands".
output = self.get_console()
- # Verify all "initCommands" were found in console output
+ # Verify all "initCommands" are found in console output
self.verify_commands("initCommands", output, initCommands)
- # Verify all "preRunCommands" were found in console output
+ # Verify all "preRunCommands" are found in console output
self.verify_commands("preRunCommands", output, preRunCommands)
- # Verify all "launchCommands" were founc in console output
+ # Verify all "launchCommands" are found in console output
# The launch should fail due to the invalid command.
self.verify_commands("launchCommands", output, launchCommands)
self.assertRegex(output, re.escape(bad_path) + r".*does not exist")
diff --git a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
index 45f836a2fa3c3..6b348866e44fb 100644
--- a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
+++ b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
@@ -1,12 +1,9 @@
"""
-Test lldb-dap locations request
+Test lldb-dap locations request.
"""
-
-import dap_server
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
import lldbdap_testcase
import os
diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
index 74062f3ab2164..159dd3bc1fa3a 100644
--- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
+++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py
@@ -3,10 +3,8 @@
"""
from base64 import b64decode
-import dap_server
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
import lldbdap_testcase
import os
diff --git a...
[truncated]
|
JDevlieghere
approved these changes
May 20, 2025
da-viper
reviewed
May 20, 2025
da-viper
approved these changes
May 20, 2025
…nstable in certain build configuraitons. This revealed a number of tests that were unstable, so I also included fixes for those tests as well.
|
This patch has grown a bit due to discovering a number of issues in some of the tests. I think I'll try splitting it up into smaller patches instead. |
ashgti
added a commit
that referenced
this pull request
May 23, 2025
In lldb-dap, we have existing tests that are known to be unstable when lldb and lldb-dap are built in the Debug configuration. This decorator lets us skip those tests in CI jobs that are to slow with those configurations. This was split out from #140777 to make the patches smaller.
sivan-shani
pushed a commit
to sivan-shani/llvm-project
that referenced
this pull request
Jun 3, 2025
In lldb-dap, we have existing tests that are known to be unstable when lldb and lldb-dap are built in the Debug configuration. This decorator lets us skip those tests in CI jobs that are to slow with those configurations. This was split out from llvm#140777 to make the patches smaller.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A bit of test gardenining. Enabling tests that were marked as skipped as part of #137660. Fixed up some comments and doc comments and fixed some test helpers.