Skip to content

[lldb][test] Fix running TestWithLimitDebugInfo.py on Windows #150579

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.

Sign up for GitHub

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 3 commits into from
Jul 30, 2025
+8 −9
Merged

[lldb][test] Fix running TestWithLimitDebugInfo.py on Windows #150579

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f687e98
[lldb][test] Fix running TestWithLimitDebugInfo.py on Windows
igorkudrin Jul 25, 2025
8739406
Merge branch 'main' into lldb-fix-TestWithLimitDebugInfo-on-Windows
igorkudrin Jul 29, 2025
85f8b0b
fixup! Use the set difference operator to calculate 'other_categories'
igorkudrin Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions lldb/packages/Python/lldbsuite/test/lldbtest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1778,16 +1778,20 @@ def no_reason(_):
attrvalue, "__no_debug_info_test__", False
):
# If any debug info categories were explicitly tagged, assume that list to be
# authoritative. If none were specified, try with all debug
# info formats.
all_dbginfo_categories = set(
# authoritative. If none were specified, try with all debug info formats.
test_method_categories = getattr(attrvalue, "categories", [])
dbginfo_categories = set(test_method_categories) & set(
Copy link
Collaborator

@labath labath Jul 28, 2025

Choose a reason for hiding this comment

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

How about this:

Suggested change
test_method_categories = getattr(attrvalue, "categories", [])
dbginfo_categories = set(test_method_categories) & set(
test_method_categories = set(getattr(attrvalue, "categories", []))
all_dbginfo_categories = set(

Copy link
Collaborator Author

@igorkudrin igorkudrin Jul 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

With this change, the property may have different types in different cases. Would this be OK? I mean, from the design perspective, because, of course, Python is lenient enough for this to work. And I suppose that you are also suggesting replacing other_categories + [cat] with other_categories | {cat}, right?

Copy link
Collaborator

@labath labath Jul 29, 2025

Choose a reason for hiding this comment

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

We should keep the type consistent, but you can do that by converting the result to a list with list(xxx) at an appropriate time. The thing that was bothering me was the combination of list- and set-based operations, which made it hard to follow.

Changing the property type to always be a set might also be good idea, but I'd do that separately.

Copy link
Collaborator Author

@igorkudrin igorkudrin Jul 29, 2025

Choose a reason for hiding this comment

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

Thanks, I've updated the patch accordingly.

I wouldn't expect any noticeable performance gain from changing the property type, so I'm not inclined to work on it.

test_categories.debug_info_categories.keys()
)
categories = (
set(getattr(attrvalue, "categories", [])) & all_dbginfo_categories
)
if not categories:
categories = [
if dbginfo_categories:
other_categories = [
category
for category in test_method_categories
if category not in dbginfo_categories
]
else:
other_categories = test_method_categories
dbginfo_categories = [
Copy link
Collaborator

@labath labath Jul 28, 2025

Choose a reason for hiding this comment

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

Suggested change
if dbginfo_categories:
other_categories = [
category
for category in test_method_categories
if category not in dbginfo_categories
]
else:
other_categories = test_method_categories
dbginfo_categories = [
dbginfo_categories = test_method_categories & all_dbginfo_categories
other_categories = test_method_categories.difference(all_dbginfo_categories)
if not dbginfo_categories:
dbginfo_categories = [

category
for category, can_replicate in test_categories.debug_info_categories.items()
if can_replicate
Expand All @@ -1799,16 +1803,16 @@ def no_reason(_):
skip_for_debug_info_cat_fn = getattr(
attrvalue, "__skip_for_debug_info_cat_fn__", no_reason
)
for cat in categories:
for cat in dbginfo_categories:

@decorators.add_test_categories([cat])
@wraps(attrvalue)
def test_method(self, attrvalue=attrvalue):
return attrvalue(self)

method_name = attrname + "_" + cat
test_method.__name__ = method_name
test_method.debug_info = cat
test_method.categories = other_categories + [cat]

xfail_reason = xfail_for_debug_info_cat_fn(cat)
if xfail_reason:
Expand Down
Loading