refactor(tests/scripts): modularize _get_notebook_id for reuse and add pytest coverage for manifest file validation (#2497)

Author: jiridanek

ci/__init__.py

@@ -165,7 +165,9 @@ def gha_log_group(title):
 # https://docs.python.org/3/library/unittest.mock-examples.html#patch-decorators
 @unittest.mock.patch("time.sleep", unittest.mock.Mock())
 class TestMakeTest(unittest.TestCase):
-    @unittest.mock.patch("make_test.execute")
+    target = execute.__module__ + "." + execute.__name__
+
+    @unittest.mock.patch(target)
     def test_make_commands_jupyter(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("jupyter-minimal-ubi9-python-3.11")
@@ -174,7 +176,7 @@ def test_make_commands_jupyter(self, mock_execute: unittest.mock.Mock) -> None:
         assert "make test-jupyter-minimal-ubi9-python-3.11" in commands
         assert "make undeploy9-jupyter-minimal-ubi9-python-3.11" in commands
 
-    @unittest.mock.patch("make_test.execute")
+    @unittest.mock.patch(target)
     def test_make_commands_jupyter_rocm(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("rocm-jupyter-tensorflow-ubi9-python-3.11")
@@ -183,7 +185,7 @@ def test_make_commands_jupyter_rocm(self, mock_execute: unittest.mock.Mock) -> N
         assert "make test-jupyter-rocm-tensorflow-ubi9-python-3.11" in commands
         assert "make undeploy9-jupyter-rocm-tensorflow-ubi9-python-3.11" in commands
 
-    @unittest.mock.patch("make_test.execute")
+    @unittest.mock.patch(target)
     def test_make_commands_codeserver(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("codeserver-ubi9-python-3.11")
@@ -192,7 +194,7 @@ def test_make_commands_codeserver(self, mock_execute: unittest.mock.Mock) -> Non
         assert "make validate-codeserver-image image=codeserver-ubi9-python-3.11" in commands
         assert "make undeploy9-codeserver-ubi9-python-3.11" in commands
 
-    @unittest.mock.patch("make_test.execute")
+    @unittest.mock.patch(target)
     def test_make_commands_rstudio(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("rstudio-c9s-python-3.11")
@@ -201,7 +203,7 @@ def test_make_commands_rstudio(self, mock_execute: unittest.mock.Mock) -> None:
         assert "make validate-rstudio-image image=rstudio-c9s-python-3.11" in commands
         assert "make undeploy-c9s-rstudio-c9s-python-3.11" in commands
 
-    @unittest.mock.patch("make_test.execute")
+    @unittest.mock.patch(target)
     def test_make_commands_rsudio_rhel(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("rstudio-rhel9-python-3.11")
@@ -210,7 +212,7 @@ def test_make_commands_rsudio_rhel(self, mock_execute: unittest.mock.Mock) -> No
         assert "make validate-rstudio-image image=rstudio-rhel9-python-3.11" in commands
         assert "make undeploy-rhel9-rstudio-rhel9-python-3.11" in commands
 
-    @unittest.mock.patch("make_test.execute")
+    @unittest.mock.patch(target)
     def test_make_commands_cuda_rstudio(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("cuda-rstudio-c9s-python-3.11")
@@ -219,7 +221,7 @@ def test_make_commands_cuda_rstudio(self, mock_execute: unittest.mock.Mock) -> N
         assert "make validate-rstudio-image image=cuda-rstudio-c9s-python-3.11" in commands
         assert "make undeploy-c9s-rstudio-c9s-python-3.11" in commands
 
-    @unittest.mock.patch("make_test.execute")
+    @unittest.mock.patch(target)
     def test_make_commands_cuda_rstudio_rhel(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("cuda-rstudio-rhel9-python-3.11")
@@ -228,7 +230,7 @@ def test_make_commands_cuda_rstudio_rhel(self, mock_execute: unittest.mock.Mock)
         assert "make validate-rstudio-image image=cuda-rstudio-rhel9-python-3.11" in commands
         assert "make undeploy-rhel9-rstudio-rhel9-python-3.11" in commands
 
-    @unittest.mock.patch("make_test.execute")
+    @unittest.mock.patch(target)
     def test_make_commands_runtime(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("runtime-datascience-ubi9-python-3.11")
@@ -237,7 +239,7 @@ def test_make_commands_runtime(self, mock_execute: unittest.mock.Mock) -> None:
         assert "make validate-runtime-image image=runtime-datascience-ubi9-python-3.11" in commands
         assert "make undeploy9-runtimes-datascience-ubi9-python-3.11" in commands
 
-    @unittest.mock.patch("make_test.execute")
+    @unittest.mock.patch(target)
     def test_make_commands_rocm_runtime(self, mock_execute: unittest.mock.Mock) -> None:
         """Compares the commands with what we had in the openshift/release yaml"""
         run_tests("rocm-runtime-pytorch-ubi9-python-3.11")

ci/cached-builds/__init__.py

@@ -0,0 +1 @@
+cached-builds

ci/cached-builds/make_test.py

@@ -335,14 +335,7 @@ function _test_datascience_notebook()
     _run_test "${jupyter_datascience_notebook_id}"
 }
 
-# Description:
-# 	"Orchestration" function computes necessary parameters and prepares the running notebook workload for papermill tests to be invoked
-#		- notebook_id is calculated based on the workload name and computed accelerator value
-#		- Appropriate "source of truth" file to be used in asserting package version is copied into the running pod
-#		- papermill is installed on the running pod
-#		- All relevant tests based on the notebook_id are invoked
-function _handle_test()
-{
+function _get_notebook_id() {
     local notebook_id=
 
     # Due to existing logic - cuda accelerator value needs to be treated as empty string
@@ -372,6 +365,20 @@ function _handle_test()
             ;;
     esac
 
+    echo "${notebook_id}"
+}
+
+# Description:
+# 	"Orchestration" function computes necessary parameters and prepares the running notebook workload for papermill tests to be invoked
+#		- notebook_id is calculated based on the workload name and computed accelerator value
+#		- Appropriate "source of truth" file to be used in asserting package version is copied into the running pod
+#		- papermill is installed on the running pod
+#		- All relevant tests based on the notebook_id are invoked
+function _handle_test()
+{
+    local notebook_id=
+    notebook_id=$(_get_notebook_id)
+
     _create_test_versions_source_of_truth "${notebook_id}"
 
     "${kbin}" exec "${notebook_workload_name}" -- /bin/sh -c "python3 -m pip install papermill"
@@ -413,10 +420,15 @@ if ! [ -e "${yqbin}" ]; then
     exit 1
 fi
 
-printf '%s\n' "Waiting for ${notebook_name} workload to be ready.  This could take a few minutes..."
-_wait_for_workload "${notebook_name}"
+function main() {
+  printf '%s\n' "Waiting for ${notebook_name} workload to be ready.  This could take a few minutes..."
+  _wait_for_workload "${notebook_name}"
 
-notebook_workload_name=$("${kbin}" get pods -l app="${notebook_name}" -o jsonpath='{.items[0].metadata.name}')
+  notebook_workload_name=$("${kbin}" get pods -l app="${notebook_name}" -o jsonpath='{.items[0].metadata.name}')
 
-_handle_test
+  _handle_test
+}
 
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+    main "$@"
+fi

ci/cached_builds

@@ -3,10 +3,20 @@
 
 import dataclasses
 import enum
+import shlex
 import shutil
-import unittest
+import subprocess
+import sys
+import typing
 from pathlib import Path
 
+import pytest
+
+if typing.TYPE_CHECKING:
+    from collections.abc import Generator, Iterable
+
+ROOT_DIR = Path(__file__).parent.parent
+
 JUPYTER_MINIMAL_NOTEBOOK_ID = "minimal"
 JUPYTER_DATASCIENCE_NOTEBOOK_ID = "datascience"
 JUPYTER_TRUSTYAI_NOTEBOOK_ID = "trustyai"
@@ -192,7 +202,7 @@ def get_source_of_truth_filepath(
     return filepath
 
 
-class SelfTests(unittest.TestCase):
+class TestManifests:
     def test_rstudio_path(self):
         metadata = extract_metadata_from_path(Path("notebooks/rstudio/rhel9-python-3.11"))
         assert metadata == NotebookMetadata(
@@ -261,3 +271,96 @@ def test_source_of_truth_jupyter_tensorflow_rocm(self):
         metadata = extract_metadata_from_path(Path("notebooks/jupyter/rocm/tensorflow/ubi9-python-3.12"))
         path = get_source_of_truth_filepath(root_repo_directory=Path("notebooks"), metadata=metadata)
         assert path == Path("notebooks/manifests/base/jupyter-rocm-tensorflow-notebook-imagestream.yaml")
+
+    def run_shell_function(
+        self,
+        shell_script_path: Path,
+        shell_function_name: str,
+        args: Iterable[str] = (),
+        env: dict[str, str] | None = None,
+    ) -> str:
+        env = env or {}
+        shell_notebook_id = subprocess.run(
+            f"""source {shell_script_path} && {shell_function_name} {" ".join(shlex.quote(arg) for arg in args)}""",
+            shell=True,
+            executable="/bin/bash",
+            env=env,
+            stdout=subprocess.PIPE,
+            text=True,
+            check=True,
+        )
+        return shell_notebook_id.stdout.rstrip()
+
+    @staticmethod
+    def get_targets() -> Generator[tuple[str, Path], None, None]:
+        # TODO(jdanek): should systematize import paths to avoid this hack
+        sys.path.insert(0, str(ROOT_DIR / "ci/cached-builds"))
+        from ci.cached_builds import gen_gha_matrix_jobs  # noqa: PLC0415
+
+        python_311 = gen_gha_matrix_jobs.extract_image_targets(ROOT_DIR, env={"RELEASE_PYTHON_VERSION": "3.11"})
+        python_312 = gen_gha_matrix_jobs.extract_image_targets(ROOT_DIR, env={"RELEASE_PYTHON_VERSION": "3.12"})
+        targets = python_311 + python_312
+        expected_manifest_paths = {
+            "jupyter-minimal-ubi9-python-3.12": ROOT_DIR / "manifests/base/jupyter-minimal-notebook-imagestream.yaml",
+            "runtime-minimal-ubi9-python-3.12": ROOT_DIR / "manifests/base/jupyter-minimal-notebook-imagestream.yaml",
+            # no -gpu-?
+            "cuda-jupyter-minimal-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-minimal-notebook-imagestream.yaml",
+            "rocm-jupyter-minimal-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-minimal-notebook-imagestream.yaml",
+            "jupyter-datascience-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-datascience-notebook-imagestream.yaml",
+            "runtime-datascience-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-datascience-notebook-imagestream.yaml",
+            "cuda-jupyter-pytorch-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-pytorch-notebook-imagestream.yaml",
+            "runtime-cuda-pytorch-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-pytorch-notebook-imagestream.yaml",
+            "rocm-jupyter-pytorch-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-pytorch-notebook-imagestream.yaml",
+            "rocm-runtime-pytorch-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-pytorch-notebook-imagestream.yaml",
+            "cuda-jupyter-pytorch-llmcompressor-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-pytorch-notebook-imagestream.yaml",
+            "runtime-cuda-pytorch-llmcompressor-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-pytorch-notebook-imagestream.yaml",
+            "cuda-jupyter-tensorflow-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-tensorflow-notebook-imagestream.yaml",
+            "runtime-cuda-tensorflow-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-tensorflow-notebook-imagestream.yaml",
+            "rocm-jupyter-tensorflow-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-tensorflow-notebook-imagestream.yaml",
+            "rocm-runtime-tensorflow-ubi9-python-3.12": ROOT_DIR
+            / "manifests/base/jupyter-tensorflow-notebook-imagestream.yaml",
+            "jupyter-trustyai-ubi9-python-3.12": ROOT_DIR / "manifests/base/jupyter-trustyai-notebook-imagestream.yaml",
+            "codeserver-ubi9-python-3.12": ROOT_DIR / "manifests/base/code-server-notebook-imagestream.yaml",
+            "rstudio-ubi9-python-3.11": ROOT_DIR / "manifests/base/rstudio-buildconfig.yaml",
+            "rstudio-c9s-python-3.11": ROOT_DIR / "manifests/base/rstudio-buildconfig.yaml",
+            "cuda-rstudio-c9s-python-3.11": ROOT_DIR / "manifests/base/cuda-rstudio-buildconfig.yaml",
+            "rstudio-rhel9-python-3.11": ROOT_DIR / "manifests/base/rstudio-buildconfig.yaml",
+            "cuda-rstudio-rhel9-python-3.11": ROOT_DIR / "manifests/base/cuda-rstudio-buildconfig.yaml",
+        }
+        for target in targets:
+            if "codeserver" in target:
+                continue
+            if "rstudio" in target:
+                continue
+            yield target, expected_manifest_paths[target]
+
+    @pytest.mark.parametrize("target,expected_manifest_path", get_targets())
+    def test_compare_with_shell_implementation(self, target: str, expected_manifest_path: Path):
+        shell_script_path = ROOT_DIR / "scripts/test_jupyter_with_papermill.sh"
+
+        notebook_id = self.run_shell_function(
+            shell_script_path,
+            "_get_notebook_id",
+            env={"notebook_workload_name": target},
+        )
+        assert notebook_id
+
+        source_of_truth_filepath = self.run_shell_function(
+            shell_script_path,
+            "_get_source_of_truth_filepath",
+            [notebook_id],
+        )
+        assert source_of_truth_filepath == str(expected_manifest_path)