Skip to content

Add some typing to tests #1573

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

Merged
merged 6 commits into from
Jan 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .gitattributes
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
* text=auto eol=lf

12 changes: 7 additions & 5 deletions all-spark-notebook/test/test_spark_notebooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,22 @@
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(
"test_file",
# 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
Expand All @@ -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],
)
Expand Down
37 changes: 22 additions & 15 deletions base-notebook/test/test_container_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=''"])
Expand All @@ -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.
"""
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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",
Expand All @@ -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.
"""
Expand Down Expand Up @@ -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 {}
Expand Down
6 changes: 5 additions & 1 deletion base-notebook/test/test_package_managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import logging
import pytest

from conftest import TrackedContainer

LOGGER = logging.getLogger(__name__)


Expand All @@ -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 ..."
Expand Down
4 changes: 3 additions & 1 deletion base-notebook/test/test_pandoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion base-notebook/test/test_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
14 changes: 12 additions & 2 deletions base-notebook/test/test_start_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@

import logging
import pytest
import requests

from conftest import TrackedContainer

LOGGER = logging.getLogger(__name__)

Expand All @@ -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} ..."
Expand Down Expand Up @@ -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:
Expand Down
19 changes: 10 additions & 9 deletions conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Distributed under the terms of the Modified BSD License.
import os
import logging
import typing

import docker
import pytest
Expand All @@ -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)
Expand All @@ -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")

Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -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).

Expand Down
4 changes: 3 additions & 1 deletion datascience-notebook/test/test_julia.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 3 additions & 1 deletion minimal-notebook/test/test_inkscape.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading