fixup, add more logging and selftests

Author: jiridanek

ci/cached-builds/gen_gha_matrix_jobs.py

@@ -7,6 +7,7 @@
 import re
 import string
 import sys
+import unittest
 from typing import Iterable
 
 import gha_pr_changed_files
@@ -120,11 +121,13 @@ def write_github_workflow_file(tree: dict[str, list[str]], path: pathlib.Path) -
 def flatten(list_of_lists):
     return list(itertools.chain.from_iterable(list_of_lists))
 
+
 def compute_leafs_in_dependency_tree(tree: dict[str, list[str]]) -> list[str]:
     key_set = set(tree.keys())
     value_set = set(flatten(tree.values()))
     return [key for key in key_set if key not in value_set]
 
+
 def print_github_actions_pr_matrix(tree: dict[str, list[str]], leafs: list[str]) -> list[str]:
     """Outputs GitHub matrix definition Json
     """
@@ -148,10 +151,14 @@ def main() -> None:
     logging.basicConfig(level=logging.DEBUG, stream=sys.stderr)
 
     argparser = argparse.ArgumentParser()
-    argparser.add_argument("--owner", type=str, required=False, help="GitHub repo owner/org (for the --skip-unchanged feature)")
-    argparser.add_argument("--repo", type=str, required=False, help="GitHub repo name (for the --skip-unchanged feature)")
-    argparser.add_argument("--pr-number", type=int, required=False, help="PR number under owner/repo (for the --skip-unchanged feature)")
-    argparser.add_argument("--skip-unchanged", type=bool, required=False, default=False, action=argparse.BooleanOptionalAction)
+    argparser.add_argument("--owner", type=str, required=False,
+                           help="GitHub repo owner/org (for the --skip-unchanged feature)")
+    argparser.add_argument("--repo", type=str, required=False,
+                           help="GitHub repo name (for the --skip-unchanged feature)")
+    argparser.add_argument("--pr-number", type=int, required=False,
+                           help="PR number under owner/repo (for the --skip-unchanged feature)")
+    argparser.add_argument("--skip-unchanged", type=bool, required=False, default=False,
+                           action=argparse.BooleanOptionalAction)
     args = argparser.parse_args()
 
     # https://www.gnu.org/software/make/manual/make.html#Reading-Makefiles
@@ -164,7 +171,8 @@ def main() -> None:
     leafs = compute_leafs_in_dependency_tree(tree)
     if args.skip_unchanged:
         logging.info(f"Skipping targets not modified in PR #{args.pr_number}")
-        leafs = gha_pr_changed_files.filter_out_unchanged(args.owner, args.repo, args.pr_number, leafs)
+        changed_files = gha_pr_changed_files.list_changed_files(args.owner, args.repo, args.pr_number)
+        leafs = gha_pr_changed_files.filter_out_unchanged(leafs, changed_files)
     output = print_github_actions_pr_matrix(tree, leafs)
 
     print("leafs", leafs)
@@ -176,3 +184,18 @@ def main() -> None:
 
 if __name__ == '__main__':
     main()
+
+
+class SelfTests(unittest.TestCase):
+    def test_select_changed_targets(self):
+        with open(project_dir / "Makefile", "rt") as makefile:
+            lines = read_makefile_lines(makefile)
+        tree = extract_target_dependencies(lines)
+        leafs = compute_leafs_in_dependency_tree(tree)
+
+        changed_files = ["jupyter/datascience/ubi9-python-3.9/Dockerfile"]
+
+        leafs = gha_pr_changed_files.filter_out_unchanged(leafs, changed_files)
+        assert set(leafs) == {'cuda-jupyter-tensorflow-ubi9-python-3.9',
+                              'jupyter-trustyai-ubi9-python-3.9',
+                              'jupyter-pytorch-ubi9-python-3.9'}

ci/cached-builds/gha_pr_changed_files.py

@@ -16,7 +16,8 @@ def get_github_token() -> str:
 
 
 # https://docs.github.com/en/graphql/guides/forming-calls-with-graphql
-def compose_gh_api_request(pull_number: int, owner="opendatahub-io", repo="notebooks", per_page=100, cursor="") -> urllib.request.Request:
+def compose_gh_api_request(pull_number: int, owner="opendatahub-io", repo="notebooks", per_page=100,
+                           cursor="") -> urllib.request.Request:
     github_token = get_github_token()
 
     return urllib.request.Request(
@@ -52,7 +53,8 @@ def list_changed_files(owner: str, repo: str, pr_number: int, per_page=100) -> l
 
     CURSOR = ""
     while CURSOR is not None:
-        request = compose_gh_api_request(pull_number=pr_number, owner=owner, repo=repo, per_page=per_page, cursor=CURSOR)
+        request = compose_gh_api_request(pull_number=pr_number, owner=owner, repo=repo, per_page=per_page,
+                                         cursor=CURSOR)
         response = urllib.request.urlopen(request)
         data = json.loads(response.read().decode("utf-8"))
         response.close()
@@ -81,21 +83,30 @@ def analyze_build_directories(make_target) -> list[str]:
         print(e.stderr, e.stdout)
         raise
 
-    logging.debug(f"Running make in --just-print mode for target {make_target}")
+    logging.debug(f"Target {make_target} depends on files in directories {directories}")
     return directories
 
 
-def should_build_target(changed_files: list[str], target_directories: list[str]) -> bool:
+def should_build_target(changed_files: list[str], target_directories: list[str]) -> str:
+    """Returns truthy if there is at least one changed file necessitating a build.
+    Falsy (empty) string is returned otherwise."""
     for directory in target_directories:
-        if any(changed_file.startswith(directory) for changed_file in changed_files):
-            return True
-    return False
+        for changed_file in changed_files:
+            if changed_file.startswith(directory):
+                return changed_file
+    return ""
 
 
-def filter_out_unchanged(owner: str, repo: str, pr_number: int, targets: list[str]) -> list[str]:
-    changed_files = list_changed_files(owner, repo, pr_number)
-    return [target for target in targets if
-            should_build_target(changed_files, target_directories=analyze_build_directories(target))]
+def filter_out_unchanged(targets: list[str], changed_files: list[str]) -> list[str]:
+    changed = []
+    for target in targets:
+        target_directories = analyze_build_directories(target)
+        if reason := should_build_target(changed_files, target_directories):
+            logging.info(f"✅ Will build {target} because file {reason} has been changed")
+            changed.append(target)
+        else:
+            logging.info(f"❌ Won't build {target}")
+    return changed
 
 
 class SelTestsTest(unittest.TestCase):
@@ -104,12 +115,12 @@ def test_compose_gh_api_request__call_without_asserting(self):
         print(request.data)
 
     def test_list_changed_files__pagination_works(self):
-        changed_files = list_changed_files(556, per_page=1)
-        assert changed_files == ['codeserver/ubi9-python-3.9/Dockerfile',
-                                 'codeserver/ubi9-python-3.9/run-code-server.sh']
+        changed_files = list_changed_files(owner="opendatahub-io", repo="notebooks", pr_number=556, per_page=1)
+        assert set(changed_files) == {'codeserver/ubi9-python-3.9/Dockerfile',
+                                      'codeserver/ubi9-python-3.9/run-code-server.sh'}
 
     def test_analyze_build_directories(self):
         directories = analyze_build_directories("jupyter-intel-pytorch-ubi9-python-3.9")
-        assert directories == ["base/ubi9-python-3.9",
-                               "intel/base/gpu/ubi9-python-3.9",
-                               "jupyter/intel/pytorch/ubi9-python-3.9"]
+        assert set(directories) == {"base/ubi9-python-3.9",
+                                    "intel/base/gpu/ubi9-python-3.9",
+                                    "jupyter/intel/pytorch/ubi9-python-3.9"}