Skip to content

Add test filter standardisation for miopen-provider - #5775

Merged
dileepr1 merged 3 commits into
mainfrom
users/dravindr/tr_miopen_prov
Jun 26, 2026
Merged

Add test filter standardisation for miopen-provider#5775
dileepr1 merged 3 commits into
mainfrom
users/dravindr/tr_miopen_prov

Conversation

@dileepr1

@dileepr1 dileepr1 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Motivation

Add test filter standardisation for miopen-provider.
This works with the companion PR ROCm/rocm-libraries#7901

Technical Details

  • fetch_test_configurations.py: miopenprovider test_script -> test_runner.py so the generic runner drives ctest from THEROCK_BIN_DIR/miopen_plugin using the quick/standard/comprehensive/full category labels (replacing the legacy test_miopenprovider.py one-off).
  • test_runner.py: add miopenprovider -> miopen_plugin to COMPONENT_DIR_MAPPING so the job name maps to the actual install dir name under THEROCK_BIN_DIR.
  • ml-libs/artifact-miopenprovider.toml: broaden the test-component include to bin/miopen_plugin/** so the YAML-generated install-tree CTestTestfile.cmake (and the binaries it references via relative paths) ship in the artifact. The companion rocm-libraries change in users/dravindr/tf_miopen_prov writes that file to bin/miopen_plugin/CTestTestfile.cmake.

Test Plan

Confirm that the CI tests for miopen-provider work once ROCm/rocm-libraries#7901 is merged.

Submission Checklist

Made with Cursor

@dileepr1 @cursoragent
Add test filter standardisation for miopen-provider
e5bba86 
Paired with rocm-libraries branch users/dravindr/tf_miopen_prov which
adds dnn-providers/miopen-provider/test_categories.yaml and installs a
YAML-generated bin/miopen_plugin/CTestTestfile.cmake (via the shared
apply_test_category_labels() helper, applied to both miopen_plugin_tests
and miopen_plugin_integration_tests).

- fetch_test_configurations.py: miopenprovider test_script switched from
  test_miopenprovider.py to the generic test_runner.py. The generic
  runner drives ctest from THEROCK_BIN_DIR/miopen_plugin using the
  YAML-generated quick / standard / comprehensive / full category labels
  instead of the legacy GTEST_FILTER="-Full*" one-off.
- test_executable_scripts/test_runner.py: add
  "miopenprovider": "miopen_plugin" to COMPONENT_DIR_MAPPING so the
  runner CDs into THEROCK_BIN_DIR/miopen_plugin/ (the install dir name
  differs from the job name, as already noted in
  artifact-miopenprovider.toml).
- ml-libs/artifact-miopenprovider.toml: broaden the test component's
  include from individually-listed CTestTestfile.cmake + *.toml entries
  to bin/miopen_plugin/** so the install-tree CTestTestfile.cmake and
  any sibling config TOMLs (e.g. MIOPEN_ENGINE.toml) are bundled in the
  miopenprovider artifact. Without this, test_runner.py errors with
  "Test directory does not exist: build/bin/miopen_plugin".

Legacy test_miopenprovider.py is kept in this PR as a fallback for one
release and as the documented source of the YAML's exclude list.

Co-authored-by: Cursor <cursoragent@cursor.com>
@dileepr1
dileepr1 marked this pull request as draft June 11, 2026 13:56
@dileepr1
dileepr1 marked this pull request as ready for review June 26, 2026 15:37
@therock-pr-bot

therock-pr-bot Bot commented Jun 26, 2026 •
edited
Loading

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
───
Error: PR description must reference a JIRA ID or ISSUE ID.
Expected: include a JIRA ID or ISSUE ID line. The separator may be : or - (or omitted), and the value can be a JIRA key, a number (with or without #), or a link. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
Current: no valid JIRA/ISSUE reference found
⛔ 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 commented Jun 26, 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
───
Error: PR description must reference a JIRA ID or ISSUE ID.
Expected: include a JIRA ID or ISSUE ID line. The separator may be : or - (or omitted), and the value can be a JIRA key, a number (with or without #), or a link. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
Current: no valid JIRA/ISSUE reference found
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ⏳ Pending ⏳ Still running…
🚫 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 26, 2026
@therock-pr-bot

therock-pr-bot Bot commented Jun 26, 2026

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.

1 similar comment
@therock-pr-bot

therock-pr-bot Bot commented Jun 26, 2026

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.

geomin12
geomin12 approved these changes Jun 26, 2026
View reviewed changes

@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 pass but 11 mins for "quick" tests is too long

@dileepr1

dileepr1 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

tests pass but 11 mins for "quick" tests is too long

@geomin12 Thanks for bringing this into attention. I looked into this and the increase in time is caused by the heavy logs produced by ctest -V, which is not there in test_miopenprovider.py. I've landed an opt out from -V option in #5522 .
I'll merge this now and opt out of -V for miopen-provider once #5522 lands.

@dileepr1
dileepr1 merged commit 3abb2fc into main Jun 26, 2026
167 of 182 checks passed
@dileepr1
dileepr1 deleted the users/dravindr/tr_miopen_prov branch June 26, 2026 21:09
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Jun 26, 2026
@adickin-amd adickin-amd mentioned this pull request Jul 31, 2026
Merged
1 of 4 tasks
brockhargreaves-amd pushed a commit that referenced this pull request Aug 5, 2026
@adickin-amd
ci(hipkernelprovider): add test filter standardization (#7042)
820c35c 
## Summary

Switches TheRock's `hipkernelprovider` test job from the bespoke
`test_hipkernelprovider.py` to the shared `test_runner.py`, so
hip-kernel-provider tests are selected by the standardized
`quick`/`standard`/`comprehensive`/`full` CTest category labels instead
of an ad hoc `GTEST_FILTER=-Full*`. This mirrors the miopen-provider
standardization in [PR
#5775](#5775). The labeled
`bin/hip_kernel_provider/CTestTestfile.cmake` this relies on comes in
through the regular `rocm-libraries` submodule pointer on `main`; this
PR no longer bumps the submodule itself.

JIRA ID : ALMIOPEN-2278

## Risk Assessment

Low risk. CI configuration only — no product code, no public API, no
build wiring, no submodule movement. The behavioral change is which
tests the `hipkernelprovider` job selects; a mislabeled or missing CTest
label surfaces as tests being skipped rather than as a build or product
failure.

## ASIC Coverage

TheRock PR CI runs the `hipkernelprovider` component job on all
supported GPU families (the job declares no `exclude_family`), so
passing PR CI is the required coverage; no separate multi-arch sweep is
needed. The label-driven selection is arch-independent —
`HIP_MLOPS_ENGINE_test_categories.yaml` matches on GTest names, not
architecture — so no arch-scoped gate applies.

## Testing Summary

- Unit coverage for the CI configuration generator, confirming the
`hipkernelprovider` entry still produces a valid test configuration
after the `test_script` swap.
- Runtime validation of the new path (label discovery, `ctest -L
^quick$` selection, install-dir resolution) is deferred to PR CI, which
is the only place the built artifact exists.
- Requires the `rocm-libraries` pointer on `main` to already carry the
labeled install-tree `CTestTestfile.cmake`; if PR CI reports zero
selected tests, that pointer has not landed yet.

## Testing Checklist

- [x] CI config generator unit tests - `pytest
build_tools/github_actions/tests/fetch_test_configurations_test.py` -
Status: Passed
- [ ] hipkernelprovider component tests - `test_runner.py` with
`TEST_COMPONENT=hipkernelprovider` - Status: Pending
- [ ] Test coverage for the new `COMPONENT_DIR_MAPPING` entry - Status:
Not added
- [ ] PR CI - GitHub PR checks - Status: Pending

## Technical Changes

- `fetch_test_configurations.py`: `hipkernelprovider` `test_script` now
points at `test_runner.py`, so the generic runner drives `ctest` from
`THEROCK_BIN_DIR/hip_kernel_provider` using the category labels,
replacing the legacy `test_hipkernelprovider.py` one-off.
- `test_runner.py`: adds `hipkernelprovider -> hip_kernel_provider` to
`COMPONENT_DIR_MAPPING`. `TEST_COMPONENT` is set from the job name,
which does not match the provider's install directory name under
`THEROCK_BIN_DIR`.
- `rocm-libraries`: no longer touched. The earlier bump to
`8e3b05a0dd51` has been reverted to the base pointer; the labeled
`CTestTestfile.cmake` and tiered GTest/external-integration/rocKE labels
arrive via the normal submodule advance on `main`.
- `ml-libs/artifact-hipkernelprovider.toml` intentionally unchanged:
unlike the miopen-provider manifest, its test component already ships
`CTestTestfile.cmake`, both GTest binaries, the engine config TOMLs, and
the rocKE test payload, so no include broadening is required.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@geomin12 geomin12 geomin12 approved these changes

Assignees

No one assigned

Labels

Not ready to Review PR has unresolved policy failures — reviews blocked

Projects

TheRock Triage
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dileepr1 @geomin12