Skip to content

fix(ci): respect trigger_test_label_only on postsubmit push events - #8407

Merged
geomin12 merged 2 commits into
mainfrom
users/geomin12/fix-trigger-test-label-only-postsubmit
Sep 22, 2026
Merged

geomin12 merged 2 commits into
mainfrom
users/geomin12/fix-trigger-test-label-only-postsubmit

Conversation

@geomin12

@geomin12 geomin12 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Fixes gfx125X tests running on postsubmit when they should only run with label
  • Changes trigger_test_label_only to gate tests on both pull_request and push triggers
  • Only workflow_dispatch bypasses the check (for manual testing)

Test plan

  • Unit tests updated and passing (TestFamilyTestFilters)
  • Verify gfx125X tests are skipped on postsubmit in rocm-libraries

@therock-pr-bot

therock-pr-bot Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

❌ PR Check — Action Required

Check Status Details
📝 PR Description ❌ Fail Error: PR description must reference a JIRA ID, ISSUE ID, or a GitHub closing keyword.
Expected: include a JIRA ID / ISSUE ID line (separator : or -, or omitted; value may be a JIRA key, a number with/without #, or a link), OR a closing keyword + issue reference. Accepted examples:
• JIRA ID : TESTAUTO-6039
• JIRA ID - #330
• JIRA ID #330
• JIRA ID (on separate line)
ROCM-25757
• ISSUE ID : TESTUTO-3334
• ISSUE ID #3334
• ISSUE ID - TESTAUTO-3433
• ISSUE ID (on separate line)
AIRUNTIME-2352
• ISSUE ID : https://github.com/<org_name>/<repo_name>/issues/1234
• Closes #10
• Fixes octo-org/octo-repo#100
• Resolves: #123
• #123
• https://github.com/<org_name>/<repo_name>/issues/123
Current: no valid JIRA/ISSUE/closing-keyword 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 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.

🙋 Wish to Override Policy?

@therock-pr-bot therock-pr-bot Bot added the Not ready to Review PR has unresolved policy failures — reviews blocked label Sep 21, 2026
@therock-pr-bot

Copy link
Copy Markdown

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

  • ❌ PR Description

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

Previously, families with trigger_test_label_only (like gfx125X) would
skip tests on PRs without the label but always run tests on postsubmit
push events. This was unintended for hardware-constrained families.

Now trigger_test_label_only gates tests on both pull_request and push
triggers. Only workflow_dispatch bypasses the check (for manual testing).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@geomin12
geomin12 force-pushed the users/geomin12/fix-trigger-test-label-only-postsubmit branch from a7340cc to 312231a Compare September 21, 2026 23:58
if (
platform_info.get("trigger_test_label_only", False)
and ci_inputs.is_pull_request
and not ci_inputs.is_workflow_dispatch

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.

This disables post-submit testing for all gfx, including 90a and 950. Is this intentional? PR description seems to target gfx125x behaviour only.

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.

ah intended! for gfx950, we lack enough machines, so we only want to run on submodule bumps.

for gfx90a due to outdated driver version, but once that is resolved, we will want to enable for postsubmit. this is a temporary fix, until I get #8322 in which makes all these if and conditionals go away


def test_trigger_test_label_only_push_always_runs_tests(self):
"""Push (postsubmit) always runs tests regardless of trigger_test_label_only."""
def test_trigger_test_label_only_push_without_label_skips_tests(self):

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.

Push events wouldn't have PR labels, so this appears to be an invalid test case.

Push events don't have PR labels, so testing that push without labels
skips tests is tautological. The behavior is already covered by the
push-with-label test which verifies external callers can pass labels.

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

@jayhawk-commits jayhawk-commits 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.

nit: Please add note in commit message and PR description that behaviour for gfx950 and gfx90a is changing and intentional.

@geomin12
geomin12 merged commit 9ecb548 into main Sep 22, 2026
185 of 191 checks passed
@geomin12
geomin12 deleted the users/geomin12/fix-trigger-test-label-only-postsubmit branch September 22, 2026 17:28
@github-project-automation github-project-automation Bot moved this from TODO to Done in TheRock Triage Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants