Skip to content

Add test filter standardization for rocshmem#5691

Draft
dileepr1 wants to merge 8 commits into
mainfrom
users/dravindr/tr_rocshmem
Draft

Add test filter standardization for rocshmem#5691
dileepr1 wants to merge 8 commits into
mainfrom
users/dravindr/tr_rocshmem

Conversation

@dileepr1

@dileepr1 dileepr1 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

TheRock side of test-filter standardization for rocSHMEM, paired with ROCm/rocm-systems#6334.

rocm-systems#6334 adds native CTest integration for rocSHMEM: projects/rocshmem/tests/test_categories.yaml plus apply_ctest_category_labels() wiring that installs relocatable bin/rocshmem/**/CTestTestfile.cmake files (a root dispatcher that subdirs() into tests/functional and tests/unit) carrying the quick/standard/comprehensive/full tier labels. This PR wires TheRock so the generic test_runner.py can drive those tests from the installed artifact:

  • comm-libs/CMakeLists.txt — pass -DBUILD_CTESTS=${THEROCK_BUILD_TESTING} to the rocshmem subproject. The CTest registration and the install-tree CTestTestfile.cmake generation in (feat): [AILIKFD-40] add kfdtest to TheRock CI #6334 are gated behind BUILD_CTESTS (default OFF), so without this the install tree would contain no bin/rocshmem/** and test_runner.py would fail with Test directory does not exist.
  • comm-libs/artifact-rocshmem.toml — populate the previously-empty test component: bin/rocshmem/** (the install-tree CTest files), bin/rocshmem_unit_tests, the share/rocshmem/* CTest wrapper scripts, and the device_bitcode HSACOs. The functional binary and rocshmem_info already live in the run component (fetched via rocshmem_run).
  • build_tools/install_rocm_from_artifacts.py — add a --rocshmem artifact selector that pulls rocshmem_lib/_run/_test plus OpenMPI (mpirun + libs) needed to launch the tests.
  • build_tools/github_actions/fetch_test_configurations.py — add a rocshmem test job using test_runner.py (fetch_artifact_args = "--rocshmem --tests"), on Linux multi-GPU runners (the functional/unit tests launch via mpirun with RANKS 2..64, same setup as rccl).
  • fetch_test_configurations_test.py — relax the assertion that assumed exactly one multi-GPU job repo-wide, now that rocshmem is a second multi-GPU job alongside rccl.

test_runner.py needs no COMPONENT_DIR_MAPPING entry (job name rocshmem == install dir bin/rocshmem).

Test plan

  • python3 -m pytest build_tools/github_actions/tests/fetch_test_configurations_test.py (22 passed)
  • black --check on the edited Python files
  • --rocshmem registered in install_rocm_from_artifacts.py --help
  • artifact-rocshmem.toml parses; test include resolves
  • CI: build with THEROCK_BUILD_TESTING=ON, confirm bin/rocshmem/CTestTestfile.cmake + tests/{functional,unit}/CTestTestfile.cmake are installed and bundled in the test artifact
  • CI: TEST_COMPONENT=rocshmem TEST_TYPE=quick python test_runner.py discovers labels and runs ctest -L quick from THEROCK_BIN_DIR/rocshmem

⚠️ WIP — merge ordering

Depends on rocm-systems#6334. Do not merge until #6334 lands and TheRock's rocm-systems source includes it (otherwise the rocshmem source has no CTest infrastructure for BUILD_CTESTS to act on — see SKILL pitfall #8). Paired validation runs from #6334's CI by pointing its TheRock ref at users/dravindr/tr_rocshmem.

Made with Cursor

Paired with rocm-systems#6334, which adds native CTest integration for
rocSHMEM (tests/test_categories.yaml + apply_ctest_category_labels) and
installs relocatable bin/rocshmem/**/CTestTestfile.cmake files carrying the
quick/standard/comprehensive/full tier labels.

TheRock side:
- comm-libs/CMakeLists.txt: pass -DBUILD_CTESTS=${THEROCK_BUILD_TESTING} to the
  rocshmem subproject so the CTest registration runs and the install-tree
  CTestTestfile.cmake files are produced (the registration is gated behind
  BUILD_CTESTS, which defaults OFF).
- comm-libs/artifact-rocshmem.toml: include the install-tree CTest files
  (bin/rocshmem/**), the unit test binary, the CTest wrapper scripts and the
  device_bitcode HSACOs in the test component so the artifact bundles
  everything ctest needs (the functional binary and rocshmem_info come from
  the run component).
- install_rocm_from_artifacts.py: add a --rocshmem artifact selector that
  fetches rocshmem_lib/_run/_test plus OpenMPI (mpirun + libs) for the tests.
- fetch_test_configurations.py: add a rocshmem test job driven by the generic
  test_runner.py (ctest -L <quick|standard|comprehensive|full> from
  THEROCK_BIN_DIR/rocshmem) on linux multi-GPU runners.
- fetch_test_configurations_test.py: relax the single-multi-GPU-job assertion
  now that rocshmem is a second multi-GPU job alongside rccl.

WIP: depends on rocm-systems#6334. Do not merge until #6334 lands and TheRock's
rocm-systems source includes it; paired validation runs from #6334 CI by
pointing its TheRock ref at users/dravindr/tr_rocshmem.

Co-authored-by: Cursor <cursoragent@cursor.com>
Point the rocm-systems submodule to the latest develop tip
(068718c) which now contains the merged rocshmem CTest test-filter
standardization work (rocm-systems#6334, merge c541756). This lets
TheRock build rocshmem with the native CTest infrastructure and
exercise the tsf test job end-to-end.

Co-authored-by: Cursor <cursoragent@cursor.com>
@dileepr1 dileepr1 requested a review from a team as a code owner June 8, 2026 19:19
@dileepr1 dileepr1 marked this pull request as draft June 8, 2026 19:19
dileepr1 and others added 2 commits June 11, 2026 14:46
Point the rocm-systems submodule at develop tip 87fa69c, which now
includes rocshmem#6804 (Simple Local Runtime). #6804 removes the
configure-time MPIEXEC early-return in CTestFunctionalTests.cmake that
was force-disabling BUILD_CTESTS and preventing installation of
bin/rocshmem/CTestTestfile.cmake. With SLR as the MPI-less fallback,
the install-tree CTest file is generated and test_runner.py can find
build/bin/rocshmem.

Co-authored-by: Cursor <cursoragent@cursor.com>
Bump the rocm-systems submodule to rocshmem/therock-ci-fix (51ec214),
which fixes the install-tree CTest gating so ctest can discover tests
on a no-MPI test host. The install-root CTestTestfile template now
gates the mpiexec early-return behind USE_SLR_LAUNCHER, so in SLR mode
it falls through to the subdirs() calls and bin/rocshmem tests are
listed (previously "No tests found in build/bin/rocshmem").

Co-authored-by: Cursor <cursoragent@cursor.com>

@geomin12 geomin12 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.

tests fail + merge conflict

but it looks good

…shmem

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	build_tools/github_actions/tests/fetch_test_configurations_test.py
#	rocm-systems
@therock-pr-bot

therock-pr-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
🌿 Branch Name ✅ Pass
📝 PR Title/Description ❌ Fail Error: Title does not follow Conventional Commits style.
Expected: start with a valid type (feat, fix, docs, …).
Desired format: type(optional-scope): short description
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled

⚠️ 1 policy check(s) failed. Please address the issues above before this PR can be Reviewed.

🚫 Please fix the failed policies

  • ❌ PR Title/Description

The Not ready to Review label was added to this PR. Once all policies pass, the label is removed automatically.

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

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

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Title/Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

dileepr1 and others added 2 commits June 30, 2026 13:43
The rocshmem functional and unit test executables install to
share/rocshmem/ (CMAKE_INSTALL_DATADIR), but artifact-rocshmem.toml
referenced them as bin/rocshmem_functional_tests / bin/rocshmem_unit_tests,
so neither was packaged into any artifact. At test time the install-tree
CTestTestfile.cmake invokes them via ../../../../share/rocshmem/, which
was absent, causing 74/77 functional tests to fail with
"rocshmem_functional_tests: No such file or directory".

Include the binaries at their real share/rocshmem/ paths in the test
component and drop the dead bin/rocshmem_functional_tests glob from the
run component.

Co-authored-by: Cursor <cursoragent@cursor.com>
…shmem

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	rocm-systems
@therock-pr-bot therock-pr-bot Bot removed the Not ready to Review PR has unresolved policy failures — reviews blocked label Jun 30, 2026
…shmem

Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	rocm-systems
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: TODO

Development

Successfully merging this pull request may close these issues.

2 participants