Skip to content

[TheRock CI] Update projects_to_test lists#3749

Merged
jayhawk-commits merged 14 commits into
developfrom
users/jayhawk-commits/projects_to_test_march4
Mar 6, 2026
Merged

[TheRock CI] Update projects_to_test lists#3749
jayhawk-commits merged 14 commits into
developfrom
users/jayhawk-commits/projects_to_test_march4

Conversation

@jayhawk-commits
Copy link
Copy Markdown
Collaborator

@jayhawk-commits jayhawk-commits commented Mar 4, 2026

- Based on recent tests that were enabled on TheRock repo, updating the `projects_to_test` entries for different projects.
@github-actions github-actions Bot added the github actions Pull requests that update GitHub Actions code label Mar 4, 2026
"profiler": {
"cmake_options": "-DTHEROCK_ENABLE_ALL=ON",
"projects_to_test": "rocprofiler-tests",
"projects_to_test": "aqlprofile, rocprofiler-compute, rocprofiler_systems",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks like rocprofiler-compute has a dash -, whereas rocprofiler_systems has an underscore _.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, there was inconsistency between rocprofiler-compute and rocprofiler-systems with these definitions. I created a PR to address this; we can fix this in a later PR: ROCm/TheRock#3757

Comment thread .github/scripts/therock_matrix.py Outdated
@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

I think I need to wait for #3709

@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

Updating TheRock SHAs to test against ROCm/TheRock#3768

Copy link
Copy Markdown
Contributor

@geomin12 geomin12 left a comment

Choose a reason for hiding this comment

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

overall lgtm

github issues right now, but if CI passes, lgtm!

Comment thread .github/scripts/therock_matrix.py
@jbonnell-amd
Copy link
Copy Markdown
Collaborator

I noticed that the rocprofiler_compute tests are failing due to missing Python requirements. We'll probably need to port over these changes from TheRock's test_component.yml script over to rocm-system's therock-test-component.yml equivalent to get these requirements installed.

https://github.com/ROCm/TheRock/blob/main/.github/workflows/test_component.yml#L134-L138

@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

jayhawk-commits commented Mar 5, 2026

The tests get launched with my PR's changes. I will remove the changes to TheRock SHAs in this PR, and will rely on #3709

@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

I noticed that the rocprofiler_compute tests are failing due to missing Python requirements. We'll probably need to port over these changes from TheRock's test_component.yml script over to rocm-system's therock-test-component.yml equivalent to get these requirements installed.

https://github.com/ROCm/TheRock/blob/main/.github/workflows/test_component.yml#L134-L138

Thanks for catching this. I have added the step to this repo.

@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

Test environment for hip-tests and rocgdb are in a bad state.

@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

jayhawk-commits commented Mar 6, 2026

Further changes are required to support the custom container for rocgdb testing, so that will be deferred to a separate PR. @lumachad

rocr-debug-agent testing looks OK in this PR.

@lumachad
Copy link
Copy Markdown
Contributor

lumachad commented Mar 6, 2026

Further changes are required to support the custom container for rocgdb testing, so that will be deferred to a separate PR. @lumachad

rocr-debug-agent testing looks OK in this PR.

Nice! thanks. But I think the latest run failed to find the testing scripts for some reason. On rocgdb testing being deferred to another PR, that's fine. Please ping me on the PR and I can keep an eye out for it.

@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

Further changes are required to support the custom container for rocgdb testing, so that will be deferred to a separate PR. @lumachad
rocr-debug-agent testing looks OK in this PR.

Nice! thanks. But I think the latest run failed to find the testing scripts for some reason. On rocgdb testing being deferred to another PR, that's fine. Please ping me on the PR and I can keep an eye out for it.

Was fine in this run https://github.com/ROCm/rocm-systems/actions/runs/22729017651/job/65931062366 where it was a different SHA of TheRock.

…b.com:ROCm/rocm-systems into users/jayhawk-commits/projects_to_test_march4
@lumachad
Copy link
Copy Markdown
Contributor

lumachad commented Mar 6, 2026

Further changes are required to support the custom container for rocgdb testing, so that will be deferred to a separate PR. @lumachad
rocr-debug-agent testing looks OK in this PR.

Nice! thanks. But I think the latest run failed to find the testing scripts for some reason. On rocgdb testing being deferred to another PR, that's fine. Please ping me on the PR and I can keep an eye out for it.

Was fine in this run https://github.com/ROCm/rocm-systems/actions/runs/22729017651/job/65931062366 where it was a different SHA of TheRock.

So it was. Looks like we're failing to checkout a particular TheRock revision. From looking at the rocgdb testing failure, your assessment that we're missing using the proper rocgdb norocm image is correct. I expect things will run correctly once we use the right container image.

@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

jayhawk-commits commented Mar 6, 2026

For rocminfo and amd-smi coverage, we need to add the sanity check that TheRock does. We can do that in another PR. See ROCm/TheRock#3823.

@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

Created #3847 for rocgdb gap.

@jayhawk-commits jayhawk-commits marked this pull request as ready for review March 6, 2026 21:56
@jayhawk-commits jayhawk-commits requested a review from a team as a code owner March 6, 2026 21:56
Copilot AI review requested due to automatic review settings March 6, 2026 21:56
@jayhawk-commits
Copy link
Copy Markdown
Collaborator Author

jayhawk-commits commented Mar 6, 2026

TheRock CI passed before I changed it from draft mode.
See https://github.com/ROCm/rocm-systems/actions/runs/22776829238
Canceled the new CI run from post-draft to save resources.

Copy link
Copy Markdown
Contributor

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

Updates TheRock-based CI workflows to use a newer TheRock commit SHA (2026-03-06) and refreshes the per-component projects_to_test lists so the generated test matrix aligns with current TheRock test configurations.

Changes:

  • Bump TheRock checkout refs across multiple workflows to ad524887d223cbd04d6b423691958fe86b589944.
  • Update .github/scripts/therock_matrix.py projects_to_test mappings (core/profiler/debug/all).
  • Add a workflow step to install “additional requirements” before running component tests.

Reviewed changes

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

Show a summary per file
File Description
.github/workflows/therock-test-packages.yml Updates TheRock ref used to generate the test matrix and run package tests.
.github/workflows/therock-test-component.yml Updates TheRock ref and adds a step to install component-specific extra requirements.
.github/workflows/therock-rccl-test-packages-multi-node.yml Updates TheRock ref used for multi-node RCCL tests (plus whitespace cleanup).
.github/workflows/therock-rccl-ci-linux.yml Updates TheRock ref used by RCCL CI on Linux.
.github/workflows/therock-ci-windows.yml Updates TheRock ref used by Windows CI.
.github/workflows/therock-ci-linux.yml Updates TheRock ref used by Linux CI.
.github/scripts/therock_matrix.py Refreshes which tests to run per logical project grouping.

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

Comment thread .github/workflows/therock-test-component.yml
Comment thread .github/scripts/therock_matrix.py
Comment thread .github/scripts/therock_matrix.py
@jayhawk-commits jayhawk-commits merged commit a0712d4 into develop Mar 6, 2026
30 of 33 checks passed
@jayhawk-commits jayhawk-commits deleted the users/jayhawk-commits/projects_to_test_march4 branch March 6, 2026 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github actions Pull requests that update GitHub Actions code organization: ROCm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants