Skip to content

fix(ci): skip gfx90a tests on PRs unless explicitly labeled - #8319

Merged
geomin12 merged 2 commits into
mainfrom
users/geomin12/gfx90a-off
Sep 17, 2026
Merged

geomin12 merged 2 commits into
mainfrom
users/geomin12/gfx90a-off

Conversation

@geomin12

Copy link
Copy Markdown
Contributor

gfx90a is a postsubmit-only architecture that was being tested on PRs when ci:run-all-archs label was present (e.g., submodule bumps). This adds trigger_test_label_only to gfx90a so tests only run when the gfx90a label is explicitly added.

Also fixes the trigger_test_label_only check to only apply to PRs - push (postsubmit) and workflow_dispatch should always run tests.

After this PR, I will work on #8222 to make this clearer

gfx90a is a postsubmit-only architecture that was being tested on PRs
when ci:run-all-archs label was present (e.g., submodule bumps). This
adds trigger_test_label_only to gfx90a so tests only run when the
gfx90a label is explicitly added.

Also fixes the trigger_test_label_only check to only apply to PRs -
push (postsubmit) and workflow_dispatch should always run tests.

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

therock-pr-bot Bot commented Sep 17, 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

Copy link
Copy Markdown

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

Comment on lines +1981 to +1982
def test_gfx90a_always_runs_tests(self):
"""gfx90a (no trigger_test_label_only) runs tests on push and workflow_dispatch."""
def test_gfx90a_runs_tests_on_push_and_workflow_dispatch(self):
"""gfx90a (trigger_test_label_only) runs tests on push and workflow_dispatch."""

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.

This file is growing quite a lot of test code for single targets that appear to be change detector tests that will fail as soon as the code changes too. The expectation (whether in policies, our heads, or in agent default behavior) that "all code changes have associated test changes" is hurting us here.

How about writing some tests with a test version of amdgpu_family_info_matrix_postsubmit that show how trigger_test_label_only and other fields should work and then letting the CI configuration change without always needing updates to unit tests?

Replace tests that depend on real production family names (gfx90a,
gfx125x, gfx950) with tests using synthetic mock families. This
decouples tests from CI configuration changes and avoids "change
detector" tests that break whenever the family matrix is modified.

The mock families (mock-presubmit, mock-postsubmit-labeled, mock-quick-only)
demonstrate how trigger_test_label_only and test_type_for_family should
behave without depending on actual production configurations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@geomin12
geomin12 merged commit 8438943 into main Sep 17, 2026
20 checks passed
@geomin12
geomin12 deleted the users/geomin12/gfx90a-off branch September 17, 2026 21:34
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants