Skip to content

feat(ci): add ci:cpu-test-only label to skip GPU tests (#7375) - #8768

Merged
geomin12 merged 2 commits into
mainfrom
users/geomin12/cpu-only-label
Oct 7, 2026
Merged

geomin12 merged 2 commits into
mainfrom
users/geomin12/cpu-only-label

Conversation

@geomin12

@geomin12 geomin12 commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Add a new PR label ci:cpu-test-only that disables GPU tests while still allowing CPU-only tests to run. This is useful for changes that don't require GPU testing, such as documentation, build scripts, or CPU-only components.

The label works by setting test-runs-on to empty (disabling GPU tests) while keeping test-runs-on-cpu populated, so components with linux_cpu_runner: True can still run their tests.

Changes:

  • Add label definition to configure_ci_labels.py
  • Add gating logic in configure_multi_arch_ci.py
  • Document label in ci_behavior_manipulation.md
  • Add unit tests for the new functionality

Working here: https://github.com/ROCm/TheRock/actions/runs/37391207371/job/112069740981?pr=8768 (i needed to fix unit tests)

Progress on #7375
ISSUE ID: #7375

@geomin12 geomin12 added the ci:cpu-test-only Run only CPU tests (skip GPU tests) label Oct 5, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 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.

🙋 Wish to Override Policy?

@therock-pr-bot

therock-pr-bot Bot commented Oct 5, 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.

@geomin12
geomin12 force-pushed the users/geomin12/cpu-only-label branch from 608e237 to 4d28b11 Compare October 5, 2026 23:03
@therock-pr-bot

therock-pr-bot Bot commented Oct 5, 2026

Copy link
Copy Markdown

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

@geomin12
geomin12 marked this pull request as ready for review October 6, 2026 16:31
Comment on lines 1631 to 1635

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also disable multi-GPU when ci:cpu-test-only is set. See on the test run https://github.com/ROCm/TheRock/actions/runs/37391207371?pr=8768 how "Multi-GPU Runner" is still set:

Image

I wonder if the unit tests could then be reworked to check for the no mentions of "gpu" or "gfx" in the family info, instead of just looking for test-runs-on. Well, at least checking test-runs-on-multi-gpu in addition would help, but thinking of more general test design that catches bugs like this without needing to code directly for it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was resolved with c91d526

Comment on lines 1608 to 1611
# CPU test runner for components that don't need GPU access (e.g.,
# components with linux_cpu_runner: True). This allows CPU-only tests
# to run even when GPU testing is gated (e.g., trigger_test_label_only).
test_runs_on_cpu = get_cpu_test_runner(platform)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future follow-up, we may want to use small/medium machines for CPU testing of some components. The default large runners are pretty oversized for some ctest/pytest projects. For that we could build off the code recently added here and plumb these through too:

    # Select build runner using weighted distribution
    build_runs_on_large = select_build_runner(platform, build_variant, size="large")
    build_runs_on_small = select_build_runner(platform, build_variant, size="small")
    build_runs_on_medium = select_build_runner(platform, build_variant, size="medium")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh 100%. i'll do as follow-up

@geomin12
geomin12 requested a review from ScottTodd October 7, 2026 15:48
Comment on lines +3049 to +3065
# ---------------------------------------------------------------------------
# Emergency test-queue lever (ROCm/TheRock#8688)
#
# gfx110X Windows presubmit testing was removed from tests_on_trigger for
# test-queue remediation; builds still run on presubmit and tests are taken
# on-demand via the `ci:test:gfx110x` label. These tests pin that behavior.
# Superseded by the permanent build/test label plumbing in #8692.
# ---------------------------------------------------------------------------
class TestGfx110xWindowsTestLever(unittest.TestCase):
"""gfx110X Windows: no presubmit tests by default, re-enabled by label.

The lever is scoped to Windows (ROCm/TheRock#8688): the `ci:test:gfx110x`
label must not re-enable Linux gfx110X presubmit tests, which stay
nightly-only.
"""

def _pr_inputs(self, **kwargs):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These TestGfx110xWindowsTestLever tests look unrelated to the cpu-test-only label? Are they from a different commit/PR/merge?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that was messy with commits. it's resolved.

geomin12 and others added 2 commits October 7, 2026 11:15
Add a new PR label `ci:cpu-test-only` that disables GPU tests while
still allowing CPU-only tests to run. This is useful for changes that
don't require GPU testing, such as documentation, build scripts, or
CPU-only components.

The label works by setting `test-runs-on` to empty (disabling GPU tests)
while keeping `test-runs-on-cpu` populated, so components with
`linux_cpu_runner: True` can still run their tests.

Changes:
- Add label definition to configure_ci_labels.py
- Add gating logic in configure_multi_arch_ci.py
- Document label in ci_behavior_manipulation.md
- Add unit tests for the new functionality

Closes #7375

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The ci:cpu-test-only label was disabling single-GPU tests but multi-GPU
runner info was still being included in the family_info, causing
multi-GPU test jobs to be scheduled.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@geomin12
geomin12 force-pushed the users/geomin12/cpu-only-label branch from eddc186 to 546cbfa Compare October 7, 2026 18:17
@geomin12
geomin12 requested a review from ScottTodd October 7, 2026 18:20
@geomin12
geomin12 merged commit b1645ed into main Oct 7, 2026
45 checks passed
@geomin12
geomin12 deleted the users/geomin12/cpu-only-label branch October 7, 2026 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:cpu-test-only Run only CPU tests (skip GPU tests)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants