Skip to content

Revise the Windows skip logic of UT #2383

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Revise the Windows skip logic of UT #2383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions test/xpu/run_test_with_skip.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,26 @@ def should_skip_entire_file(skip_list):
"""Check if the skip list contains any entire file skip pattern (*.py::)"""
if not skip_list:
return False
return any(item.endswith(".py::") for item in skip_list)
return any(item.endswith(".py") for item in skip_list)


# Import window skip dictionary if skip-cases is True
if args.skip_cases:
try:
# Import the window skip dictionary module
from window_skip_dict import skip_dict as window_skip_dict
from windows_skip_cases import skip_dict as window_skip_dict

# Merge the window skip dictionary with the default one using intelligent strategy
merged_skip_dict = {}

# First, copy all keys from default skip_dict
for key in skip_dict:
merged_skip_dict[key] = skip_dict[key].copy() if skip_dict[key] else []
if skip_dict[key] is None:
merged_skip_dict[key] = []
elif isinstance(skip_dict[key], tuple):
merged_skip_dict[key] = list(skip_dict[key])
else:
merged_skip_dict[key] = skip_dict[key].copy() if skip_dict[key] else []
Comment on lines +44 to +49
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

[nitpick] The nested if-elif-else logic can be simplified. When skip_dict[key] is None or falsy, all branches result in an empty list. Consider consolidating: merged_skip_dict[key] = list(skip_dict[key]) if isinstance(skip_dict[key], tuple) else (skip_dict[key].copy() if skip_dict[key] else [])

Suggested change
if skip_dict[key] is None:
merged_skip_dict[key] = []
elif isinstance(skip_dict[key], tuple):
merged_skip_dict[key] = list(skip_dict[key])
else:
merged_skip_dict[key] = skip_dict[key].copy() if skip_dict[key] else []
merged_skip_dict[key] = list(skip_dict[key]) if isinstance(skip_dict[key], tuple) else (skip_dict[key].copy() if skip_dict[key] else [])

Copilot uses AI. Check for mistakes.

# Then merge with window_skip_dict using intelligent strategy
for key in window_skip_dict:
Expand Down Expand Up @@ -104,6 +109,10 @@ def should_skip_entire_file(skip_list):
skip_list = None
# For "selected" case, use the skip_list as is

# If skip_list is empty, set it to None
if skip_list is not None and len(skip_list) == 0:
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

[nitpick] This can be simplified to if skip_list is not None and not skip_list: since empty lists are falsy in Python, making the length check unnecessary.

Suggested change
if skip_list is not None and len(skip_list) == 0:
if skip_list is not None and not skip_list:

Copilot uses AI. Check for mistakes.
skip_list = None

print(f"Running test case: {key}")
if skip_list:
print(f"Skip list: {skip_list}")
Expand Down
4 changes: 2 additions & 2 deletions test/xpu/windows_skip_cases.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

skip_dict = {
# Windows: Skip entire files using *.py:: pattern
"test_decomp": [
"test_decomp.py::", # Skip entire file on Windows
"test_decomp.py": [
"test_decomp.py", # Skip entire file on Windows
],
# Files where Windows only needs to skip specific tests (will merge with Linux defaults)
# "test_linalg": [
Expand Down
4 changes: 2 additions & 2 deletions test/xpu/xpu_test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ def copy_tests(

def launch_test(test_case, skip_list=None, exe_list=None):
os.environ["PYTORCH_TEST_WITH_SLOW"] = "1"
if skip_list is not None:
if skip_list and len(skip_list) > 0:
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

The check skip_list and len(skip_list) > 0 is redundant. If skip_list is truthy and not empty, if skip_list: is sufficient since empty lists are falsy in Python. The len(skip_list) > 0 check adds no additional value.

Copilot uses AI. Check for mistakes.
skip_options = ' -k "not ' + skip_list[0]
for skip_case in skip_list[1:]:
skip_option = " and not " + skip_case
Expand All @@ -1103,7 +1103,7 @@ def launch_test(test_case, skip_list=None, exe_list=None):
+ test_case
)
test_command += skip_options
elif exe_list is not None:
elif exe_list and len(exe_list) > 0:
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

The check exe_list and len(exe_list) > 0 is redundant. If exe_list is truthy and not empty, if exe_list: is sufficient since empty lists are falsy in Python. The len(exe_list) > 0 check adds no additional value.

Suggested change
elif exe_list and len(exe_list) > 0:
elif exe_list:

Copilot uses AI. Check for mistakes.
exe_options = ' -k "' + exe_list[0]
for exe_case in exe_list[1:]:
exe_option = " or " + exe_case
Expand Down