Skip to content

fix(profilers): fix rocprof-attach and add tests for rocpd and rocprofv3 executables - #9509

Merged
yhuiYH merged 3 commits into
mainfrom
users/yhui/add-new-tests
Nov 2, 2026
Merged

yhuiYH merged 3 commits into
mainfrom
users/yhui/add-new-tests

Conversation

@yhuiYH

@yhuiYH yhuiYH commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Add rocpd and rocprofiler-sdk executable tests

Adding tests identified rocprof-attach issue.
Fixes: #9545

Technical Details

When these trampolines were added for rocpd, the basic tests were not added for the executables. a8a4ba6

Now, we should add and run these basic --help console tests to ensure the binaries run correctly.

Immediately identified and fixes rocprof-attach problem:

Test Plan

This PR adds tests

Test Result

Passing Python tests

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot therock-pr-bot Bot added the Not ready to Review PR has unresolved policy failures — reviews blocked label Oct 27, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@yhuiYH
yhuiYH requested a balanced review from Copilot October 27, 2026 21:38
@therock-pr-bot therock-pr-bot Bot removed the Not ready to Review PR has unresolved policy failures — reviews blocked label Oct 27, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues were identified for the profiler smoke tests and rocprof-attach packaging changes.

No open issues 🎉

What changed in this PR

This PR adds profiler command smoke tests to the ROCm Python packages and fixes the missing attach executable reported in #9545 by exposing and packaging rocprof-attach.

Changes:

  • Add --help smoke tests for eight rocpd and rocprofiler-sdk commands.
  • Rename the attach console entry point and launcher to rocprof-attach.
  • Narrow packaging exclusions so rocprof-attach remains in the core wheel.
File Description
build_tools/​packaging/​python/​templates/​rocm/​src/​rocm_sdk/​tests/​core_test.py Add profiler command smoke tests.
build_tools/​packaging/​python/​templates/​rocm-sdk-core/​src/​rocm_sdk_core/​_cli.py Launch the correct rocprof-attach executable.
build_tools/​packaging/​python/​templates/​rocm-sdk-core/​setup.py Register the rocprof-attach console entry point.
build_tools/​build_python_packages.py Retain the attach executable while excluding compute and systems profiler binaries.

Review effort: Balanced

@yhuiYH yhuiYH changed the title chore(test): add profiler tests for rocpd and sdk executables fix(profilers): fix rocprof-attach and add tests for rocpd and sdk executables Oct 27, 2026
@yhuiYH yhuiYH changed the title fix(profilers): fix rocprof-attach and add tests for rocpd and sdk executables fix(profilers): fix rocprof-attach and add tests for rocpd and rocprofv3 executables Oct 27, 2026
@yhuiYH
yhuiYH marked this pull request as ready for review October 28, 2026 00:05
("rocminfo", [], "", True),
("rocm-smi", [], "Management", True),
("hipify-perl", ["--help"], "USAGE:", True),
("rocpd", ["--help"], "Aggregate", True),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you help me understand how this relates to #9535 "Fix ROCpd dependencies in Python wheels" ? The tool can run its --help but it has additional python requirements for full functionality?

Adding this test coverage is great, then I think we'll need to be careful about how to enable and test the rest if there are other python deps needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the tool can run it's --help, but some sub-modules need additional python packages to fully use some features. The tool will instruct the user to install it by showing the instructions:

ImportError: pandas module not found. Please install it using 'pip install pandas' to export to other formats

I believe the author of #9535 is trying to solve that problem, to have the additional packages pre-installed.

@yhuiYH
yhuiYH merged commit 2fd7f96 into main Nov 2, 2026
399 of 407 checks passed
@yhuiYH
yhuiYH deleted the users/yhui/add-new-tests branch November 2, 2026 18:02
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Issue]: With ROCm 10.1, rocprofv3-attach is not found

3 participants