Skip to content

Fix a couple of tests that were incorrectly using configuration.dwarf_version #114161

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

Merged
merged 2 commits into from
Oct 30, 2024

Conversation

jimingham
Copy link
Collaborator

The tests were using the variable directly to get the dwarf version used for the test. That's only the overridden value, and won't be set if we're using the compiler default. I also put a comment by the variable to make sure people don't make the same mistake in the future.

configuration.dwarf_version

directly to get the dwarf version used for the test.  That's
only the overridden value, and won't be set if we're using the
compiler default.  I also put a comment by the variable to make
sure people don't make the same mistake in the future.
@llvmbot
Copy link
Member

llvmbot commented Oct 30, 2024

@llvm/pr-subscribers-lldb

Author: None (jimingham)

Changes

The tests were using the variable directly to get the dwarf version used for the test. That's only the overridden value, and won't be set if we're using the compiler default. I also put a comment by the variable to make sure people don't make the same mistake in the future.


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

3 Files Affected:

  • (modified) lldb/packages/Python/lldbsuite/test/configuration.py (+4)
  • (modified) lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py (+2-2)
  • (modified) lldb/test/API/python_api/type/TestTypeList.py (+2-2)
diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py b/lldb/packages/Python/lldbsuite/test/configuration.py
index 1bacd74a968c31..bcc179346836d1 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -46,6 +46,10 @@
 make_path = None
 
 # The overriden dwarf verison.
+# Don't use this to test the current compiler's
+# DWARF version, as this won't be set if the
+# version isn't overridden.
+# Use lldbplatformutils.getDwarfVersion() instead.
 dwarf_version = 0
 
 # Any overridden settings.
diff --git a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
index b5e8115160d209..fbc8884a927fee 100644
--- a/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ b/lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -8,7 +8,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
-
+from lldbsuite.test import lldbplatformutil
 
 class NamespaceLookupTestCase(TestBase):
     def setUp(self):
@@ -167,7 +167,7 @@ def test_scope_lookup_with_run_command(self):
         self.runToBkpt("continue")
         # FIXME: In DWARF 5 with dsyms, the ordering of functions is slightly
         # different, which also hits the same issues mentioned previously.
-        if configuration.dwarf_version <= 4 or self.getDebugInfo() == "dwarf":
+        if int(lldbplatformutil.getDwarfVersion()) <= 4 or self.getDebugInfo() == "dwarf":
             self.expect_expr("func()", result_type="int", result_value="2")
 
         # Continue to BP_ns_scope at ns scope
diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py
index bc4d00c17c5551..09879276b44aa3 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -6,7 +6,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
-
+from lldbsuite.test import lldbplatformutil
 
 class TypeAndTypeListTestCase(TestBase):
     def setUp(self):
@@ -248,7 +248,7 @@ def test(self):
         self.assertEqual(myint_arr_element_type, myint_type)
 
         # Test enum methods. Requires DW_AT_enum_class which was added in Dwarf 4.
-        if configuration.dwarf_version >= 4:
+        if int(lldbplatformutil.getDwarfVersion()) >= 4:
             enum_type = target.FindFirstType("EnumType")
             self.assertTrue(enum_type)
             self.DebugSBType(enum_type)

Copy link

github-actions bot commented Oct 30, 2024

✅ With the latest revision this PR passed the Python code formatter.

@jimingham jimingham merged commit a575e6e into llvm:main Oct 30, 2024
7 checks passed
@jimingham jimingham deleted the dwarf_version branch October 30, 2024 16:25
Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
…_version (llvm#114161)

The tests were using the variable directly to get the dwarf version used
for the test. That's only the overridden value, and won't be set if
we're using the compiler default. I also put a comment by the variable
to make sure people don't make the same mistake in the future.
felipepiovezan pushed a commit to felipepiovezan/llvm-project that referenced this pull request Nov 4, 2024
…_version (llvm#114161)

The tests were using the variable directly to get the dwarf version used
for the test. That's only the overridden value, and won't be set if
we're using the compiler default. I also put a comment by the variable
to make sure people don't make the same mistake in the future.

(cherry picked from commit a575e6e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants