fix: fix test uploading to not overwrite itself

[nv-nmailhot]

Overview:

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• closes GitHub issue: #xxx

Summary by CodeRabbit

• Chores
  • Improved test result organization by generating uniquely named test artifacts per run, incorporating framework, test type, and platform information.
  • Enhanced test artifact aggregation workflow to discover and process multiple metadata files.

[coderabbitai]

Walkthrough

The pull request introduces uniquely named test result artifacts incorporating framework, test_type, and platform_arch into filenames. It updates the pytest action to generate and reference these uniquely named metadata and JUnit XML files, and modifies the metrics upload workflow to discover multiple metadata files using a wildcard pattern instead of a single file match.

Changes

Cohort / File(s)	Summary
Test Result Artifact Generation \.github/actions/pytest/action\.yml	Reworks test result handling to generate uniquely named metadata and JUnit XML files by incorporating framework, test_type, and platform_arch. Introduces METADATA_FILE and JUNIT_UNIQUE_NAME variables, renames JUnit XML to unique name, updates metadata content to reference the unique file, and adjusts artifact upload step. Adds verbose status logging for file operations.
Test Metadata Discovery \.github/workflows/upload_complete_workflow_metrics\.py	Updates test metadata file lookup pattern from single file match (test_metadata.json) to wildcard pattern (test_metadata_*.json) to enable discovery of multiple framework/test_type/arch-specific metadata files. Iterates over expanded set of metadata files to extract framework, test type, and step information.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰✨ From scattered records to neat little stacks,
We organize test files with unique-named tracks!
Framework and platform now woven into names,
Our metadata dances—no more lost test claims!
A wildcard wave gathers each variant with care,
Hop-skip to clarity in the testing air! 🌟

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description consists entirely of template placeholders with no actual content provided. All required sections—Overview, Details, and Where should the reviewer start—contain only HTML comments or are empty. The Related Issues section retains the placeholder text "closes GitHub issue: #xxx" without specifying an actual issue number. The description does not explain the problem being solved, the changes made, or guide reviewers on where to focus their review.	The author must provide meaningful content for each required section. The Overview should explain why test uploading was overwriting files, the Details should describe how unique file naming solves this problem, the reviewer guide should highlight the modified action.yml and workflow Python script, and the Related Issues section should reference the actual GitHub issue number instead of the placeholder "#xxx".

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "fix: fix test uploading to not overwrite itself" directly relates to the main objective of the changeset, which is to prevent test result files from overwriting each other by incorporating unique identifiers (framework, test_type, and platform_arch) into file names. The title is clear, concise, and specific enough that a teammate reviewing the history would understand the primary problem being addressed without additional context.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

.github/actions/pytest/action.yml (1)

87-104: Consider adding error handling for the file rename operation.

While the unique naming scheme correctly prevents overwrites, the mv command at line 93 lacks error checking. If the rename fails (e.g., due to filesystem issues), the metadata will reference a non-existent file, and subsequent processing could fail silently.

Consider adding error checking:

           JUNIT_UNIQUE_NAME="pytest_test_report_${{ inputs.framework }}_${{ inputs.test_type }}_${{ inputs.platform_arch }}.xml"
 
           # Rename XML file to unique name
-          mv "$JUNIT_FILE" "test-results/$JUNIT_UNIQUE_NAME"
+          if ! mv "$JUNIT_FILE" "test-results/$JUNIT_UNIQUE_NAME"; then
+            echo "❌ Failed to rename JUnit XML file"
+            exit 1
+          fi

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f98e91 and 461c178.

📒 Files selected for processing (2)

• .github/actions/pytest/action.yml (2 hunks)
• .github/workflows/upload_complete_workflow_metrics.py (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

• GitHub Check: trtllm (amd64)
• GitHub Check: trtllm (arm64)
• GitHub Check: sglang
• GitHub Check: operator (arm64)
• GitHub Check: operator (amd64)
• GitHub Check: vllm (amd64)
• GitHub Check: Build and Test - dynamo

🔇 Additional comments (2)

.github/workflows/upload_complete_workflow_metrics.py (1)

837-838: LGTM! Wildcard pattern correctly enables multi-artifact discovery.

The change from a single test_metadata.json to test_metadata_*.json properly supports the new unique naming scheme introduced in the pytest action. This allows the script to discover and process multiple framework/test_type/arch-specific metadata files without conflicts.

.github/actions/pytest/action.yml (1)

120-122: LGTM! Upload paths correctly reference the uniquely named artifacts.

The upload paths properly match the unique filenames created in the "Process Test Results" step, ensuring that each framework/test_type/arch combination uploads distinct artifacts without conflicts.

[grahamking]

Could you edit the title to say which test or tests or test suite, and remove the dup 'fix'?