diff --git a/.gitattributes b/.gitattributes index 2b9b79e26c..6313b56c57 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1 @@ * text=auto eol=lf - diff --git a/all-spark-notebook/test/test_spark_notebooks.py b/all-spark-notebook/test/test_spark_notebooks.py index a87f43f81a..9879a43807 100644 --- a/all-spark-notebook/test/test_spark_notebooks.py +++ b/all-spark-notebook/test/test_spark_notebooks.py @@ -4,10 +4,12 @@ import logging import pytest -import os +from pathlib import Path + +from conftest import TrackedContainer LOGGER = logging.getLogger(__name__) -THIS_DIR = os.path.dirname(os.path.realpath(__file__)) +THIS_DIR = Path(__file__).parent.resolve() @pytest.mark.parametrize( @@ -15,9 +17,9 @@ # TODO: add local_sparklyr ["local_pyspark", "local_spylon", "local_sparkR", "issue_1168"], ) -def test_nbconvert(container, test_file): +def test_nbconvert(container: TrackedContainer, test_file: str) -> None: """Check if Spark notebooks can be executed""" - host_data_dir = os.path.join(THIS_DIR, "data") + host_data_dir = THIS_DIR / "data" cont_data_dir = "/home/jovyan/data" output_dir = "/tmp" timeout_ms = 600 @@ -29,7 +31,7 @@ def test_nbconvert(container, test_file): + f"--execute {cont_data_dir}/{test_file}.ipynb" ) c = container.run( - volumes={host_data_dir: {"bind": cont_data_dir, "mode": "ro"}}, + volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, command=["start.sh", "bash", "-c", command], ) diff --git a/base-notebook/test/test_container_options.py b/base-notebook/test/test_container_options.py index 467857bc59..9deb677dbc 100644 --- a/base-notebook/test/test_container_options.py +++ b/base-notebook/test/test_container_options.py @@ -4,11 +4,14 @@ import logging import pytest +import requests + +from conftest import TrackedContainer LOGGER = logging.getLogger(__name__) -def test_cli_args(container, http_client): +def test_cli_args(container: TrackedContainer, http_client: requests.Session) -> None: """Container should respect notebook server command line args (e.g., disabling token security)""" c = container.run(command=["start-notebook.sh", "--NotebookApp.token=''"]) @@ -26,7 +29,9 @@ def test_cli_args(container, http_client): @pytest.mark.filterwarnings("ignore:Unverified HTTPS request") -def test_unsigned_ssl(container, http_client): +def test_unsigned_ssl( + container: TrackedContainer, http_client: requests.Session +) -> None: """Container should generate a self-signed SSL certificate and notebook server should use it to enable HTTPS. """ @@ -48,7 +53,7 @@ def test_unsigned_ssl(container, http_client): assert warnings[0].startswith("WARNING: Jupyter Notebook deprecation notice") -def test_uid_change(container): +def test_uid_change(container: TrackedContainer) -> None: """Container should change the UID of the default user.""" c = container.run( tty=True, @@ -65,7 +70,7 @@ def test_uid_change(container): assert "uid=1010(jovyan)" in c.logs(stdout=True).decode("utf-8") -def test_gid_change(container): +def test_gid_change(container: TrackedContainer) -> None: """Container should change the GID of the default user.""" c = container.run( tty=True, @@ -82,7 +87,7 @@ def test_gid_change(container): assert "groups=110(jovyan),100(users)" in logs -def test_nb_user_change(container): +def test_nb_user_change(container: TrackedContainer) -> None: """Container should change the user name (`NB_USER`) of the default user.""" nb_user = "nayvoj" running_container = container.run( @@ -131,7 +136,7 @@ def test_nb_user_change(container): ), f"Hidden folder .jupyter was not copied properly to {nb_user} home folder. stat: {output}, expected {expected_output}" -def test_chown_extra(container): +def test_chown_extra(container: TrackedContainer) -> None: """Container should change the UID/GID of a comma separated CHOWN_EXTRA list of folders.""" c = container.run( @@ -160,7 +165,7 @@ def test_chown_extra(container): assert "/opt/conda/bin/jupyter:1010:101" in logs -def test_chown_home(container): +def test_chown_home(container: TrackedContainer) -> None: """Container should change the NB_USER home directory owner and group to the current value of NB_UID and NB_GID.""" c = container.run( @@ -183,7 +188,7 @@ def test_chown_home(container): assert "/home/kitten/.bashrc:1010:101" in logs -def test_sudo(container): +def test_sudo(container: TrackedContainer) -> None: """Container should grant passwordless sudo to the default user.""" c = container.run( tty=True, @@ -199,7 +204,7 @@ def test_sudo(container): assert "uid=0(root)" in logs -def test_sudo_path(container): +def test_sudo_path(container: TrackedContainer) -> None: """Container should include /opt/conda/bin in the sudo secure_path.""" c = container.run( tty=True, @@ -215,7 +220,7 @@ def test_sudo_path(container): assert logs.rstrip().endswith("/opt/conda/bin/jupyter") -def test_sudo_path_without_grant(container): +def test_sudo_path_without_grant(container: TrackedContainer) -> None: """Container should include /opt/conda/bin in the sudo secure_path.""" c = container.run( tty=True, @@ -230,7 +235,7 @@ def test_sudo_path_without_grant(container): assert logs.rstrip().endswith("/opt/conda/bin/jupyter") -def test_group_add(container): +def test_group_add(container: TrackedContainer) -> None: """Container should run with the specified uid, gid, and secondary group. It won't be possible to modify /etc/passwd since gid is nonzero, so additionally verify that setting gid=0 is suggested in a warning. @@ -252,7 +257,7 @@ def test_group_add(container): assert "uid=1010 gid=1010 groups=1010,100(users)" in logs -def test_set_uid(container): +def test_set_uid(container: TrackedContainer) -> None: """Container should run with the specified uid and NB_USER. The /home/jovyan directory will not be writable since it's owned by 1000:users. Additionally verify that "--group-add=users" is suggested in a warning to restore @@ -274,7 +279,7 @@ def test_set_uid(container): assert "--group-add=users" in warnings[0] -def test_set_uid_and_nb_user(container): +def test_set_uid_and_nb_user(container: TrackedContainer) -> None: """Container should run with the specified uid and NB_USER.""" c = container.run( user="1010", @@ -294,7 +299,7 @@ def test_set_uid_and_nb_user(container): assert "user is kitten but home is /home/jovyan" in warnings[0] -def test_container_not_delete_bind_mount(container, tmp_path): +def test_container_not_delete_bind_mount(container: TrackedContainer, tmp_path) -> None: """Container should not delete host system files when using the (docker) -v bind mount flag and mapping to /home/jovyan. """ @@ -324,7 +329,9 @@ def test_container_not_delete_bind_mount(container, tmp_path): @pytest.mark.parametrize("enable_root", [False, True]) -def test_jupyter_env_vars_to_unset_as_root(container, enable_root): +def test_jupyter_env_vars_to_unset_as_root( + container: TrackedContainer, enable_root: bool +) -> None: """Environment variables names listed in JUPYTER_ENV_VARS_TO_UNSET should be unset in the final environment.""" root_args = {"user": "root"} if enable_root else {} diff --git a/base-notebook/test/test_package_managers.py b/base-notebook/test/test_package_managers.py index 62ebaadf0e..6d06dedd4d 100644 --- a/base-notebook/test/test_package_managers.py +++ b/base-notebook/test/test_package_managers.py @@ -4,6 +4,8 @@ import logging import pytest +from conftest import TrackedContainer + LOGGER = logging.getLogger(__name__) @@ -17,7 +19,9 @@ ("pip", "--version"), ], ) -def test_package_manager(container, package_manager, version_arg): +def test_package_manager( + container: TrackedContainer, package_manager: str, version_arg: tuple[str, ...] +) -> None: """Test the notebook start-notebook script""" LOGGER.info( f"Test that the package manager {package_manager} is working properly ..." diff --git a/base-notebook/test/test_pandoc.py b/base-notebook/test/test_pandoc.py index 5c8d33b103..fa2cfa3946 100644 --- a/base-notebook/test/test_pandoc.py +++ b/base-notebook/test/test_pandoc.py @@ -3,10 +3,12 @@ import logging +from conftest import TrackedContainer + LOGGER = logging.getLogger(__name__) -def test_pandoc(container): +def test_pandoc(container: TrackedContainer) -> None: """Pandoc shall be able to convert MD to HTML.""" c = container.run( tty=True, diff --git a/base-notebook/test/test_python.py b/base-notebook/test/test_python.py index 67cecf824a..5ba6aaf8b8 100644 --- a/base-notebook/test/test_python.py +++ b/base-notebook/test/test_python.py @@ -4,10 +4,14 @@ from packaging import version +from conftest import TrackedContainer + LOGGER = logging.getLogger(__name__) -def test_python_version(container, python_next_version="3.10"): +def test_python_version( + container: TrackedContainer, python_next_version: str = "3.10" +) -> None: """Check that python version is lower than the next version""" LOGGER.info(f"Checking that python version is lower than {python_next_version}") c = container.run( diff --git a/base-notebook/test/test_start_container.py b/base-notebook/test/test_start_container.py index 81a131a513..c644a8580d 100644 --- a/base-notebook/test/test_start_container.py +++ b/base-notebook/test/test_start_container.py @@ -3,6 +3,9 @@ import logging import pytest +import requests + +from conftest import TrackedContainer LOGGER = logging.getLogger(__name__) @@ -14,7 +17,12 @@ (None, "notebook"), ], ) -def test_start_notebook(container, http_client, env, expected_server): +def test_start_notebook( + container: TrackedContainer, + http_client: requests.Session, + env, + expected_server: str, +) -> None: """Test the notebook start-notebook script""" LOGGER.info( f"Test that the start-notebook launches the {expected_server} server from the env {env} ..." @@ -46,7 +54,9 @@ def test_start_notebook(container, http_client, env, expected_server): assert msg in logs, f"Expected warning message {msg} not printed" -def test_tini_entrypoint(container, pid=1, command="tini"): +def test_tini_entrypoint( + container: TrackedContainer, pid: int = 1, command: str = "tini" +) -> None: """Check that tini is launched as PID 1 Credits to the following answer for the ps options used in the test: diff --git a/conftest.py b/conftest.py index 3c9571429c..07f350a8df 100644 --- a/conftest.py +++ b/conftest.py @@ -2,6 +2,7 @@ # Distributed under the terms of the Modified BSD License. import os import logging +import typing import docker import pytest @@ -15,7 +16,7 @@ @pytest.fixture(scope="session") -def http_client(): +def http_client() -> requests.Session: """Requests session with retries and backoff.""" s = requests.Session() retries = Retry(total=5, backoff_factor=1) @@ -25,13 +26,13 @@ def http_client(): @pytest.fixture(scope="session") -def docker_client(): +def docker_client() -> docker.DockerClient: """Docker client configured based on the host environment""" return docker.from_env() @pytest.fixture(scope="session") -def image_name(): +def image_name() -> str: """Image name to test""" return os.getenv("TEST_IMAGE") @@ -50,13 +51,15 @@ class TrackedContainer: Default keyword arguments to pass to docker.DockerClient.containers.run """ - def __init__(self, docker_client, image_name, **kwargs): + def __init__( + self, docker_client: docker.DockerClient, image_name: str, **kwargs: typing.Any + ): self.container = None self.docker_client = docker_client self.image_name = image_name self.kwargs = kwargs - def run(self, **kwargs): + def run(self, **kwargs: typing.Any): """Runs a docker container using the preconfigured image name and a mix of the preconfigured container options and those passed to this method. @@ -74,9 +77,7 @@ def run(self, **kwargs): ------- docker.Container """ - all_kwargs = {} - all_kwargs.update(self.kwargs) - all_kwargs.update(kwargs) + all_kwargs = self.kwargs | kwargs LOGGER.info(f"Running {self.image_name} with args {all_kwargs} ...") self.container = self.docker_client.containers.run( self.image_name, @@ -91,7 +92,7 @@ def remove(self): @pytest.fixture(scope="function") -def container(docker_client, image_name): +def container(docker_client: docker.DockerClient, image_name: str): """Notebook container with initial configuration appropriate for testing (e.g., HTTP port exposed to the host for HTTP calls). diff --git a/datascience-notebook/test/test_julia.py b/datascience-notebook/test/test_julia.py index 610fab3db2..d1a79aa671 100644 --- a/datascience-notebook/test/test_julia.py +++ b/datascience-notebook/test/test_julia.py @@ -2,10 +2,12 @@ # Distributed under the terms of the Modified BSD License. import logging +from conftest import TrackedContainer + LOGGER = logging.getLogger(__name__) -def test_julia(container): +def test_julia(container: TrackedContainer) -> None: """Basic julia test""" LOGGER.info("Test that julia is correctly installed ...") running_container = container.run( diff --git a/minimal-notebook/test/test_inkscape.py b/minimal-notebook/test/test_inkscape.py index a39bcb152d..451ef9b5b5 100644 --- a/minimal-notebook/test/test_inkscape.py +++ b/minimal-notebook/test/test_inkscape.py @@ -3,10 +3,12 @@ import logging +from conftest import TrackedContainer + LOGGER = logging.getLogger(__name__) -def test_inkscape(container): +def test_inkscape(container: TrackedContainer) -> None: """Inkscape shall be installed to be able to convert SVG files.""" LOGGER.info("Test that inkscape is working by printing its version ...") c = container.run( diff --git a/minimal-notebook/test/test_nbconvert.py b/minimal-notebook/test/test_nbconvert.py index e5d280865f..755c7f5c35 100644 --- a/minimal-notebook/test/test_nbconvert.py +++ b/minimal-notebook/test/test_nbconvert.py @@ -4,10 +4,10 @@ import logging import pytest -import os +from pathlib import Path LOGGER = logging.getLogger(__name__) -THIS_DIR = os.path.dirname(os.path.realpath(__file__)) +THIS_DIR = Path(__file__).parent.resolve() @pytest.mark.parametrize( @@ -19,9 +19,9 @@ ("notebook_svg", "html"), ], ) -def test_nbconvert(container, test_file, output_format): +def test_nbconvert(container, test_file: str, output_format: str) -> None: """Check if nbconvert is able to convert a notebook file""" - host_data_dir = os.path.join(THIS_DIR, "data") + host_data_dir = THIS_DIR / "data" cont_data_dir = "/home/jovyan/data" output_dir = "/tmp" LOGGER.info( @@ -29,7 +29,7 @@ def test_nbconvert(container, test_file, output_format): ) command = f"jupyter nbconvert {cont_data_dir}/{test_file}.ipynb --output-dir {output_dir} --to {output_format}" c = container.run( - volumes={host_data_dir: {"bind": cont_data_dir, "mode": "ro"}}, + volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, command=["start.sh", "bash", "-c", command], ) diff --git a/pyspark-notebook/test/test_spark.py b/pyspark-notebook/test/test_spark.py index 1cfba7e06a..66abeb9a24 100644 --- a/pyspark-notebook/test/test_spark.py +++ b/pyspark-notebook/test/test_spark.py @@ -2,11 +2,13 @@ # Distributed under the terms of the Modified BSD License. import logging +from conftest import TrackedContainer + LOGGER = logging.getLogger(__name__) -def test_spark_shell(container): +def test_spark_shell(container: TrackedContainer) -> None: """Checking if Spark (spark-shell) is running properly""" c = container.run( tty=True, diff --git a/scipy-notebook/test/test_extensions.py b/scipy-notebook/test/test_extensions.py index 03f80d5fb9..386fe4f67e 100644 --- a/scipy-notebook/test/test_extensions.py +++ b/scipy-notebook/test/test_extensions.py @@ -4,6 +4,8 @@ import pytest +from conftest import TrackedContainer + LOGGER = logging.getLogger(__name__) @@ -16,7 +18,7 @@ "jupyter-matplotlib", ], ) -def test_check_extension(container, extension): +def test_check_extension(container: TrackedContainer, extension: str) -> None: """Basic check of each extension The list of extensions can be obtained through this command diff --git a/scipy-notebook/test/test_matplotlib.py b/scipy-notebook/test/test_matplotlib.py index e6233182ee..10229df7e9 100644 --- a/scipy-notebook/test/test_matplotlib.py +++ b/scipy-notebook/test/test_matplotlib.py @@ -4,10 +4,12 @@ import logging import pytest -import os +from pathlib import Path + +from conftest import TrackedContainer LOGGER = logging.getLogger(__name__) -THIS_DIR = os.path.dirname(os.path.realpath(__file__)) +THIS_DIR = Path(__file__).parent.resolve() @pytest.mark.parametrize( @@ -25,19 +27,21 @@ ), ], ) -def test_matplotlib(container, test_file, expected_file, description): +def test_matplotlib( + container: TrackedContainer, test_file: str, expected_file: str, description: str +): """Various tests performed on matplotlib - Test that matplotlib is able to plot a graph and write it as an image - Test matplotlib latex fonts, which depend on the cm-super package """ - host_data_dir = os.path.join(THIS_DIR, "data") + host_data_dir = THIS_DIR / "data" cont_data_dir = "/home/jovyan/data" output_dir = "/tmp" LOGGER.info(description) command = "sleep infinity" running_container = container.run( - volumes={host_data_dir: {"bind": cont_data_dir, "mode": "ro"}}, + volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, command=["start.sh", "bash", "-c", command], ) diff --git a/tagging/create_manifests.py b/tagging/create_manifests.py index 1eceba9dff..7058cbd0e7 100755 --- a/tagging/create_manifests.py +++ b/tagging/create_manifests.py @@ -11,7 +11,7 @@ from .manifests import ManifestHeader, ManifestInterface -logger = logging.getLogger(__name__) +LOGGER = logging.getLogger(__name__) BUILD_TIMESTAMP = datetime.datetime.utcnow().isoformat()[:-7] + "Z" @@ -24,7 +24,7 @@ def append_build_history_line( wiki_path: str, all_tags: list[str], ) -> None: - logger.info("Appending build history line") + LOGGER.info("Appending build history line") date_column = f"`{BUILD_TIMESTAMP}`" image_column = MARKDOWN_LINE_BREAK.join( @@ -58,7 +58,7 @@ def create_manifest_file( container, ) -> None: manifest_names = [manifest.__name__ for manifest in manifests] - logger.info(f"Using manifests: {manifest_names}") + LOGGER.info(f"Using manifests: {manifest_names}") commit_hash_tag = GitHelper.commit_hash_tag() manifest_file = os.path.join( @@ -76,7 +76,7 @@ def create_manifest_file( def create_manifests(short_image_name: str, owner: str, wiki_path: str) -> None: - logger.info(f"Creating manifests for image: {short_image_name}") + LOGGER.info(f"Creating manifests for image: {short_image_name}") taggers, manifests = get_taggers_and_manifests(short_image_name) image = f"{owner}/{short_image_name}:latest" @@ -100,6 +100,6 @@ def create_manifests(short_image_name: str, owner: str, wiki_path: str) -> None: arg_parser.add_argument("--wiki-path", required=True, help="Path to the wiki pages") args = arg_parser.parse_args() - logger.info(f"Current build timestamp: {BUILD_TIMESTAMP}") + LOGGER.info(f"Current build timestamp: {BUILD_TIMESTAMP}") create_manifests(args.short_image_name, args.owner, args.wiki_path) diff --git a/tagging/docker_runner.py b/tagging/docker_runner.py index 15780b7e73..1419fd96be 100644 --- a/tagging/docker_runner.py +++ b/tagging/docker_runner.py @@ -4,7 +4,7 @@ import logging -logger = logging.getLogger(__name__) +LOGGER = logging.getLogger(__name__) class DockerRunner: @@ -20,27 +20,27 @@ def __init__( self.docker_client = docker_client def __enter__(self): - logger.info(f"Creating container for image {self.image_name} ...") + LOGGER.info(f"Creating container for image {self.image_name} ...") self.container = self.docker_client.containers.run( image=self.image_name, command=self.command, detach=True, ) - logger.info(f"Container {self.container.name} created") + LOGGER.info(f"Container {self.container.name} created") return self.container def __exit__(self, exc_type, exc_value, traceback): - logger.info(f"Removing container {self.container.name} ...") + LOGGER.info(f"Removing container {self.container.name} ...") if self.container: self.container.remove(force=True) - logger.info(f"Container {self.container.name} removed") + LOGGER.info(f"Container {self.container.name} removed") @staticmethod def run_simple_command(container, cmd: str, print_result: bool = True): - logger.info(f"Running cmd: '{cmd}' on container: {container}") + LOGGER.info(f"Running cmd: '{cmd}' on container: {container}") out = container.exec_run(cmd) result = out.output.decode("utf-8").rstrip() if print_result: - logger.info(f"Command result: {result}") + LOGGER.info(f"Command result: {result}") assert out.exit_code == 0, f"Command: {cmd} failed" return result diff --git a/tagging/manifests.py b/tagging/manifests.py index 2c262fc1ea..e56b4a6afe 100644 --- a/tagging/manifests.py +++ b/tagging/manifests.py @@ -1,14 +1,10 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. -import logging from plumbum.cmd import docker from .docker_runner import DockerRunner from .git_helper import GitHelper -logger = logging.getLogger(__name__) - - def quoted_output(container, cmd: str) -> str: return "\n".join( [ diff --git a/tagging/tag_image.py b/tagging/tag_image.py index 105da483cb..fca9818e92 100755 --- a/tagging/tag_image.py +++ b/tagging/tag_image.py @@ -9,7 +9,7 @@ from .github_set_env import github_set_env -logger = logging.getLogger(__name__) +LOGGER = logging.getLogger(__name__) def tag_image(short_image_name: str, owner: str) -> None: @@ -20,7 +20,7 @@ def tag_image(short_image_name: str, owner: str) -> None: Tags are in a GitHub Actions environment also saved to environment variables in a format making it easy to append them. """ - logger.info(f"Tagging image: {short_image_name}") + LOGGER.info(f"Tagging image: {short_image_name}") taggers, _ = get_taggers_and_manifests(short_image_name) image = f"{owner}/{short_image_name}:latest" @@ -31,7 +31,7 @@ def tag_image(short_image_name: str, owner: str) -> None: tagger_name = tagger.__name__ tag_value = tagger.tag_value(container) tags.append(tag_value) - logger.info( + LOGGER.info( f"Applying tag tagger_name: {tagger_name} tag_value: {tag_value}" ) docker["tag", image, f"{owner}/{short_image_name}:{tag_value}"]() diff --git a/tagging/taggers.py b/tagging/taggers.py index ee7e6f02d5..8274aa2c00 100644 --- a/tagging/taggers.py +++ b/tagging/taggers.py @@ -1,14 +1,10 @@ # Copyright (c) Jupyter Development Team. # Distributed under the terms of the Modified BSD License. from datetime import datetime -import logging from .git_helper import GitHelper from .docker_runner import DockerRunner -logger = logging.getLogger(__name__) - - def _get_program_version(container, program: str) -> str: return DockerRunner.run_simple_command(container, cmd=f"{program} --version") diff --git a/test/helpers.py b/test/helpers.py index e58abecc0c..b9d9ae8dd9 100644 --- a/test/helpers.py +++ b/test/helpers.py @@ -27,25 +27,27 @@ from itertools import chain import logging import json +from typing import Optional from tabulate import tabulate +from conftest import TrackedContainer + LOGGER = logging.getLogger(__name__) class CondaPackageHelper: """Conda package helper permitting to get information about packages""" - def __init__(self, container): - # if isinstance(container, TrackedContainer): + def __init__(self, container: TrackedContainer): self.running_container = CondaPackageHelper.start_container(container) - self.specs = None - self.installed = None - self.available = None - self.comparison = None + self.requested: Optional[dict[str, set[str]]] = None + self.installed: Optional[dict[str, set[str]]] = None + self.available: Optional[dict[str, set[str]]] = None + self.comparison: list[dict[str, str]] = [] @staticmethod - def start_container(container): + def start_container(container: TrackedContainer): """Start the TrackedContainer and return an instance of a running container""" LOGGER.info(f"Starting container {container.image_name} ...") return container.run( @@ -54,32 +56,34 @@ def start_container(container): ) @staticmethod - def _conda_export_command(from_history=False): + def _conda_export_command(from_history: bool) -> list[str]: """Return the mamba export command with or without history""" cmd = ["mamba", "env", "export", "-n", "base", "--json", "--no-builds"] if from_history: cmd.append("--from-history") return cmd - def installed_packages(self): + def installed_packages(self) -> dict[str, set[str]]: """Return the installed packages""" if self.installed is None: LOGGER.info("Grabing the list of installed packages ...") self.installed = CondaPackageHelper._packages_from_json( - self._execute_command(CondaPackageHelper._conda_export_command()) + self._execute_command( + CondaPackageHelper._conda_export_command(from_history=False) + ) ) return self.installed - def specified_packages(self): - """Return the specifications (i.e. packages installation requested)""" - if self.specs is None: - LOGGER.info("Grabing the list of specifications ...") - self.specs = CondaPackageHelper._packages_from_json( + def requested_packages(self) -> dict[str, set[str]]: + """Return the requested package (i.e. `mamba install `)""" + if self.requested is None: + LOGGER.info("Grabing the list of manually requested packages ...") + self.requested = CondaPackageHelper._packages_from_json( self._execute_command( CondaPackageHelper._conda_export_command(from_history=True) ) ) - return self.specs + return self.requested def _execute_command(self, command): """Execute a command on a running container""" @@ -87,14 +91,14 @@ def _execute_command(self, command): return rc.output.decode("utf-8") @staticmethod - def _packages_from_json(env_export): + def _packages_from_json(env_export) -> dict[str, set[str]]: """Extract packages and versions from the lines returned by the list of specifications""" # dependencies = filter(lambda x: isinstance(x, str), json.loads(env_export).get("dependencies")) dependencies = json.loads(env_export).get("dependencies") # Filtering packages installed through pip in this case it's a dict {'pip': ['toree==0.3.0']} # Since we only manage packages installed through mamba here dependencies = filter(lambda x: isinstance(x, str), dependencies) - packages_dict = dict() + packages_dict: dict[str, set[str]] = dict() for split in map(lambda x: re.split("=?=", x), dependencies): # default values package = split[0] @@ -129,14 +133,16 @@ def _extract_available(lines): ddict[pkg].add(version) return ddict - def check_updatable_packages(self, specifications_only=True): + def check_updatable_packages( + self, requested_only: bool = True + ) -> list[dict[str, str]]: """Check the updatable packages including or not dependencies""" - specs = self.specified_packages() + requested = self.requested_packages() installed = self.installed_packages() available = self.available_packages() - self.comparison = list() + self.comparison = [] for pkg, inst_vs in installed.items(): - if not specifications_only or pkg in specs: + if not requested_only or pkg in requested: avail_vs = sorted( list(available[pkg]), key=CondaPackageHelper.semantic_cmp ) @@ -156,7 +162,7 @@ def check_updatable_packages(self, specifications_only=True): return self.comparison @staticmethod - def semantic_cmp(version_string): + def semantic_cmp(version_string: str): """Manage semantic versioning for comparison""" def mysplit(string): @@ -165,14 +171,14 @@ def version_substrs(x): return list(chain(map(version_substrs, string.split(".")))) - def str_ord(string): + def str_ord(string: str) -> int: num = 0 for char in string: num *= 255 num += ord(char) return num - def try_int(version_str): + def try_int(version_str: str) -> int: try: return int(version_str) except ValueError: @@ -181,13 +187,13 @@ def try_int(version_str): mss = list(chain(*mysplit(version_string))) return tuple(map(try_int, mss)) - def get_outdated_summary(self, specifications_only=True): + def get_outdated_summary(self, requested_only: bool = True) -> str: """Return a summary of outdated packages""" - nb_packages = len(self.specs if specifications_only else self.installed) + nb_packages = len(self.requested if requested_only else self.installed) nb_updatable = len(self.comparison) updatable_ratio = nb_updatable / nb_packages return f"{nb_updatable}/{nb_packages} ({updatable_ratio:.0%}) packages could be updated" - def get_outdated_table(self): + def get_outdated_table(self) -> str: """Return a table of outdated packages""" return tabulate(self.comparison, headers="keys") diff --git a/test/test_notebook.py b/test/test_notebook.py index 548c272d6c..4794cc5fc3 100644 --- a/test/test_notebook.py +++ b/test/test_notebook.py @@ -2,7 +2,13 @@ # Distributed under the terms of the Modified BSD License. -def test_secured_server(container, http_client): +import requests +from conftest import TrackedContainer + + +def test_secured_server( + container: TrackedContainer, http_client: requests.Session +) -> None: """Notebook server should eventually request user login.""" container.run() resp = http_client.get("http://localhost:8888") diff --git a/test/test_outdated.py b/test/test_outdated.py index 91c9156179..28fc46302d 100644 --- a/test/test_outdated.py +++ b/test/test_outdated.py @@ -4,6 +4,7 @@ import logging import pytest +from conftest import TrackedContainer from helpers import CondaPackageHelper @@ -11,10 +12,10 @@ @pytest.mark.info -def test_outdated_packages(container, specifications_only=True): +def test_outdated_packages(container: TrackedContainer, requested_only: bool = True): """Getting the list of updatable packages""" LOGGER.info(f"Checking outdated packages in {container.image_name} ...") pkg_helper = CondaPackageHelper(container) - pkg_helper.check_updatable_packages(specifications_only) - LOGGER.info(pkg_helper.get_outdated_summary(specifications_only)) + pkg_helper.check_updatable_packages(requested_only) + LOGGER.info(pkg_helper.get_outdated_summary(requested_only)) LOGGER.info(f"\n{pkg_helper.get_outdated_table()}\n") diff --git a/test/test_packages.py b/test/test_packages.py index 92d51ee27c..835c9a8081 100644 --- a/test/test_packages.py +++ b/test/test_packages.py @@ -12,9 +12,9 @@ - #1012: issue importing `sympy` - #966: isssue importing `pyarrow` -This module checks dynamically, through the `CondaPackageHelper`, only the specified packages i.e. packages requested by `mamba install` in the `Dockerfile`s. +This module checks dynamically, through the `CondaPackageHelper`, only the requested packages i.e. packages requested by `mamba install` in the `Dockerfile`s. This means that it does not check dependencies. This choice is a tradeoff to cover the main requirements while achieving reasonable test duration. -However it could be easily changed (or completed) to cover also dependencies `package_helper.installed_packages()` instead of `package_helper.specified_packages()`. +However it could be easily changed (or completed) to cover also dependencies `package_helper.installed_packages()` instead of `package_helper.requested_packages()`. Example: @@ -25,7 +25,7 @@ # --------------------------------------------------------------------------------------------- live log setup ---------------------------------------------------------------------------------------------- # 2020-03-08 09:56:04 [ INFO] Starting container jupyter/datascience-notebook ... (helpers.py:51) # 2020-03-08 09:56:04 [ INFO] Running jupyter/datascience-notebook with args {'detach': True, 'ports': {'8888/tcp': 8888}, 'tty': True, 'command': ['start.sh', 'bash', '-c', 'sleep infinity']} ... (conftest.py:78) - # 2020-03-08 09:56:04 [ INFO] Grabing the list of specifications ... (helpers.py:76) + # 2020-03-08 09:56:04 [ INFO] Grabing the list of manually requested packages ... (helpers.py:76) # ---------------------------------------------------------------------------------------------- live log call ---------------------------------------------------------------------------------------------- # 2020-03-08 09:56:07 [ INFO] Testing the import of packages ... (test_packages.py:125) # 2020-03-08 09:56:07 [ INFO] Trying to import conda (test_packages.py:127) @@ -38,6 +38,7 @@ import logging import pytest +from conftest import TrackedContainer from helpers import CondaPackageHelper @@ -75,57 +76,66 @@ @pytest.fixture(scope="function") -def package_helper(container): +def package_helper(container: TrackedContainer) -> CondaPackageHelper: """Return a package helper object that can be used to perform tests on installed packages""" return CondaPackageHelper(container) @pytest.fixture(scope="function") -def packages(package_helper): - """Return the list of specified packages (i.e. packages explicitely installed excluding dependencies)""" - return package_helper.specified_packages() +def packages(package_helper: CondaPackageHelper) -> dict[str, set[str]]: + """Return the list of requested packages (i.e. packages explicitly installed excluding dependencies)""" + return package_helper.requested_packages() -def package_map(package): +def package_map(package: str) -> str: """Perform a mapping between the python package name and the name used for the import""" return PACKAGE_MAPPING.get(package, package) -def excluded_package_predicate(package): +def excluded_package_predicate(package: str) -> bool: """Return whether a package is excluded from the list (i.e. a package that cannot be tested with standard imports)""" return package in EXCLUDED_PACKAGES -def python_package_predicate(package): +def python_package_predicate(package: str) -> bool: """Predicate matching python packages""" return not excluded_package_predicate(package) and not r_package_predicate(package) -def r_package_predicate(package): +def r_package_predicate(package: str) -> bool: """Predicate matching R packages""" return not excluded_package_predicate(package) and package.startswith("r-") -def _check_import_package(package_helper, command): +def _check_import_package( + package_helper: CondaPackageHelper, command: list[str] +) -> int: """Generic function executing a command""" LOGGER.debug(f"Trying to import a package with [{command}] ...") rc = package_helper.running_container.exec_run(command) return rc.exit_code -def check_import_python_package(package_helper, package): +def check_import_python_package( + package_helper: CondaPackageHelper, package: str +) -> int: """Try to import a Python package from the command line""" return _check_import_package(package_helper, ["python", "-c", f"import {package}"]) -def check_import_r_package(package_helper, package): +def check_import_r_package(package_helper: CondaPackageHelper, package: str) -> int: """Try to import a R package from the command line""" return _check_import_package( package_helper, ["R", "--slave", "-e", f"library({package})"] ) -def _import_packages(package_helper, filtered_packages, check_function, max_failures): +def _import_packages( + package_helper: CondaPackageHelper, + filtered_packages: dict[str, set[str]], + check_function, + max_failures: int, +) -> None: """Test if packages can be imported Note: using a list of packages instead of a fixture for the list of packages since pytest prevents use of multiple yields @@ -147,7 +157,7 @@ def _import_packages(package_helper, filtered_packages, check_function, max_fail @pytest.fixture(scope="function") -def r_packages(packages): +def r_packages(packages: dict[str, set[str]]): """Return an iterable of R packages""" # package[2:] is to remove the leading "r-" appended on R packages return map( @@ -155,21 +165,25 @@ def r_packages(packages): ) -def test_python_packages(package_helper, python_packages, max_failures=0): - """Test the import of specified python packages""" +def test_r_packages( + package_helper: CondaPackageHelper, r_packages, max_failures: int = 0 +): + """Test the import of specified R packages""" return _import_packages( - package_helper, python_packages, check_import_python_package, max_failures + package_helper, r_packages, check_import_r_package, max_failures ) @pytest.fixture(scope="function") -def python_packages(packages): +def python_packages(packages: dict[str, set[str]]): """Return an iterable of Python packages""" return map(package_map, filter(python_package_predicate, packages)) -def test_r_packages(package_helper, r_packages, max_failures=0): - """Test the import of specified R packages""" +def test_python_packages( + package_helper: CondaPackageHelper, python_packages, max_failures: int = 0 +): + """Test the import of specified python packages""" return _import_packages( - package_helper, r_packages, check_import_r_package, max_failures + package_helper, python_packages, check_import_python_package, max_failures ) diff --git a/test/test_units.py b/test/test_units.py index bfa120f9e4..456d03e96e 100644 --- a/test/test_units.py +++ b/test/test_units.py @@ -2,32 +2,35 @@ # Distributed under the terms of the Modified BSD License. import logging -import os +from pathlib import Path + +from conftest import TrackedContainer LOGGER = logging.getLogger(__name__) -THIS_DIR = os.path.dirname(os.path.realpath(__file__)) +THIS_DIR = Path(__file__).parent.resolve() -def test_units(container): +def test_units(container: TrackedContainer) -> None: """Various units tests Add a py file in the {image}/test/units dir and it will be automatically tested """ short_image_name = container.image_name[container.image_name.rfind("/") + 1 :] - host_data_dir = os.path.join(THIS_DIR, f"../{short_image_name}/test/units") + host_data_dir = THIS_DIR / f"../{short_image_name}/test/units" LOGGER.info(f"Searching for units tests in {host_data_dir}") cont_data_dir = "/home/jovyan/data" - if not os.path.exists(host_data_dir): + if not host_data_dir.exists(): LOGGER.info(f"Not found unit tests for image: {container.image_name}") return - for test_file in os.listdir(host_data_dir): - LOGGER.info(f"Running unit test: {test_file}") + for test_file in host_data_dir.iterdir(): + test_file_name = test_file.name + LOGGER.info(f"Running unit test: {test_file_name}") c = container.run( - volumes={host_data_dir: {"bind": cont_data_dir, "mode": "ro"}}, + volumes={str(host_data_dir): {"bind": cont_data_dir, "mode": "ro"}}, tty=True, - command=["start.sh", "python", f"{cont_data_dir}/{test_file}"], + command=["start.sh", "python", f"{cont_data_dir}/{test_file_name}"], ) rv = c.wait(timeout=30) logs = c.logs(stdout=True).decode("utf-8")