Skip to content

issue/877 - return saved_file in TestManager.test method - #878

Merged
wooway777 merged 1 commit into
mainfrom
issue/877
Jan 5, 2026
Merged

issue/877 - return saved_file in TestManager.test method#878
wooway777 merged 1 commit into
mainfrom
issue/877

Conversation

@baominghelly

Copy link
Copy Markdown
Contributor

Description

return saved_file in TestManager.test method

Test evidence

test command

python test/infinicore/nn/rope.py --nvidia
python test/infinicore/tensor/narrow.py --nvidia
python test/infinicore/ops/add.py --nvidia --save
python test/infinicore/run.py --ops add acos acosh --nvidia --bench
python test/infinicore/run.py --ops add acos acosh --nvidia --bench --save
python test/infinicore/run.py --ops add acos acosh --nvidia --bench --save infinimetrics.json

test output

output.log

@baominghelly
baominghelly requested review from a team and wooway777January 4, 2026 08:12
@baominghellybaominghelly self-assigned this Jan 4, 2026
@baominghellybaominghelly linked an issue Jan 4, 2026 that may be closed by this pull request
@wooway777
wooway777 requested a review from CopilotJanuary 5, 2026 02:55

CopilotAI left a comment

Copy link
Copy Markdown

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 modifies the test framework to track and return the paths of saved report files throughout the execution chain. The main change is updating TestManager.test() to return a tuple of (success, saved_files) instead of just a boolean success value.

Key Changes

  • Updated TestManager.test() to collect and return saved report file paths alongside the success status
  • Modified the entire call chain to propagate saved file paths from JSON save operations up through the test execution stack
  • Added saved_file field to OperatorResult dataclass to store report paths

Reviewed changes

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

Show a summary per file
FileDescription
test/infinicore/run.pyUpdated callers to unpack tuple return from test_manager.test(); minor style improvements (quote consistency, whitespace)
test/infinicore/framework/utils/json_utils.pyModified save_json_report to return the saved file path or None on failure
test/infinicore/framework/test_manager.pyAdded saved_files list collection and modified return statement to return tuple (all_passed, saved_files)
test/infinicore/framework/runner.pyAdded saved_file instance variable and modified _save_report to return and store the saved path
test/infinicore/framework/results.pyAdded saved_file field to OperatorResult; updated TestSummary.save_report to return the saved path
test/infinicore/framework/executor.pyAdded assignment of runner.saved_file to result.saved_file after test execution

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

)
return all_passed

return all_passed, saved_files

CopilotAIJan 5, 2026

Copy link

Choose a reason for hiding this comment

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

The docstring for the test method should be updated to document the return value. Since the method now returns a tuple of (success: bool, saved_files: list), this should be documented in the docstring with a Returns section explaining both values.

Copilot uses AI. Check for mistakes.
test_summary.process_operator_result(result, test_results)

# Store saved report file if available
result.saved_file = runner.saved_file

CopilotAIJan 5, 2026

Copy link

Choose a reason for hiding this comment

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

The saved_file field is declared as type str with default value "", but runner.saved_file can be None (when save is not requested or when saving fails). Consider either changing the type annotation to Optional[str] or ensuring runner.saved_file is always a string (e.g., runner.saved_file or "").

Suggested change
result.saved_file=runner.saved_file
result.saved_file=runner.saved_fileor""

Copilot uses AI. Check for mistakes.
@wooway777
wooway777 merged commit 2d9d5c3 into mainJan 5, 2026
16 checks passed
@wooway777
wooway777 deleted the issue/877 branch January 5, 2026 05:36
Sign up for freeto 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.

[DEV] Return saved files in TestManger.test method

3 participants

@baominghelly@wooway777