diff --git a/build_tools/github_actions/tests/fetch_package_targets_test.py b/build_tools/github_actions/tests/fetch_package_targets_test.py index b0acb831fd8..7d4f2434fd6 100644 --- a/build_tools/github_actions/tests/fetch_package_targets_test.py +++ b/build_tools/github_actions/tests/fetch_package_targets_test.py @@ -6,8 +6,71 @@ sys.path.insert(0, os.fspath(Path(__file__).parent.parent)) import fetch_package_targets +from workflow_utils import WORKFLOWS_DIR, get_choice_options, load_workflow + class FetchPackageTargetsTest(unittest.TestCase): + def test_amdgpu_families_accepted_by_choice_workflows(self): + """Checks workflow choice lists against fetch_package_targets outputs. + + fetch_package_targets.py produces amdgpu_family values that get + dispatched to workflows. Those workflow inputs can either use + "type: choice" or "type: string". If they use "type: choice", this tests + that the choices are valid: + + 1. Check that every amdgpu_family from the matrix is in every workflow's + choice list. If a produced value isn't in the target's choice list, + GitHub rejects the dispatch. + See https://github.com/ROCm/TheRock/issues/3634. + 2. Check that the workflow choice list does not contain any extra + families. While not an error, extra families may indicate an + incomplete migration. + """ + # Collect all amdgpu_family values the matrix can produce. + produced_families = set() + for platform in ("linux", "windows"): + targets = fetch_package_targets.determine_package_targets( + {"AMDGPU_FAMILIES": None, "THEROCK_PACKAGE_PLATFORM": platform} + ) + for t in targets: + produced_families.add(t["amdgpu_family"]) + + self.assertGreater(len(produced_families), 0) + + # Find all workflows with amdgpu_family as type: choice and check. + choice_workflows = {} + for workflow_path in sorted(WORKFLOWS_DIR.glob("*.yml")): + workflow = load_workflow(workflow_path) + options = get_choice_options(workflow, "amdgpu_family") + if options is not None: + choice_workflows[workflow_path.name] = options + + self.assertGreater( + len(choice_workflows), + 0, + "No workflows found with amdgpu_family as type: choice - " + "was the input renamed?", + ) + + errors = [] + for workflow_name, options in choice_workflows.items(): + # Test for missing families + missing_families = produced_families - set(options) + if missing_families: + errors.append( + f"{workflow_name} is missing amdgpu_family options " + f"that fetch_package_targets can produce: {sorted(missing_families)}" + ) + # Test for extra families + extra_families = set(options) - produced_families + if extra_families: + errors.append( + f"{workflow_name} has extra amdgpu_family options not listed in fetch_package_targets: {sorted(extra_families)} " + ) + + if errors: + self.fail("\n".join(errors)) + def test_linux_single_family(self): args = { "AMDGPU_FAMILIES": "gfx94x", diff --git a/build_tools/github_actions/tests/workflow_dispatch_inputs_test.py b/build_tools/github_actions/tests/workflow_dispatch_inputs_test.py index 0dfc17f9599..feedcd7830d 100644 --- a/build_tools/github_actions/tests/workflow_dispatch_inputs_test.py +++ b/build_tools/github_actions/tests/workflow_dispatch_inputs_test.py @@ -10,24 +10,7 @@ add an extra layer of validation. This file creates test cases for each file in .github/workflows/ that uses -benc-uk/workflow-dispatch. It is run like a standard unit test, e.g.: - -``` -python ./build_tools/github_actions/tests/workflow_dispatch_inputs_test.py --v -test_no_unexpected_inputs__release_portable_linux_packages (__main__.WorkflowDispatchInputsTest.test_no_unexpected_inputs__release_portable_linux_packages) -No unexpected dispatch inputs in release_portable_linux_packages.yml ... ok -test_no_unexpected_inputs__release_windows_packages (__main__.WorkflowDispatchInputsTest.test_no_unexpected_inputs__release_windows_packages) -No unexpected dispatch inputs in release_windows_packages.yml ... ok -test_required_inputs_passed__release_portable_linux_packages (__main__.WorkflowDispatchInputsTest.test_required_inputs_passed__release_portable_linux_packages) -All required dispatch inputs passed in release_portable_linux_packages.yml ... ok -test_required_inputs_passed__release_windows_packages (__main__.WorkflowDispatchInputsTest.test_required_inputs_passed__release_windows_packages) -All required dispatch inputs passed in release_windows_packages.yml ... ok - ----------------------------------------------------------------------- -Ran 4 tests in 0.087s - -OK -``` +benc-uk/workflow-dispatch. It is run like a standard unit test. """ from dataclasses import dataclass @@ -35,78 +18,18 @@ from pathlib import Path import unittest -import yaml - -WORKFLOWS_DIR = Path(__file__).resolve().parents[3] / ".github" / "workflows" +from workflow_utils import ( + WORKFLOWS_DIR, + get_choice_options, + get_required_workflow_dispatch_inputs, + get_workflow_dispatch_inputs, + load_workflow, +) WORKFLOW_DISPATCH_ACTION_NAME = "benc-uk/workflow-dispatch" -def load_workflow(path: Path) -> dict: - """Loads a YAML workflow file from the given Path as a JSON dictionary.""" - with open(path) as f: - return yaml.safe_load(f) - - -def get_workflow_dispatch_inputs(workflow: dict) -> set: - """Extracts input names from a workflow's on.workflow_dispatch.inputs section. - - For a workflow with: - on: - workflow_dispatch: - inputs: - amdgpu_family: ... - release_type: ... - - Returns: {"amdgpu_family", "release_type"} - """ - # PyYAML parses the unquoted YAML key `on:` as boolean True. - on_block = workflow.get("on") or workflow.get(True) - if not isinstance(on_block, dict): - return set() - dispatch = on_block.get("workflow_dispatch") - if not isinstance(dispatch, dict): - return set() - inputs = dispatch.get("inputs") - if not isinstance(inputs, dict): - return set() - return set(inputs.keys()) - - -def get_required_workflow_dispatch_inputs(workflow: dict) -> set: - """Extracts required input names (no default) from workflow_dispatch. - - For a workflow with: - on: - workflow_dispatch: - inputs: - amdgpu_family: - required: true - release_type: - required: true - default: dev - - Returns: {"amdgpu_family"} (release_type has a default) - """ - # PyYAML parses the unquoted YAML key `on:` as boolean True. - on_block = workflow.get("on") or workflow.get(True) - if not isinstance(on_block, dict): - return set() - dispatch = on_block.get("workflow_dispatch") - if not isinstance(dispatch, dict): - return set() - inputs_def = dispatch.get("inputs") - if not isinstance(inputs_def, dict): - return set() - required = set() - for name, props in inputs_def.items(): - if isinstance(props, dict): - if props.get("required", False) and "default" not in props: - required.add(name) - return required - - -def parse_dispatch_inputs_json(inputs_raw: str) -> set: +def parse_dispatch_inputs_json(inputs_raw: str) -> dict: """Parses the JSON inputs string from a benc-uk/workflow-dispatch step. For an action step with: @@ -114,18 +37,18 @@ def parse_dispatch_inputs_json(inputs_raw: str) -> set: with: inputs: | { "amdgpu_family": "${{ matrix.amdgpu_family }}", - "release_type": "${{ env.RELEASE_TYPE }}" } + "release_type": "dev" } - Parses the inputs value and returns: {"amdgpu_family", "release_type"} + Returns: {"amdgpu_family": "${{ matrix.amdgpu_family }}", "release_type": "dev"} """ if not inputs_raw: - return set() + return {} parsed = json.loads(inputs_raw) if isinstance(parsed, dict): - return set(parsed.keys()) + return parsed - return set() + return {} @dataclass @@ -134,7 +57,7 @@ class DispatchCall: step_name: str target_workflow: str - passed_inputs: set + passed_inputs: dict def find_dispatch_calls_in_workflow(workflow: dict) -> list[DispatchCall]: @@ -192,7 +115,7 @@ def test_method(self): target_workflow = load_workflow(target_path) accepted_inputs = get_workflow_dispatch_inputs(target_workflow) - unexpected = call.passed_inputs - accepted_inputs + unexpected = call.passed_inputs.keys() - accepted_inputs if unexpected: errors.append( f"step '{call.step_name}' passes unexpected inputs to " @@ -220,7 +143,7 @@ def test_method(self): target_workflow = load_workflow(target_path) required_inputs = get_required_workflow_dispatch_inputs(target_workflow) - missing = required_inputs - call.passed_inputs + missing = required_inputs - call.passed_inputs.keys() if missing: errors.append( f"step '{call.step_name}' does not pass required inputs to " @@ -233,6 +156,49 @@ def test_method(self): return test_method +def _is_expression(value: str) -> bool: + """Returns True if the value contains a GitHub Actions expression.""" + return "${{" in str(value) + + +def _make_literal_choice_values_test(workflow_path: Path): + """Creates a test that checks literal values are valid for choice inputs. + + When a dispatch step passes a literal string (not a ${{ }} expression) to a + target input that is type: choice, the literal must be in the target's + allowed options list. GitHub rejects invalid choice values at dispatch time. + """ + + def test_method(self): + workflow = load_workflow(workflow_path) + calls = find_dispatch_calls_in_workflow(workflow) + errors = [] + for call in calls: + target_path = WORKFLOWS_DIR / call.target_workflow + if not target_path.exists(): + continue + + target_workflow = load_workflow(target_path) + for input_name, value in call.passed_inputs.items(): + if _is_expression(value): + continue + options = get_choice_options(target_workflow, input_name) + if options is None: + continue + if value not in options: + errors.append( + f"step '{call.step_name}' passes literal " + f"'{value}' for '{input_name}' to " + f"'{call.target_workflow}', but allowed options are: " + f"{options}" + ) + + if errors: + self.fail("\n".join(errors)) + + return test_method + + def _workflow_name_to_test_suffix(workflow_path: Path) -> str: """Converts a workflow filename to a valid Python identifier suffix.""" return workflow_path.stem.replace("-", "_").replace(".", "_") @@ -256,6 +222,16 @@ def _workflow_name_to_test_suffix(workflow_path: Path) -> str: WorkflowDispatchInputsTest, f"test_required_inputs_passed__{_suffix}", _test ) + _test = _make_literal_choice_values_test(_workflow_path) + _test.__doc__ = ( + f"Literal dispatch values are valid for choice inputs in {_workflow_path.name}" + ) + setattr( + WorkflowDispatchInputsTest, + f"test_literal_values_valid_for_choices__{_suffix}", + _test, + ) + if __name__ == "__main__": unittest.main() diff --git a/build_tools/github_actions/tests/workflow_utils.py b/build_tools/github_actions/tests/workflow_utils.py new file mode 100644 index 00000000000..d4e79d27eb0 --- /dev/null +++ b/build_tools/github_actions/tests/workflow_utils.py @@ -0,0 +1,103 @@ +"""Shared helpers for workflow YAML tests.""" + +from pathlib import Path + +import yaml + +WORKFLOWS_DIR = Path(__file__).resolve().parents[3] / ".github" / "workflows" + + +def load_workflow(path: Path) -> dict: + """Loads a YAML workflow file from the given Path as a JSON dictionary.""" + with open(path) as f: + return yaml.safe_load(f) + + +def _get_workflow_dispatch_block(workflow: dict) -> dict | None: + """Returns the workflow_dispatch block, or None.""" + # PyYAML parses the unquoted YAML key `on:` as boolean True. + on_block = workflow.get("on") or workflow.get(True) + if not isinstance(on_block, dict): + return None + dispatch = on_block.get("workflow_dispatch") + if not isinstance(dispatch, dict): + return None + return dispatch + + +def _get_dispatch_inputs(workflow: dict) -> dict: + """Returns the workflow_dispatch inputs dict, or empty dict.""" + dispatch = _get_workflow_dispatch_block(workflow) + if dispatch is None: + return {} + inputs = dispatch.get("inputs") + if not isinstance(inputs, dict): + return {} + return inputs + + +def get_workflow_dispatch_inputs(workflow: dict) -> set: + """Extracts input names from a workflow's on.workflow_dispatch.inputs section. + + For a workflow with: + on: + workflow_dispatch: + inputs: + amdgpu_family: ... + release_type: ... + + Returns: {"amdgpu_family", "release_type"} + """ + return set(_get_dispatch_inputs(workflow).keys()) + + +def get_required_workflow_dispatch_inputs(workflow: dict) -> set: + """Extracts required input names (no default) from workflow_dispatch. + + For a workflow with: + on: + workflow_dispatch: + inputs: + amdgpu_family: + required: true + release_type: + required: true + default: dev + + Returns: {"amdgpu_family"} (release_type has a default) + """ + required = set() + for name, props in _get_dispatch_inputs(workflow).items(): + if isinstance(props, dict): + if props.get("required", False) and "default" not in props: + required.add(name) + return required + + +def get_choice_options(workflow: dict, input_name: str) -> list | None: + """Extracts the options list for a type: choice workflow_dispatch input. + + For a workflow with: + on: + workflow_dispatch: + inputs: + amdgpu_family: + type: choice + options: + - gfx94X-dcgpu + - gfx110X-all + + get_choice_options(workflow, "amdgpu_family") returns: + ["gfx94X-dcgpu", "gfx110X-all"] + + Returns None if the input doesn't exist or isn't type: choice. + """ + input_def = _get_dispatch_inputs(workflow).get(input_name) + if not isinstance(input_def, dict): + return None + if input_def.get("type") != "choice": + return None + options = input_def.get("options") + if not isinstance(options, list): + return None + return options