Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions build_tools/github_actions/amdgpu_family_matrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ def select_build_runner(platform: str, build_variant: str) -> str:
"family": "gfx90a",
"fetch-gfx-targets": ["gfx90a"],
"build_variants": ["release"],
# Only run tests when gfx90a label is present on PR
"trigger_test_label_only": True,
},
"windows": {
"test-runs-on": "",
Expand Down
4 changes: 2 additions & 2 deletions build_tools/github_actions/configure_multi_arch_ci.py
Original file line number Diff line number Diff line change
Expand Up @@ -1366,10 +1366,10 @@ def _expand_build_config_for_platform(
# label (e.g., gfx950-dcgpu, gfx125X-dcgpu) is present on the PR.
# This allows families with limited hardware to have tests opt-in via
# PR labels rather than always running. Builds always run regardless.
# workflow_dispatch bypasses this check to allow manual test triggering.
# push and workflow_dispatch bypass this check (postsubmit always runs tests).
if (
platform_info.get("trigger_test_label_only", False)
and not ci_inputs.is_workflow_dispatch
and ci_inputs.is_pull_request
):
family_label = platform_info["family"]
if family_label not in ci_inputs.pr_labels:
Expand Down
48 changes: 39 additions & 9 deletions build_tools/github_actions/tests/configure_multi_arch_ci_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ def test_release_produces_configs_for_both_platforms(self):
"test-runs-on",
"sanity_check_only_for_family",
}
optional_keys = {"test-runs-on-labels"}
optional_keys = {"test-runs-on-labels", "test_type"}
for config in [result.linux, result.windows]:
self.assertIsNotNone(config)
per_family = config.per_family_info
Expand Down Expand Up @@ -1978,8 +1978,8 @@ def _find_family_info(self, outputs, family_name):
return family_info
return None

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?

test_cases = [
{
"name": "push",
Expand Down Expand Up @@ -2048,22 +2048,52 @@ def test_trigger_test_label_only_behavior(self):
"extra_inputs": {"linux_amdgpu_families": ["gfx125x"]},
"expect_tests": False,
},
# gfx950 tests
# gfx90a tests (postsubmit family with trigger_test_label_only)
# ci:run-all-archs includes gfx90a in PRs, but tests need gfx90a label
{
"name": "gfx950_push_with_label_enabled",
"name": "gfx90a_pr_with_label_enabled",
"family_key": "gfx90a",
"family_name": "gfx90a",
"event_name": "pull_request",
"pr_labels": ["ci:run-all-archs", "gfx90a"],
"extra_inputs": {},
"expect_tests": True,
},
{
"name": "gfx90a_pr_without_label_disabled",
"family_key": "gfx90a",
"family_name": "gfx90a",
"event_name": "pull_request",
"pr_labels": ["ci:run-all-archs"],
"extra_inputs": {},
"expect_tests": False,
},
# gfx950 push tests - push (postsubmit) always runs tests regardless of label
{
"name": "gfx950_push_always_runs_tests",
"family_key": "gfx950",
"family_name": "gfx950-dcgpu",
"event_name": "push",
"pr_labels": ["gfx950-dcgpu"],
"pr_labels": [],
"extra_inputs": {},
"expect_tests": True,
},
# gfx950 PR tests - requires label (ci:run-all-archs includes it in PRs)
{
"name": "gfx950_push_without_label_disabled",
"name": "gfx950_pr_with_label_enabled",
"family_key": "gfx950",
"family_name": "gfx950-dcgpu",
"event_name": "push",
"pr_labels": [],
"event_name": "pull_request",
"pr_labels": ["ci:run-all-archs", "gfx950-dcgpu"],
"extra_inputs": {},
"expect_tests": True,
},
{
"name": "gfx950_pr_without_label_disabled",
"family_key": "gfx950",
"family_name": "gfx950-dcgpu",
"event_name": "pull_request",
"pr_labels": ["ci:run-all-archs"],
"extra_inputs": {},
"expect_tests": False,
},
Expand Down
Loading