Skip to content

test(hipkernelprovider): Activate rocke python tests#9249

Draft
brockhargreaves-amd wants to merge 9 commits into
developfrom
users/brockhargreaves-amd/activate-rocke-python-tests
Draft

test(hipkernelprovider): Activate rocke python tests#9249
brockhargreaves-amd wants to merge 9 commits into
developfrom
users/brockhargreaves-amd/activate-rocke-python-tests

Conversation

@brockhargreaves-amd

Copy link
Copy Markdown
Contributor

Motivation

ISSUE ID : AICK-1517

The rocKE engine has a comprehensive Python test suite (tests covering DSL optimization, dispatch, instances, IR serialization, multiarch, and more) that currently only runs from the source tree. This PR wires the pytest suite into the installed test artifact and registers it with ctest so it runs alongside the existing hip-kernel-provider tests in CI.

Technical Details

Three files changed:

hip-kernel-provider/CMakeLists.txt

rocke/platform/CMakeLists.txt

  • Add install(DIRECTORY tests/ ...) to stage the 31 pytest test files, conftest.py, pytest.ini, and __init__.py files into the test artifact. Excludes C/C++ parity emitters (compiled against rocke_core at build time), the differential harness (needs compiler + source), and run_all.py (dev-time runner).
  • Fix Windows compatibility: invoke Python test scripts via python command rather than executing .py files directly (shebangs are not supported on Windows).
  • Register a rocke_pytest ctest entry that invokes python -m pytest with PYTHONPATH set to the installed rocke package location, so import rocke resolves without the source-tree sys.path shims.

rocke/platform/tests/test_rocke_ci_static.py

  • Fix examples path resolution to work in both the source tree (Python/rocke/examples/) and the installed layout (rocke/examples/). The installed layout does not have the intermediate Python/ directory.

Test Plan

  • Built TheRock locally on Windows (gfx1151) with THEROCK_FLAG_HIPKERNELPROVIDER_ENABLE_ROCKE=ON and BUILD_TESTING=ON
  • Verified test files are staged to build/ml-libs/hipkernelprovider/stage/bin/hip_kernel_provider/tests/
  • Ran ctest from the stage directory — existing rocke tests (rocke_ir_serialize_roundtrip, rocke_tiled_attention_2d_reentrancy, rocke_python_import_smoke, rocke_golden_static) all pass
  • Ran python -m pytest ./tests from the stage directory with PYTHONPATH=. and ROCM_PATH set

Test Result

With ROCM_PATH set (as CI's test_hipkernelprovider.py already does):

  • 396 passed, 0 failed, 32 skipped, 37 subtests passed

We will also need to wait on CI to ensure it passes.

Submission Checklist

brockhargreaves-amd and others added 2 commits July 9, 2026 13:30
…th ctest

Enable ROCKE_INSTALL_PYTHON_TESTS in provider mode (was explicitly OFF
while artifact packaging was not yet wired) and install the full pytest
suite into the test artifact alongside the existing smoke and golden
gate scripts.

Changes:
- Remove `set(ROCKE_INSTALL_PYTHON_TESTS OFF)` from the provider
  CMakeLists.txt — TheRock's artifact-hipkernelprovider.toml now
  captures the Python files.
- Add `install(DIRECTORY tests/ ...)` to stage the 31 pytest test files,
  conftest.py, pytest.ini, and __init__.py files. Excludes C/C++ parity
  emitters, the differential harness, and run_all.py (all build-time
  only).
- Register a `rocke_pytest` ctest entry that invokes `python -m pytest`
  with PYTHONPATH set to the installed rocke package location.
- Fix Windows compatibility: invoke Python test scripts via `python`
  command rather than executing .py files directly (shebangs don't work
  on Windows).
- Fix test_rocke_ci_static.py examples path resolution to work in both
  the source tree (Python/rocke/examples/) and the installed layout
  (rocke/examples/).

Tested locally: 398 passed, 0 failed, 30 skipped (with ROCM_PATH set).

Co-Authored-By: Claude <noreply@anthropic.com>
REVERT BEFORE MERGING — therock-ref bumps are managed by rocm-libraries
maintainers. This is a temporary override for CI testing of the rocKE
pytest suite changes.

Co-Authored-By: Claude <noreply@anthropic.com>
@therock-pr-bot

therock-pr-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
🌿 Branch Name ✅ Pass
📝 PR Title/Description ✅ Pass
Forbidden Files ✅ Pass
🧪 Unit Test ✅ Pass PR does not contain code files — Unit Test auto-passed
🔎 pre-commit ✅ Pass
🚫 Draft PR 🔜 To Be Enabled
🚩 Feature Flag 🔜 To Be Enabled
📊 Code Coverage 🔜 To Be Enabled
🤖 therock-pr-bot ✅ Pass

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

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

@therock-pr-bot

therock-pr-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

Pre-commit check failed

pre-commit failed

Please run locally:

  • python -m pip install pre-commit
  • pre-commit install
  • pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

brockhargreaves-amd and others added 5 commits July 10, 2026 08:59
REVERT BEFORE MERGING — therock-ref bumps are managed by rocm-libraries
maintainers. This is a temporary override for CI testing of the rocKE
pytest suite and GPU detection changes.

Co-Authored-By: Claude <noreply@anthropic.com>
@brockhargreaves-amd brockhargreaves-amd marked this pull request as ready for review July 10, 2026 22:31
@brockhargreaves-amd brockhargreaves-amd requested review from a team as code owners July 10, 2026 22:31
@brockhargreaves-amd brockhargreaves-amd marked this pull request as draft July 10, 2026 22:32
brockhargreaves-amd and others added 2 commits July 10, 2026 17:35
…RN clauses

Multiple PATTERN clauses are valid CMake syntax for install(DIRECTORY).
The signature explicitly allows repeating the pattern block:

  [[PATTERN <pattern> | REGEX <regex>]
   [EXCLUDE] [PERMISSIONS permissions...]] [...])

See: https://cmake.org/cmake/help/latest/command/install.html

cmake-lint E1122 ("Duplicate keyword argument") does not account for
this, so we suppress it inline around the pytest suite install command.

Co-Authored-By: Claude <noreply@anthropic.com>
…RN clauses

cmake-lint does not support re-enabling a suppressed diagnostic (no
"enable" directive), so wrap the install() in a block() scope to
contain the suppression.

Co-Authored-By: Claude <noreply@anthropic.com>
@therock-pr-bot

Copy link
Copy Markdown

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

@brockhargreaves-amd brockhargreaves-amd marked this pull request as ready for review July 11, 2026 01:07
@brockhargreaves-amd brockhargreaves-amd marked this pull request as draft July 11, 2026 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants