ci: sync pr bot policy_check.py from TheRock and add hip-tests path pattern by amd-chrissosa · Pull Request #8778 · ROCm/rocm-systems

amd-chrissosa · 2026-07-17T18:07:44Z

Summary

Three changes to fix the unit test check incorrectly flagging CLR PRs:

1. Sync policy_check.py from ROCm/TheRock (canonical source)

The version in rocm-systems was behind and lacked _is_test_file(), which supports path-based test file recognition. Patterns containing / are now matched against the full repo-relative file path; patterns without / match the basename only.

2. Add projects/hip-tests/** to test_file_patterns in policy.yml

CLR's tests live in the separate projects/hip-tests/ subproject and are named after the API they test (atomicAdd.cc, acquire_release.cc, etc.) with no test_* prefix or _test suffix. Without this path pattern, any CLR PR touching hip-tests/ alongside source changes would be incorrectly flagged as missing a unit test.

Also aligns the basename patterns with TheRock (testing_*, *_tests.*, Test*).

3. Add test_policy_check_ut.py

Synced from TheRock with rocm-systems-specific additions covering the hip-tests path pattern.

Tracks: ROCm/TheRock#6654

Closes #8778

Test plan

33 tests pass: python -m pytest tools/systems_pr_bot/test_policy_check_ut.py
test_hip_tests_path_recognised_by_shipped_policy: CLR source + projects/hip-tests/catch/unit/memory/atomicAdd.cc → passes unit test check
test_path_based_pattern_code_only_still_fails: CLR source with no hip-tests changes → still fails correctly
Existing test_* / *_test.* patterns continue to work as before

🤖 Generated with Claude Code

…attern Two related changes: 1. Sync policy_check.py from ROCm/TheRock (canonical source). The key addition is _is_test_file(), which supports two matching modes: - Basename patterns (no '/'): matched against filename only - Path patterns (contain '/'): matched against full repo-relative path The previous version lacked path-based matching entirely. 2. Add projects/hip-tests/** to test_file_patterns in policy.yml. CLR's tests live in the separate projects/hip-tests/ subproject and are named after the API they test (atomicAdd.cc, acquire_release.cc, etc.) with no test_* prefix or _test suffix. Without this path pattern, any CLR PR that touched hip-tests alongside source changes would be incorrectly flagged as missing a unit test. Also adds testing_*, *_tests.*, and Test* basename patterns from TheRock. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

therock-pr-bot · 2026-07-17T18:08:37Z

✅ All Policy Checks Passed

Check	Status	Details
🌿 Branch Name	✅ Pass	—
📝 PR Title/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.

therock-pr-bot · 2026-07-17T18:08:38Z

🚫 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.

Adds test_policy_check_ut.py, synced from ROCm/TheRock (canonical source) with rocm-systems-specific additions: - test_hip_tests_path_recognised_by_shipped_policy: integration test that loads the real policy.yml and verifies a CLR PR touching projects/hip-tests/catch/unit/memory/atomicAdd.cc alongside source changes correctly passes the unit test check - test_unit_test_patterns_exclude_unit_glob: updated to assert projects/hip-tests/** is present in the loaded policy Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR updates the Systems PR Bot’s unit-test detection logic to support path-based test file recognition (needed for projects/hip-tests/**), and aligns the policy’s test file patterns to avoid falsely flagging CLR PRs that do include tests.

Changes:

Extend unit-test detection to support full-path glob patterns (in addition to basename-only patterns).
Add projects/hip-tests/** (and align several basename patterns) to test_file_patterns in policy.yml.
Sync additional behavior from TheRock’s policy_check.py (including some refactors/removals).

Reviewed changes

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

File	Description
tools/systems_pr_bot/policy.yml	Updates test file patterns and documents basename-vs-path matching behavior; adds `projects/hip-tests/**` to recognize hip-tests as tests.
tools/systems_pr_bot/policy_check.py	Implements `_is_test_file()` for path-aware matching and updates unit-test detection logic accordingly; includes other synced changes.

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

-            build_policy_table_comment(
-                build_bump_pr_results(policy), marker, ready=True, note=note
-            ),
+            note,


        base = Path(filename).name
        ext = Path(filename).suffix.lower()


        base = Path(filename).name
        ext = Path(filename).suffix.lower()


    "Unit Test",
    "pre-commit",
    "Draft PR",
-    "PR Size",
    "Feature Flag",
    "Code Coverage",
    "therock-pr-bot",


        footer = "\n\n> 🎉 All policy checks passed!"

-    faq_url = "https://github.com/ROCm/rocm-systems/tree/develop/docs/SYSTEMS_PR_BOT_FAQ.md"
+    faq_url = "https://github.com/ROCm/TheRock/blob/main/skills/therock_pr_bot/FAQ.md"


    # PR title rules now live under the nested `title:` mapping.
    title_cfg = pr.get("title", {}) or {}
-    title_patterns_raw = title_cfg.get("pattern", []) or []
-    title_patterns = [re.compile(str(p)) for p in title_patterns_raw]
    title_min_length = int(title_cfg.get("title_min_length", 0) or 0)
    title_max_length = int(title_cfg.get("title_max_length", 0) or 0)



Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Changes to tools/systems_pr_bot/ don't touch any build subtree, but therock_configure_ci.py falls back to running all projects when modified files don't match a known subtree. Add paths-ignore to the pull_request trigger so the full Linux/Windows build matrix doesn't fire for bot, script, or tooling-only PRs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

This reverts commit 8dd1eb6.

therock_configure_ci.py fell back to evaluating all build subtrees when modified files didn't match any known subtree. For PRs that only touch tools/ (bot scripts, utilities) this triggered a full Linux+Windows build matrix with no benefit. Add CI_SKIP_PATH_PREFIXES (initially ['tools/']) and check_only_ci_skip_paths() so that PRs whose every changed file is under a skip prefix produce an empty subtree list and skip the build CI entirely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

This reverts commit 9bb66d6.

amd-chrissosa · 2026-07-17T18:51:24Z

Superseded by #8782 (clean branch without the CI investigation commits).

Copilot AI review requested due to automatic review settings July 17, 2026 18:07

Copilot started reviewing on behalf of amd-chrissosa July 17, 2026 18:08 View session

therock-pr-bot Bot added the Not ready to Review label Jul 17, 2026

Copilot AI reviewed Jul 17, 2026

View reviewed changes

systems-assistant Bot added the organization: ROCm label Jul 17, 2026

amd-chrissosa and others added 2 commits July 17, 2026 11:31

ci: apply black formatting to test_policy_check_ut.py

2e29bda

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

amd-chrissosa requested a review from a team as a code owner July 17, 2026 18:38

github-actions Bot added the github actions Pull requests that update GitHub Actions code label Jul 17, 2026

amd-chrissosa and others added 3 commits July 17, 2026 11:42

Revert "ci: skip TheRock full build CI for tools/ and bot-only changes"

7239e19

This reverts commit 8dd1eb6.

Revert "ci: skip TheRock build CI for tooling-only changes (tools/**)"

b141f16

This reverts commit 9bb66d6.

amd-chrissosa mentioned this pull request Jul 17, 2026

ci: sync pr bot from TheRock and add hip-tests path pattern #8782

Closed

4 tasks

amd-chrissosa closed this Jul 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ci: sync pr bot policy_check.py from TheRock and add hip-tests path pattern#8778

ci: sync pr bot policy_check.py from TheRock and add hip-tests path pattern#8778
amd-chrissosa wants to merge 7 commits into
developfrom
users/chrisosa/pr-bot-sync-and-hip-tests-pattern

amd-chrissosa commented Jul 17, 2026 •

edited

Loading

Uh oh!

therock-pr-bot Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

therock-pr-bot Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

amd-chrissosa commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		base = Path(filename).name
		ext = Path(filename).suffix.lower()

Uh oh!

Conversation

amd-chrissosa commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

therock-pr-bot Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Policy Checks Passed

Uh oh!

therock-pr-bot Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

amd-chrissosa commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

amd-chrissosa commented Jul 17, 2026 •

edited

Loading

therock-pr-bot Bot commented Jul 17, 2026 •

edited

Loading

therock-pr-bot Bot commented Jul 17, 2026 •

edited

Loading