Skip to content

[CI] 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 2 commits into
base: main
Choose a base branch
from
Open

[CI] Revise the Windows skip logic of UT #2383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f42fcc3
Revise windows skip logic
libohao1201 Nov 21, 2025
fe77177
Using windows skip dictionary when the platform is Windows.
libohao1201 Nov 24, 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
28 changes: 16 additions & 12 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 @@ -13,35 +13,35 @@
default="selected",
help="Test cases scope",
)
# Add skip-cases parameter to import window skip dictionary
parser.add_argument(
"--skip-cases",
action="store_true",
default=False,
help="Use window skip dictionary for test cases",
)
args = parser.parse_args()


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)
Copy link
Contributor

@daisyden daisyden Nov 27, 2025

Choose a reason for hiding this comment

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

This update will make all the test files return True, is it correct?



# Import window skip dictionary if skip-cases is True
if args.skip_cases:
platform = sys.platform
print(f"Running test on the platform: {platform}")
# Import window skip dictionary if Platform is Windows
if platform.startswith("win"):
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 +39 to +44
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 +104,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:
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": [
Copy link
Contributor

@daisyden daisyden Nov 27, 2025

Choose a reason for hiding this comment

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

to align with the naming style you could change test_decomp.py to test_decomp_xpu.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:
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:
exe_options = ' -k "' + exe_list[0]
for exe_case in exe_list[1:]:
exe_option = " or " + exe_case
Expand Down