From b86366df47ce1f5575dc0c6b298e7e1ed4c0f61a Mon Sep 17 00:00:00 2001 From: geomin12 Date: Thu, 17 Sep 2026 10:18:50 -0700 Subject: [PATCH 1/2] fix(ci): skip gfx90a tests on PRs unless explicitly labeled 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 --- .../github_actions/amdgpu_family_matrix.py | 2 + .../github_actions/configure_multi_arch_ci.py | 4 +- .../tests/configure_multi_arch_ci_test.py | 48 +++++++++++++++---- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/build_tools/github_actions/amdgpu_family_matrix.py b/build_tools/github_actions/amdgpu_family_matrix.py index b5560e074e1..9474ab50874 100644 --- a/build_tools/github_actions/amdgpu_family_matrix.py +++ b/build_tools/github_actions/amdgpu_family_matrix.py @@ -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": "", diff --git a/build_tools/github_actions/configure_multi_arch_ci.py b/build_tools/github_actions/configure_multi_arch_ci.py index 6f3656dc042..4fd99e2f2a5 100755 --- a/build_tools/github_actions/configure_multi_arch_ci.py +++ b/build_tools/github_actions/configure_multi_arch_ci.py @@ -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: diff --git a/build_tools/github_actions/tests/configure_multi_arch_ci_test.py b/build_tools/github_actions/tests/configure_multi_arch_ci_test.py index 5580fb337da..226a5a1ba12 100644 --- a/build_tools/github_actions/tests/configure_multi_arch_ci_test.py +++ b/build_tools/github_actions/tests/configure_multi_arch_ci_test.py @@ -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 @@ -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.""" test_cases = [ { "name": "push", @@ -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, }, From f0103d55b9945cfc32e1262fa0bb9538cb416df3 Mon Sep 17 00:00:00 2001 From: geomin12 Date: Thu, 17 Sep 2026 14:24:41 -0700 Subject: [PATCH 2/2] refactor(ci): use mock family matrix in TestFamilyTestFilters 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 --- .../tests/configure_multi_arch_ci_test.py | 344 ++++++++---------- 1 file changed, 149 insertions(+), 195 deletions(-) diff --git a/build_tools/github_actions/tests/configure_multi_arch_ci_test.py b/build_tools/github_actions/tests/configure_multi_arch_ci_test.py index 226a5a1ba12..464c421f4db 100644 --- a/build_tools/github_actions/tests/configure_multi_arch_ci_test.py +++ b/build_tools/github_actions/tests/configure_multi_arch_ci_test.py @@ -1968,7 +1968,54 @@ def test_windows_workflow_uses_all_ci_fields(self): class TestFamilyTestFilters(unittest.TestCase): - """Tests for family-level test filtering (trigger_test_label_only, test_type_for_family).""" + """Tests for family-level test filtering using mock family matrices. + + These tests use synthetic family configurations to verify the behavior of + trigger_test_label_only, test_type_for_family, and other family-level flags. + This decouples tests from production CI configuration, avoiding "change detector" + tests that break whenever the real family matrix changes. + """ + + # Mock family matrix for testing trigger_test_label_only behavior. + # Family keys use "mock-" prefix to avoid conflict with label parsing + # (labels starting with "gfx" get special handling in select_targets). + MOCK_FAMILIES_TRIGGER_TEST_LABEL = { + # Presubmit family - always runs on PRs + "mock-presubmit": { + "linux": { + "test-runs-on": "linux-presubmit-runner", + "family": "mock-presubmit", + "fetch-gfx-targets": ["gfx0000"], + "build_variants": ["release"], + }, + }, + # Postsubmit family with trigger_test_label_only - tests require label on PRs + "mock-postsubmit-labeled": { + "linux": { + "test-runs-on": "linux-postsubmit-runner", + "family": "mock-postsubmit-labeled", + "fetch-gfx-targets": ["gfx0001"], + "build_variants": ["release"], + "trigger_test_label_only": True, + }, + }, + # Family with test_type_for_family override + "mock-quick-only": { + "linux": { + "test-runs-on": "linux-quick-runner", + "family": "mock-quick-only", + "fetch-gfx-targets": ["gfx0002"], + "build_variants": ["release"], + "trigger_test_label_only": True, + "test_type_for_family": "quick", + }, + }, + } + + def _mock_get_all_families(self, trigger_types): + """Return mock families based on trigger types.""" + # For simplicity, return all mock families for any trigger type + return self.MOCK_FAMILIES_TRIGGER_TEST_LABEL def _find_family_info(self, outputs, family_name): """Helper to find family info in build outputs.""" @@ -1978,208 +2025,115 @@ def _find_family_info(self, outputs, family_name): return family_info return None - def test_gfx90a_runs_tests_on_push_and_workflow_dispatch(self): - """gfx90a (trigger_test_label_only) runs tests on push and workflow_dispatch.""" - test_cases = [ - { - "name": "push", - "event_name": "push", - "base_ref": "HEAD^", - "extra_inputs": {}, - "git_context": cm.GitContext( - changed_files=["CMakeLists.txt"], - submodule_paths=["rocm-systems"], - ), - }, - { - "name": "push_with_submodule_changes", - "event_name": "push", - "base_ref": "HEAD^", - "extra_inputs": {}, - "git_context": cm.GitContext( - changed_files=["some-submodule"], - submodule_paths=["some-submodule"], - ), - }, - { - "name": "workflow_dispatch", - "event_name": "workflow_dispatch", - "base_ref": "HEAD^", - "extra_inputs": {"linux_amdgpu_families": ["gfx90a"]}, - "git_context": cm.GitContext.empty(), - }, - ] + def test_trigger_test_label_only_pr_with_label_runs_tests(self): + """PR with family label runs tests when trigger_test_label_only is set.""" + with patch( + "configure_multi_arch_ci.get_all_families_for_trigger_types", + side_effect=self._mock_get_all_families, + ): + ci_inputs = cm.CIInputs( + run_id="12345", + event_name="pull_request", + commit_ref="feature", + base_ref="main", + build_variant="release", + pr_labels=["mock-postsubmit-labeled"], + linux_amdgpu_families=["mock-postsubmit-labeled"], + ) + outputs = cm.configure(ci_inputs, cm.GitContext.empty()) + family_info = self._find_family_info(outputs, "mock-postsubmit-labeled") - for tc in test_cases: - with self.subTest(tc["name"]): - ci_inputs = cm.CIInputs( - run_id="12345", - event_name=tc["event_name"], - commit_ref="main", - base_ref=tc["base_ref"], - build_variant="release", - **tc["extra_inputs"], - ) - outputs = cm.configure(ci_inputs, tc["git_context"]) - gfx90a_info = self._find_family_info(outputs, "gfx90a") + self.assertIsNotNone(family_info) + self.assertNotEqual(family_info["test-runs-on"], "") - self.assertIsNotNone(gfx90a_info) - self.assertNotEqual(gfx90a_info["test-runs-on"], "") + def test_trigger_test_label_only_pr_without_label_skips_tests(self): + """PR without family label skips tests when trigger_test_label_only is set.""" + with patch( + "configure_multi_arch_ci.get_all_families_for_trigger_types", + side_effect=self._mock_get_all_families, + ): + ci_inputs = cm.CIInputs( + run_id="12345", + event_name="pull_request", + commit_ref="feature", + base_ref="main", + build_variant="release", + pr_labels=[], # No family label + linux_amdgpu_families=["mock-postsubmit-labeled"], + ) + outputs = cm.configure(ci_inputs, cm.GitContext.empty()) + family_info = self._find_family_info(outputs, "mock-postsubmit-labeled") - def test_trigger_test_label_only_behavior(self): - """trigger_test_label_only gates tests based on label presence.""" - test_cases = [ - # gfx125x tests - { - "name": "gfx125x_pr_with_label_enabled", - "family_key": "gfx125x", - "family_name": "gfx125X-dcgpu", - "event_name": "pull_request", - "pr_labels": ["gfx125X-dcgpu"], - "extra_inputs": {"linux_amdgpu_families": ["gfx125x"]}, - "expect_tests": True, - }, - { - "name": "gfx125x_pr_without_label_disabled", - "family_key": "gfx125x", - "family_name": "gfx125X-dcgpu", - "event_name": "pull_request", - "pr_labels": [], - "extra_inputs": {"linux_amdgpu_families": ["gfx125x"]}, - "expect_tests": False, - }, - # gfx90a tests (postsubmit family with trigger_test_label_only) - # ci:run-all-archs includes gfx90a in PRs, but tests need gfx90a label - { - "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": [], - "extra_inputs": {}, - "expect_tests": True, - }, - # gfx950 PR tests - requires label (ci:run-all-archs includes it in PRs) - { - "name": "gfx950_pr_with_label_enabled", - "family_key": "gfx950", - "family_name": "gfx950-dcgpu", - "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, - }, - # workflow_dispatch bypasses label requirement - { - "name": "gfx950_workflow_dispatch_bypasses_label", - "family_key": "gfx950", - "family_name": "gfx950-dcgpu", - "event_name": "workflow_dispatch", - "pr_labels": [], - "extra_inputs": {"linux_amdgpu_families": ["gfx950"]}, - "expect_tests": True, - }, - { - "name": "gfx125x_workflow_dispatch_bypasses_label", - "family_key": "gfx125x", - "family_name": "gfx125X-dcgpu", - "event_name": "workflow_dispatch", - "pr_labels": [], - "extra_inputs": {"linux_amdgpu_families": ["gfx125x"]}, - "expect_tests": True, - }, - ] + self.assertIsNotNone(family_info) + self.assertEqual(family_info["test-runs-on"], "") - for tc in test_cases: - with self.subTest(tc["name"]): - base_ref = None if tc["event_name"] == "workflow_dispatch" else "main" - if tc["event_name"] == "push": - base_ref = "HEAD^" + def test_trigger_test_label_only_push_always_runs_tests(self): + """Push (postsubmit) always runs tests regardless of trigger_test_label_only.""" + with patch( + "configure_multi_arch_ci.get_all_families_for_trigger_types", + side_effect=self._mock_get_all_families, + ): + ci_inputs = cm.CIInputs( + run_id="12345", + event_name="push", + commit_ref="main", + base_ref="HEAD^", + build_variant="release", + ) + outputs = cm.configure(ci_inputs, cm.GitContext.empty()) + family_info = self._find_family_info(outputs, "mock-postsubmit-labeled") - ci_inputs = cm.CIInputs( - run_id="12345", - event_name=tc["event_name"], - commit_ref=( - "main" if tc["event_name"] != "pull_request" else "feature" - ), - base_ref=base_ref, - build_variant="release", - pr_labels=tc["pr_labels"], - **tc["extra_inputs"], - ) - outputs = cm.configure(ci_inputs, cm.GitContext.empty()) - family_info = self._find_family_info(outputs, tc["family_name"]) + self.assertIsNotNone(family_info) + self.assertNotEqual(family_info["test-runs-on"], "") - self.assertIsNotNone( - family_info, f"Family {tc['family_name']} not found" - ) - if tc["expect_tests"]: - self.assertNotEqual( - family_info["test-runs-on"], - "", - f"Expected tests enabled for {tc['name']}", - ) - else: - self.assertEqual( - family_info["test-runs-on"], - "", - f"Expected tests disabled for {tc['name']}", - ) + def test_trigger_test_label_only_workflow_dispatch_always_runs_tests(self): + """workflow_dispatch always runs tests regardless of trigger_test_label_only.""" + with patch( + "configure_multi_arch_ci.get_all_families_for_trigger_types", + side_effect=self._mock_get_all_families, + ): + ci_inputs = cm.CIInputs( + run_id="12345", + event_name="workflow_dispatch", + commit_ref="main", + base_ref=None, + build_variant="release", + linux_amdgpu_families=["mock-postsubmit-labeled"], + ) + outputs = cm.configure(ci_inputs, cm.GitContext.empty()) + family_info = self._find_family_info(outputs, "mock-postsubmit-labeled") + + self.assertIsNotNone(family_info) + self.assertNotEqual(family_info["test-runs-on"], "") def test_test_type_for_family_forces_test_type(self): - """test_type_for_family forces the test type for a family.""" - # gfx125x has test_type_for_family=["quick"] and trigger_test_label_only - # With label present, tests should run with forced test_type="quick" - ci_inputs = cm.CIInputs( - run_id="12345", - event_name="pull_request", - commit_ref="feature", - base_ref="main", - build_variant="release", - pr_labels=["gfx125X-dcgpu"], - linux_amdgpu_families=["gfx125x"], - ) - # Submodule changes would normally trigger test_type="full" - git_context = cm.GitContext( - changed_files=["some-submodule"], - submodule_paths=["some-submodule"], - ) - outputs = cm.configure(ci_inputs, git_context) - gfx125x_info = self._find_family_info(outputs, "gfx125X-dcgpu") - - self.assertIsNotNone(gfx125x_info) - # Tests should be enabled (label present) - self.assertNotEqual(gfx125x_info["test-runs-on"], "") - # test_type should be forced to "quick" despite global being "full" - self.assertEqual(gfx125x_info.get("test_type"), "quick") + """test_type_for_family overrides global test_type for the family.""" + with patch( + "configure_multi_arch_ci.get_all_families_for_trigger_types", + side_effect=self._mock_get_all_families, + ): + ci_inputs = cm.CIInputs( + run_id="12345", + event_name="pull_request", + commit_ref="feature", + base_ref="main", + build_variant="release", + pr_labels=["mock-quick-only"], # Label to enable tests + linux_amdgpu_families=["mock-quick-only"], + ) + # Submodule changes would normally trigger test_type="full" + git_context = cm.GitContext( + changed_files=["some-submodule"], + submodule_paths=["some-submodule"], + ) + outputs = cm.configure(ci_inputs, git_context) + family_info = self._find_family_info(outputs, "mock-quick-only") + + self.assertIsNotNone(family_info) + # Tests should be enabled (label present) + self.assertNotEqual(family_info["test-runs-on"], "") + # test_type should be forced to "quick" despite global being "full" + self.assertEqual(family_info.get("test_type"), "quick") # ---------------------------------------------------------------------------