-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[lldb] Restrict TestVariableAnnotationsDisassembler.py to ELF x86_64 (skip on Windows/COFF) #156026
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[lldb] Restrict TestVariableAnnotationsDisassembler.py to ELF x86_64 (skip on Windows/COFF) #156026
Conversation
…(skip on Windows/COFF) The test assembles `d_original_example.s`, which contains ELF-specific directives (e.g. `.ident`, `.note.GNU-stack`, `.debug_*` sections). These fail on Windows/COFF even on x86_64. Gate the test to x86_64 on ELF platforms (Linux/FreeBSD/NetBSD).
|
@llvm/pr-subscribers-lldb Author: Abdullah Mohammad Amin (UltimateForce21) ChangesThe
These directives are not understood by COFF on Windows, so the test fails This patch adds decorators to gate the test,
Follow-up to #155942. Full diff: https://github.com/llvm/llvm-project/pull/156026.diff 1 Files Affected:
diff --git a/lldb/test/API/functionalities/disassembler-variables/TestVariableAnnotationsDisassembler.py b/lldb/test/API/functionalities/disassembler-variables/TestVariableAnnotationsDisassembler.py
index 4d99ea771605a..c3873f1e6e943 100644
--- a/lldb/test/API/functionalities/disassembler-variables/TestVariableAnnotationsDisassembler.py
+++ b/lldb/test/API/functionalities/disassembler-variables/TestVariableAnnotationsDisassembler.py
@@ -22,6 +22,8 @@ def _disassemble_verbose_symbol(self, symname):
self.runCmd(f"disassemble -n {symname} -v", check=True)
return self.res.GetOutput()
+ @skipIfWindows
+ @skipUnlessPlatform(["linux","freebsd","netbsd"])
@skipIf(archs=no_match(["x86_64"]))
def test_d_original_example_O1(self):
obj = self._build_obj("d_original_example.o")
@@ -34,6 +36,8 @@ def test_d_original_example_O1(self):
self.assertNotIn("<decoding error>", out)
@no_debug_info_test
+ @skipIfWindows
+ @skipUnlessPlatform(["linux","freebsd","netbsd"])
@skipIf(archs=no_match(["x86_64"]))
def test_regs_int_params(self):
obj = self._build_obj("regs_int_params.o")
@@ -49,6 +53,8 @@ def test_regs_int_params(self):
self.assertNotIn("<decoding error>", out)
@no_debug_info_test
+ @skipIfWindows
+ @skipUnlessPlatform(["linux","freebsd","netbsd"])
@skipIf(archs=no_match(["x86_64"]))
def test_regs_fp_params(self):
obj = self._build_obj("regs_fp_params.o")
@@ -64,6 +70,8 @@ def test_regs_fp_params(self):
self.assertNotIn("<decoding error>", out)
@no_debug_info_test
+ @skipIfWindows
+ @skipUnlessPlatform(["linux","freebsd","netbsd"])
@skipIf(archs=no_match(["x86_64"]))
def test_regs_mixed_params(self):
obj = self._build_obj("regs_mixed_params.o")
@@ -79,6 +87,8 @@ def test_regs_mixed_params(self):
self.assertNotIn("<decoding error>", out)
@no_debug_info_test
+ @skipIfWindows
+ @skipUnlessPlatform(["linux","freebsd","netbsd"])
@skipIf(archs=no_match(["x86_64"]))
def test_live_across_call(self):
obj = self._build_obj("live_across_call.o")
@@ -91,6 +101,8 @@ def test_live_across_call(self):
self.assertNotIn("<decoding error>", out)
@no_debug_info_test
+ @skipIfWindows
+ @skipUnlessPlatform(["linux","freebsd","netbsd"])
@skipIf(archs=no_match(["x86_64"]))
def test_loop_reg_rotate(self):
obj = self._build_obj("loop_reg_rotate.o")
@@ -105,6 +117,8 @@ def test_loop_reg_rotate(self):
self.assertNotIn("<decoding error>", out)
@no_debug_info_test
+ @skipIfWindows
+ @skipUnlessPlatform(["linux","freebsd","netbsd"])
@skipIf(archs=no_match(["x86_64"]))
def test_seed_reg_const_undef(self):
obj = self._build_obj("seed_reg_const_undef.o")
|
|
✅ With the latest revision this PR passed the Python code formatter. |
DavidSpickett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be able to apply the skip to the class, instead of repeating it for each function. Can you try that?
One test for example does this:
# Windows does not allow quotes in file names.
@skipIf(hostoslist=["windows"])
@skipIfRemote
class TestGdbRemoteLibrariesSvr4Support(gdbremote_testcase.GdbRemoteTestCaseBase):
FEATURE_NAME = "qXfer:libraries-svr4:read"
Certain decorators might not work in that position though, so give it a go.
lldb/test/API/functionalities/disassembler-variables/TestVariableAnnotationsDisassembler.py
Outdated
Show resolved
Hide resolved
|
Also add a comment to either the first skip, or the one skip (whichever you end up doing), saying that the test requires ELF directives. Saves someone a trip to git blame later. |
This change moves the platform skip logic to the test class instead of repeating it for each method.
DavidSpickett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
Thank you for the suggestions and I have done as you have advised. |
|
Edited the PR description slightly before merging, to reflect the single decorator. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/25623 Here is the relevant piece of the build log for the reference |
|
Btw, you have some leftover debug |
And this failure you can safely ignore :) |
Sounds good! |
The
TestVariableAnnotationsDisassembler.pytest assemblesd_original_example.s,which contains ELF-specific directives such as:
.ident.section ".note.GNU-stack", "", @progbits.section .debug_line, "", @progbitsThese directives are not understood by COFF on Windows, so the test fails
on the lldb-remote-linux-win builder even when running on x86_64.
This patch adds a decorator to gate the test,
@skipUnlessPlatform(["linux", "freebsd", "netbsd", "android"])—runs only on ELF platforms
Follow-up to #155942.