Repository navigation
feat(ci): scope multi-arch nightly tests to rocm-libraries subtrees #14048
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
252e624
ci: scope multi-arch nightly tests to rocm-libraries subtrees
geomin12 2204f2c
refactor(ci): extract nightly test scope into a proper script
geomin12 45d9181
docs(ci): flag nightly test scope script as a temporary workaround
geomin12 21f8265
refactor(ci): condense nightly test scope script and drop setup-python
geomin12 21ff310
ci: restore setup-python for the nightly test scope job
geomin12 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import json | ||
| import os | ||
| import sys | ||
| import tempfile | ||
| import unittest | ||
| from pathlib import Path | ||
| from typing import List | ||
|
|
||
| sys.path.insert(0, os.fspath(Path(__file__).parent.parent)) | ||
| import therock_nightly_test_scope as scope | ||
|
|
||
|
|
||
| def make_entry(name: str, category: str) -> dict: | ||
| return { | ||
| "name": name, | ||
| "url": f"ROCm/{name}", | ||
| "branch": "develop", | ||
| "category": category, | ||
| "auto_subtree_pull": False, | ||
| "auto_subtree_push": True, | ||
| "monorepo_source_of_truth": True, | ||
| } | ||
|
|
||
|
|
||
| def write_config(tmpdir: str, entries: List[dict]) -> Path: | ||
| config_path = Path(tmpdir) / "repos-config.json" | ||
| config_path.write_text(json.dumps({"repositories": entries}), encoding="utf-8") | ||
| return config_path | ||
|
|
||
|
|
||
| class NightlyTestScopeTest(unittest.TestCase): | ||
| def test_returns_sorted_category_name_pairs(self): | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| config_path = write_config( | ||
| tmpdir, | ||
| [ | ||
| make_entry("rocblas", "projects"), | ||
| make_entry("rocsparse", "projects"), | ||
| make_entry("hipblas-common", "shared"), | ||
| ], | ||
| ) | ||
| self.assertEqual( | ||
| scope.get_test_projects(config_path), | ||
| [ | ||
| "projects/rocblas", | ||
| "projects/rocsparse", | ||
| "shared/hipblas-common", | ||
| ], | ||
| ) | ||
|
|
||
| def test_includes_entries_regardless_of_subtree_sync_flags(self): | ||
| # The nightly scope is "everything this repo owns", so entries are not | ||
| # filtered by the auto_subtree_pull/push flags that the sync tooling uses. | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| entry = make_entry("rocprim", "projects") | ||
| entry["auto_subtree_push"] = False | ||
| entry["monorepo_source_of_truth"] = False | ||
| config_path = write_config(tmpdir, [entry]) | ||
| self.assertEqual(scope.get_test_projects(config_path), ["projects/rocprim"]) | ||
|
|
||
| def test_empty_config_fails_instead_of_testing_everything(self): | ||
| # An empty changed_projects means "test everything" to TheRock. | ||
| with tempfile.TemporaryDirectory() as tmpdir: | ||
| config_path = write_config(tmpdir, []) | ||
| with self.assertRaises(SystemExit): | ||
| scope.main(["--config", os.fspath(config_path)]) | ||
|
|
||
| def test_real_config_is_non_empty(self): | ||
| projects = scope.get_test_projects(scope.DEFAULT_CONFIG_PATH) | ||
| self.assertTrue(projects) | ||
| for project in projects: | ||
| self.assertIn("/", project) | ||
|
|
||
| def test_step_summary_lists_each_subtree_on_its_own_line(self): | ||
| summary = scope.format_step_summary(["projects/rocblas", "shared/rocroller"]) | ||
| self.assertIn("### Nightly test scope", summary) | ||
| self.assertIn("projects/rocblas\nshared/rocroller", summary) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| #!/usr/bin/env python3 | ||
|
|
||
| """ | ||
| TheRock Nightly Test Scope | ||
| -------------------------- | ||
| Computes the set of projects the multi-arch nightly CI workflow should test. | ||
|
|
||
| A scheduled run has no diff to narrow testing against, so TheRock would | ||
| otherwise test every component it builds, including ROCm system components | ||
| (hip-tests, rocrtst, rocgdb, rocprofiler-*, amdsmi, rccl, ...) that no change | ||
| in this repository can affect. This script emits every subtree registered in | ||
| .github/repos-config.json in `category/name` form, which the workflow passes to | ||
| TheRock as `changed_projects` so testing stays scoped to the code this | ||
| repository owns (plus the consumers TheRock's dependency graph pulls in). | ||
|
|
||
| Arguments: | ||
| --config : OPTIONAL, path to the repos-config.json file. | ||
|
|
||
| Outputs: | ||
| Writes 'test_projects' to the GitHub Actions $GITHUB_OUTPUT file as a | ||
| comma-separated list of subtrees in `category/name` format, and appends a | ||
| human-readable scope listing to $GITHUB_STEP_SUMMARY. Both are printed to | ||
| stdout when run outside of GitHub Actions. | ||
|
|
||
| Example Usage: | ||
| python therock_nightly_test_scope.py | ||
| """ | ||
|
|
||
| import argparse | ||
| import logging | ||
| from pathlib import Path | ||
| from typing import List, Optional | ||
|
|
||
| from ci_utils import append_step_summary, set_github_output | ||
| from config_loader import load_repo_config | ||
| from pr_detect_changed_subtrees import get_valid_prefixes | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| SCRIPT_DIR = Path(__file__).resolve().parent | ||
| DEFAULT_CONFIG_PATH = SCRIPT_DIR.parent / "repos-config.json" | ||
|
|
||
|
|
||
| def parse_arguments(argv: Optional[List[str]] = None) -> argparse.Namespace: | ||
| """Parse command-line arguments.""" | ||
| parser = argparse.ArgumentParser( | ||
| description="Compute the nightly multi-arch CI test scope." | ||
| ) | ||
| parser.add_argument( | ||
| "--config", | ||
| type=Path, | ||
| default=DEFAULT_CONFIG_PATH, | ||
| help="Path to the repos-config.json file", | ||
| ) | ||
| return parser.parse_args(argv) | ||
|
|
||
|
|
||
| def get_test_projects(config_path: Path) -> List[str]: | ||
| """Return every subtree in repos-config.json as a sorted `category/name` list.""" | ||
| config = load_repo_config(str(config_path)) | ||
| return sorted(get_valid_prefixes(config)) | ||
|
|
||
|
|
||
| def format_step_summary(test_projects: List[str]) -> str: | ||
| """Render the test scope as a markdown step summary.""" | ||
| subtree_list = "\n".join(test_projects) | ||
| return ( | ||
| "### Nightly test scope\n" | ||
| "\n" | ||
| "Testing rocm-libraries subtrees and their direct consumers only:\n" | ||
| "\n" | ||
| f"```\n{subtree_list}\n```\n" | ||
| ) | ||
|
|
||
|
|
||
| def main(argv: Optional[List[str]] = None) -> None: | ||
| """Emit the nightly test scope for the multi-arch CI workflow.""" | ||
| logging.basicConfig(level=logging.INFO) | ||
| args = parse_arguments(argv) | ||
|
|
||
| test_projects = get_test_projects(args.config) | ||
| if not test_projects: | ||
| # TheRock reads an empty `changed_projects` as "test everything", which | ||
| # is the opposite of this job's purpose, so fail instead of emitting it. | ||
| raise SystemExit(f"No subtrees found in {args.config}") | ||
|
|
||
| set_github_output({"test_projects": ",".join(test_projects)}) | ||
| append_step_summary(format_step_summary(test_projects)) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this issue please:
We can build something into the scheduled CI that limits which builds/tests to run to those affected by specific submodules only. Consider that there are 11 submodules in https://github.com/ROCm/TheRock/blob/main/.gitmodules and each may want to run some tests (filtered following https://github.com/ROCm/TheRock/blob/main/docs/rfcs/RFC0013-Consumer-Based-Test-Selection.md). As implemented here, each of up to those 11 repositories may want to fork this 200 lines of code. We should teach the common code how to handle this. Each repository shouldn't need to maintain its own list of
repos-config.jsonfiles for the CI system to understand how to test changes in it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point! that's a better approach than this (hacky for sure)