Skip to content

Conversation

@LifengWang
Copy link
Collaborator

No description provided.

@LifengWang LifengWang requested a review from Copilot October 22, 2025 03:18
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds infrastructure to enable and run PyTorch Inductor CPU unit tests on Windows, including automated reporting of test results.

  • Adds a Python script to parse test logs and generate HTML failure reports
  • Provides PowerShell scripts for installing nightly PyTorch builds and running CPU unit tests
  • Implements a Jenkins/Groovy pipeline to orchestrate the testing workflow and email results

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
scripts/windows_inductor/report_ut_win.py Parses test log files for failures and generates HTML reports with PyTorch nightly version tracking
scripts/windows_inductor/install_nightly_pytorch.ps1 Manages conda environment creation and installs nightly PyTorch CPU builds with test dependencies
scripts/windows_inductor/cpu_ut.ps1 Executes Inductor CPU unit tests with and without C++ wrapper, logging results to versioned directories
groovy/inductor_ut_windows.groovy Defines Jenkins pipeline stages for environment setup, test execution, report generation, and email notifications

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -0,0 +1,192 @@
# parse_test_logs.py
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

Filename comment 'parse_test_logs.py' does not match actual filename 'report_ut_win.py'.

Suggested change
# parse_test_logs.py
# report_ut_win.py

Copilot uses AI. Check for mistakes.

if __name__ == "__main__":
if len(sys.argv) != 2:
print("Usage: python parse_test_logs.py <log_dir>")
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

Usage message references incorrect filename 'parse_test_logs.py' instead of 'report_ut_win.py'.

Copilot uses AI. Check for mistakes.
@LifengWang LifengWang requested a review from Copilot October 22, 2025 03:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

# Regular expression to match FAILED lines
# Pattern: FAILED [time] file_path::class_name::test_name
# Uses non-greedy matching (.*?) to handle special characters in paths, class names, and test names
pattern = r"FAILED \[([0-9.]+)s\] (.*?\.py)::([A-Za-z_][A-Za-z0-9_]*)::([A-Za-z_][A-Za-z0-9_]*)"
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

The regex pattern does not allow test methods or class names to start with digits (e.g., test_2d_conv or Test123). The pattern [A-Za-z_][A-Za-z0-9_]* requires the first character to be a letter or underscore. Consider using [A-Za-z0-9_]+ to allow digits anywhere in the name, or verify that the naming convention strictly prohibits leading digits.

Suggested change
pattern = r"FAILED \[([0-9.]+)s\] (.*?\.py)::([A-Za-z_][A-Za-z0-9_]*)::([A-Za-z_][A-Za-z0-9_]*)"
pattern = r"FAILED \[([0-9.]+)s\] (.*?\.py)::([A-Za-z0-9_]+)::([A-Za-z0-9_]+)"

Copilot uses AI. Check for mistakes.
Generates an HTML report based on the list of failed tests.

Args:
all_failed_tests (list): The list of all failed tests extracted from log files.
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

The docstring is missing the nightly_version parameter which is required by the function signature.

Suggested change
all_failed_tests (list): The list of all failed tests extracted from log files.
all_failed_tests (list): The list of all failed tests extracted from log files.
nightly_version (str): The version string for the nightly build.

Copilot uses AI. Check for mistakes.
Comment on lines 13 to 14
$line = (pip list | Select-String "torch ").Line
$pt_nightly = [regex]::Match($line, "dev(\d+)").Groups[1].Value
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

If pip list doesn't find 'torch' or the version format doesn't match the regex pattern, $pt_nightly will be empty, causing issues when creating the log directory path. Add error handling to verify the torch package exists and the version was successfully extracted before proceeding.

Suggested change
$line = (pip list | Select-String "torch ").Line
$pt_nightly = [regex]::Match($line, "dev(\d+)").Groups[1].Value
$line = (pip list | Select-String "torch ").Line
if (-not $line) {
Write-Error "Could not find 'torch' in pip list. Please ensure PyTorch is installed in the environment."
exit 1
}
$pt_nightly = [regex]::Match($line, "dev(\d+)").Groups[1].Value
if (-not $pt_nightly) {
Write-Error "Could not extract nightly build number from torch version. Version string: $line"
exit 1
}

Copilot uses AI. Check for mistakes.
git pull
Copy-Item -Path "${workspaceDir}/scripts/windows_inductor/cpu_ut.ps1" -Destination "."

cmd.exe "/K" (
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

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

Using /K keeps the command prompt window open after execution, which may cause the pipeline to hang waiting for manual closure. Use /C instead to execute the command and close automatically: cmd.exe \"/C\".

Suggested change
cmd.exe "/K" (
cmd.exe "/C" (

Copilot uses AI. Check for mistakes.
@LifengWang LifengWang requested a review from chuanqi129 October 23, 2025 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants