ci: sync pr bot from TheRock, add hip-tests pattern, fix CI skip for tools/#8899
Open
amd-chrissosa wants to merge 1 commit into
Open
ci: sync pr bot from TheRock, add hip-tests pattern, fix CI skip for tools/#8899amd-chrissosa wants to merge 1 commit into
amd-chrissosa wants to merge 1 commit into
Conversation
…tools/
1. Sync policy_check.py from ROCm/TheRock (canonical source).
_is_test_file() now supports path-based glob patterns (patterns
containing '/' match the full repo-relative path) in addition to
basename matching. Also fixes bump PR upsert_comment to include the
marker in the body so existing comments are found/updated correctly,
and points the FAQ link to the local docs/SYSTEMS_PR_BOT_FAQ.md.
2. Update policy.yml:
- Add projects/hip-tests/** to test_file_patterns so CLR PRs that
touch hip-tests/ (whose files are named for the API, not test_*)
correctly pass the unit test check.
- Align basename patterns with TheRock (testing_*, *_tests.*, Test*).
- Remove pr.title.pattern — not parsed or enforced by this bot version.
- Remove pre-commit from required_check_runs — the pre-commit workflow
targets testbranch not develop, so it never runs on PRs here.
- Clarify test_file_patterns comment to describe both matching modes.
3. Add test_policy_check_ut.py synced from TheRock with rocm-systems
additions: test_hip_tests_path_recognised_by_shipped_policy verifies
the real policy.yml correctly passes a CLR + hip-tests PR. Fix
docstring paths (.github/therock_pr_bot -> tools/systems_pr_bot).
4. therock_configure_ci.py: add CI_SKIP_PATH_PREFIXES=['tools/'] so PRs
whose every changed file is under tools/ skip the full build matrix
instead of falling back to evaluating all subtrees.
Closes #8778
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
github-actions
Bot
added
the
github actions
4 tasks
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the tools/systems_pr_bot policy checker and its shipped policy to better recognize unit-test changes (including path-based test patterns like projects/hip-tests/**), improves bot comment upsert behavior for bump PRs, and reduces unnecessary CI by skipping builds for tooling-only (tools/**) changes.
Changes:
- Extend unit-test file detection to support full-path glob patterns (alongside basename patterns) and update the shipped
policy.ymlaccordingly (includingprojects/hip-tests/**). - Sync/refresh PR-bot behavior (FAQ link, bump PR comment marker inclusion) and add/update unit/integration tests for the policy checker.
- Update CI subtree detection to skip the build matrix when only
tools/**paths are modified.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tools/systems_pr_bot/test_policy_check_ut.py |
Adds/updates tests for path-based unit-test pattern matching and adjusts shipped-policy assertions. |
tools/systems_pr_bot/policy.yml |
Updates policy metadata, unit test patterns (incl. hip-tests path), and required check-run configuration. |
tools/systems_pr_bot/policy_check.py |
Adjusts FAQ link formatting and ensures bump PR comments include the marker for proper upsert behavior. |
tools/systems_pr_bot/.gitignore |
Ignores __pycache__/ in the bot directory. |
.github/scripts/therock_configure_ci.py |
Skips CI when all modified paths are in tools/ to avoid unnecessary full builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
+12
| Run locally: | ||
| python -m unittest tools/systems_pr_bot/test_policy_check_ut.py -v | ||
| # or | ||
| pytest tools/systems_pr_bot/test_policy_check_ut.py |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four changes to fix the unit test check incorrectly flagging CLR PRs, address Copilot review feedback, and stop unnecessary Linux builds on bot-only PRs.
1. Sync
policy_check.pyfrom ROCm/TheRock (#6654)_is_test_file()now supports path-based glob patterns alongside basename matchingupsert_commentnow includes the marker in the body so it correctly finds/updates existing comments rather than posting new ones each rundocs/SYSTEMS_PR_BOT_FAQ.md2. Update
policy.ymlprojects/hip-tests/**totest_file_patterns— CLR tests live here and are named after the API they test (atomicAdd.cc, etc.) with notest_*prefixtesting_*,*_tests.*,Test*)pr.title.pattern— not parsed or enforced by this bot version (Copilot feedback)pre-commitfromrequired_check_runs— the pre-commit workflow targetstestbranchnotdevelop, so it never runs and the bot always timed out waiting for ittest_file_patternscomment to document both matching modes3. Add
test_policy_check_ut.py.github/therock_pr_bot→tools/systems_pr_bot) (Copilot feedback)4.
therock_configure_ci.py: skip CI fortools/**changestools/matched no known build subtree and fell back to running all projects — triggering full Linux/Windows builds for bot-only PRsCI_SKIP_PATH_PREFIXES = ['tools/']so PRs with onlytools/changes skip the build matrixCloses #8782
Tracks: ROCm/TheRock#6654
Test plan
python -m pytest tools/systems_pr_bot/test_policy_check_ut.pytest_hip_tests_path_recognised_by_shipped_policy: CLR source +projects/hip-tests/catch/unit/memory/atomicAdd.cc→ passestest_path_based_pattern_code_only_still_fails: CLR source only → fails correctly🤖 Generated with Claude Code