From cab8b27d67229c4099ee45baa717826fedda9645 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= Date: Fri, 27 Dec 2024 19:57:51 +0100 Subject: [PATCH] Make Container inherit from ParameterSet --- poetry.lock | 4 +- pyproject.toml | 11 ++- pytest_container/container.py | 169 +++++++++++++++++++++++++++------- pytest_container/plugin.py | 19 ++-- pytest_container/pod.py | 65 +++++++++++-- tests/test_container.py | 1 + tests/test_marks.py | 99 ++++++++++++++++++++ 7 files changed, 317 insertions(+), 51 deletions(-) create mode 100644 tests/test_marks.py diff --git a/poetry.lock b/poetry.lock index f77adcb..0ddebe7 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand. +# This file is automatically @generated by Poetry 1.8.4 and should not be changed by hand. [[package]] name = "alabaster" @@ -1272,4 +1272,4 @@ testing = ["func-timeout", "jaraco.itertools", "pytest (>=4.6)", "pytest-black ( [metadata] lock-version = "2.0" python-versions = ">=3.6.2,<4.0" -content-hash = "102929861937499368a9f2c3c71533af949816aa8da41fa845dd51ed97af46b8" +content-hash = "2145d8b0e029e653b3d8152e00b6a2c85863f59fe3699164637f5c41a04929d7" diff --git a/pyproject.toml b/pyproject.toml index 2e5e9ff..65173cb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,7 @@ pytest-testinfra = [ { version = ">=8.0", python = ">= 3.8" } ] dataclasses = { version = ">=0.8", python = "< 3.7" } -typing-extensions = { version = ">=3.0", markers="python_version < '3.8'" } +typing-extensions = { version = ">=3.0", markers="python_version < '3.10'" } cached-property = { version = "^1.5", markers="python_version < '3.8'" } filelock = "^3.4" deprecation = "^2.1" @@ -69,3 +69,12 @@ strict = true [[tool.mypy.overrides]] module = "testinfra,deprecation" ignore_missing_imports = true + +[tool.pytest.ini_options] +xfail_strict = true +addopts = "--strict-markers" +markers = [ + 'secretleapmark', + 'othersecretmark', + 'secretpodmark', +] diff --git a/pytest_container/container.py b/pytest_container/container.py index 5e832fd..09e994c 100644 --- a/pytest_container/container.py +++ b/pytest_container/container.py @@ -34,6 +34,11 @@ from typing import List from typing import Optional from typing import overload + +try: + from typing import Self +except ImportError: + from typing_extensions import Self from typing import Tuple from typing import Type from typing import Union @@ -45,6 +50,8 @@ import testinfra from filelock import BaseFileLock from filelock import FileLock +from pytest import Mark +from pytest import MarkDecorator from pytest_container.helpers import get_always_pull_option from pytest_container.helpers import get_extra_build_args from pytest_container.helpers import get_extra_run_args @@ -431,37 +438,46 @@ class EntrypointSelection(enum.Enum): IMAGE = enum.auto() -@dataclass -class ContainerBase: +class ContainerBase(ABC, _pytest.mark.ParameterSet): """Base class for defining containers to be tested. Not to be used directly, instead use :py:class:`Container` or :py:class:`DerivedContainer`. """ + def __new__(cls, *args, **kwargs): + # Filter out all fields of ParameterSet and invoke object.__new__ only for the + # fields that it supports + parameter_set_fields = _pytest.mark.ParameterSet._fields + filtered_kwargs = {} + for f in parameter_set_fields: + filtered_kwargs[f] = kwargs.get(f, None) + + return super().__new__(cls, *args, **filtered_kwargs) + #: Full url to this container via which it can be pulled #: #: If your container image is not available via a registry and only locally, #: then you can use the following syntax: ``containers-storage:$local_name`` - url: str = "" + # url: str = "" #: id of the container if it is not available via a registry URL - container_id: str = "" + # container_id: str = "" #: Defines which entrypoint of the container is used. #: By default either :py:attr:`custom_entry_point` will be used (if defined) #: or the container's entrypoint or cmd. If neither of the two is set, then #: :file:`/bin/bash` will be used. - entry_point: EntrypointSelection = EntrypointSelection.AUTO + # entry_point: EntrypointSelection = EntrypointSelection.AUTO #: custom entry point for this container (i.e. neither its default, nor #: :file:`/bin/bash`) - custom_entry_point: Optional[str] = None + # custom_entry_point: Optional[str] = None #: List of additional flags that will be inserted after #: `docker/podman run -d` and before the image name (i.e. these arguments #: are not passed to the entrypoint or ``CMD``). The list must be properly #: escaped, e.g. as created by ``shlex.split``. - extra_launch_args: List[str] = field(default_factory=list) + # extra_launch_args: List[str] = field(default_factory=list) #: List of additional arguments that are passed to the ``CMD`` or #: entrypoint. These arguments are inserted after the :command:`docker/podman @@ -471,44 +487,96 @@ class ContainerBase: #: The arguments must not cause the container to exit early. It must remain #: active in the background, otherwise this library will not function #: properly. - extra_entrypoint_args: List[str] = field(default_factory=list) + # extra_entrypoint_args: List[str] = field(default_factory=list) #: Time for the container to become healthy (the timeout is ignored #: when the container image defines no ``HEALTHCHECK`` or when the timeout #: is below zero). #: When the value is ``None``, then the timeout will be inferred from the #: container image's ``HEALTHCHECK`` directive. - healthcheck_timeout: Optional[timedelta] = None + # healthcheck_timeout: Optional[timedelta] = None #: additional environment variables that should be injected into the #: container - extra_environment_variables: Optional[Dict[str, str]] = None + # extra_environment_variables: Optional[Dict[str, str]] = None #: Indicate whether there must never be more than one running container of #: this type at all times (e.g. because it opens a shared port). - singleton: bool = False + # singleton: bool = False #: forwarded ports of this container - forwarded_ports: List[PortForwarding] = field(default_factory=list) + # forwarded_ports: List[PortForwarding] = field(default_factory=list) #: optional list of volumes that should be mounted in this container - volume_mounts: List[Union[ContainerVolume, BindMount]] = field( - default_factory=list - ) + # volume_mounts: List[Union[ContainerVolume, BindMount]] = field( + # default_factory=list + # ) - _is_local: bool = False + #: optional list of marks applied to this container image under test + # _marks: Collection[Union[MarkDecorator, Mark]] = field( + # default_factory=list + # ) + + # _is_local: bool = False + + def __init__( + self, + url: str = "", + container_id: str = "", + entry_point: EntrypointSelection = EntrypointSelection.AUTO, + custom_entry_point: Optional[str] = None, + extra_launch_args: Optional[List[str]] = None, + extra_entrypoint_args: Optional[List[str]] = None, + healthcheck_timeout: Optional[timedelta] = None, + extra_environment_variables: Optional[Dict[str, str]] = None, + singleton: bool = False, + forwarded_ports: Optional[List[PortForwarding]] = None, + volume_mounts: Optional[ + List[Union[ContainerVolume, BindMount]] + ] = None, + _marks: Optional[Collection[Union[MarkDecorator, Mark]]] = None, + ) -> None: + self.url = url + self.container_id = container_id + self.entry_point = entry_point + self.custom_entry_point = custom_entry_point + self.extra_launch_args = extra_launch_args or [] + self.extra_entrypoint_args = extra_entrypoint_args or [] + self.healthcheck_timeout = healthcheck_timeout + self.extra_environment_variables = extra_environment_variables + self.singleton = singleton + self.forwarded_ports = forwarded_ports or [] + self.volume_mounts = volume_mounts or [] + self._marks = _marks or [] - def __post_init__(self) -> None: local_prefix = "containers-storage:" if self.url.startswith(local_prefix): self._is_local = True # returns before_separator, separator, after_separator before, sep, self.url = self.url.partition(local_prefix) assert before == "" and sep == local_prefix + else: + self._is_local = False + + def __eq__(self, value: object) -> bool: + if not isinstance(value, ContainerBase): + return False + + if set(self.__dict__.keys()) != set(value.__dict__.keys()): + return False + + for k, v in self.__dict__.items(): + if v != value.__dict__[k]: + return False + + return True def __str__(self) -> str: return self.url or self.container_id + def __bool__(self) -> bool: + return True + @property def _build_tag(self) -> str: """Internal build tag assigned to each immage, either the image url or @@ -525,6 +593,18 @@ def local_image(self) -> bool: """ return self._is_local + @property + def marks(self) -> Collection[Union[MarkDecorator, Mark]]: + return self._marks + + @property + def values(self) -> Tuple[Self, ...]: + return (self,) + + @property + def id(self) -> str: + return str(self) + def get_launch_cmd( self, container_runtime: OciRuntimeBase, @@ -630,13 +710,6 @@ def filelock_filename(self) -> str: # that is not available on old python versions that we still support return f"{sha3_256((''.join(all_elements)).encode()).hexdigest()}.lock" - -class ContainerBaseABC(ABC): - """Abstract base class defining the methods that must be implemented by the - classes fed to the ``*container*`` fixtures. - - """ - @abstractmethod def prepare_container( self, @@ -662,8 +735,7 @@ def baseurl(self) -> Optional[str]: """ -@dataclass(unsafe_hash=True) -class Container(ContainerBase, ContainerBaseABC): +class Container(ContainerBase): """This class stores information about the Container Image under test.""" def pull_container(self, container_runtime: OciRuntimeBase) -> None: @@ -702,7 +774,7 @@ def baseurl(self) -> Optional[str]: @dataclass(unsafe_hash=True) -class DerivedContainer(ContainerBase, ContainerBaseABC): +class DerivedContainer(ContainerBase): """Class for storing information about the Container Image under test, that is build from a :file:`Containerfile`/:file:`Dockerfile` from a different image (can be any image from a registry or an instance of @@ -710,11 +782,26 @@ class DerivedContainer(ContainerBase, ContainerBaseABC): """ - base: Union[Container, "DerivedContainer", str] = "" + def __init__( + self, + base: Union[Container, "DerivedContainer", str], + containerfile: str = "", + image_format: Optional[ImageFormat] = None, + add_build_tags: Optional[List[str]] = None, + *args, + **kwargs, + ) -> None: + super().__init__(*args, **kwargs) + self.base = base + self.containerfile = containerfile + self.image_format = image_format + self.add_build_tags = add_build_tags or [] + + # base: Union[Container, "DerivedContainer", str] = "" #: The :file:`Containerfile` that is used to build this container derived #: from :py:attr:`base`. - containerfile: str = "" + # containerfile: str = "" #: An optional image format when building images with :command:`buildah`. It #: is ignored when the container runtime is :command:`docker`. @@ -723,16 +810,28 @@ class DerivedContainer(ContainerBase, ContainerBaseABC): #: ``docker`` image format will be used instead. #: Specifying an image format disables the auto-detection and uses the #: supplied value. - image_format: Optional[ImageFormat] = None + # image_format: Optional[ImageFormat] = None #: Additional build tags/names that should be added to the container once it #: has been built - add_build_tags: List[str] = field(default_factory=list) + # add_build_tags: List[str] = field(default_factory=list) - def __post_init__(self) -> None: - super().__post_init__() - if not self.base: - raise ValueError("A base container must be provided") + @staticmethod + def _get_recursive_marks( + ctr: Union[Container, "DerivedContainer", str] + ) -> Collection[Union[MarkDecorator, Mark]]: + if isinstance(ctr, str): + return [] + if isinstance(ctr, Container): + return ctr._marks + + return tuple(ctr._marks) + tuple( + DerivedContainer._get_recursive_marks(ctr.base) + ) + + @property + def marks(self) -> Collection[Union[MarkDecorator, Mark]]: + return DerivedContainer._get_recursive_marks(self) @property def baseurl(self) -> Optional[str]: diff --git a/pytest_container/plugin.py b/pytest_container/plugin.py index 186597c..afb2299 100644 --- a/pytest_container/plugin.py +++ b/pytest_container/plugin.py @@ -7,10 +7,16 @@ from subprocess import run from typing import Callable from typing import Generator +from typing import Union +from pytest_container.container import Container from pytest_container.container import container_and_marks_from_pytest_param from pytest_container.container import ContainerData from pytest_container.container import ContainerLauncher +from pytest_container.container import DerivedContainer +from pytest_container.helpers import get_extra_build_args +from pytest_container.helpers import get_extra_pod_create_args +from pytest_container.helpers import get_extra_run_args from pytest_container.logging import _logger from pytest_container.pod import pod_from_pytest_param from pytest_container.pod import PodData @@ -74,13 +80,12 @@ def fixture_funct( pytest_generate_tests. """ - try: - container, _ = container_and_marks_from_pytest_param(request.param) - except AttributeError as attr_err: - raise RuntimeError( - "This fixture was not parametrized correctly, " - "did you forget to call `auto_container_parametrize` in `pytest_generate_tests`?" - ) from attr_err + container: Union[DerivedContainer, Container] = ( + request.param + if isinstance(request.param, (DerivedContainer, Container)) + else request.param[0] + ) + assert isinstance(container, (DerivedContainer, Container)) _logger.debug("Requesting the container %s", str(container)) if scope == "session" and container.singleton: diff --git a/pytest_container/pod.py b/pytest_container/pod.py index 4ed9887..6d93095 100644 --- a/pytest_container/pod.py +++ b/pytest_container/pod.py @@ -1,18 +1,28 @@ """Module for managing podman pods.""" import contextlib import json +from abc import ABCMeta from dataclasses import dataclass from dataclasses import field from pathlib import Path from subprocess import check_output from types import TracebackType +from typing import Collection from typing import List from typing import Optional + +try: + from typing import Self +except ImportError: + from typing_extensions import Self +from typing import Tuple from typing import Type from typing import Union from _pytest.mark import ParameterSet from pytest import Config +from pytest import Mark +from pytest import MarkDecorator from pytest_container.container import Container from pytest_container.container import ContainerData from pytest_container.container import ContainerLauncher @@ -28,8 +38,7 @@ from pytest_container.runtime import PodmanRuntime -@dataclass -class Pod: +class Pod(ParameterSet): """A pod is a collection of containers that share the same network and port forwards. Currently only :command:`podman` supports creating pods. @@ -38,11 +47,55 @@ class Pod: """ - #: containers belonging to the pod - containers: List[Union[DerivedContainer, Container]] + def __new__(cls, *args, **kwargs): + # Filter out all fields of ParameterSet and invoke object.__new__ only for the + # fields that it supports + parameter_set_fields = ParameterSet._fields + filtered_kwargs = {} + for f in parameter_set_fields: + filtered_kwargs[f] = kwargs.get(f, None) + + return super().__new__(cls, *args, **filtered_kwargs) + + def __init__( + self, + containers: List[Union[DerivedContainer, Container]], + forwarded_ports: Optional[List[PortForwarding]] = None, + marks: Optional[Collection[Union[MarkDecorator, Mark]]] = None, + ) -> None: + self.containers = containers + self.forwarded_ports = forwarded_ports or [] + self._marks = marks + + # #: containers belonging to the pod + # containers: List[Union[DerivedContainer, Container]] + + # #: ports exposed by the pod + # forwarded_ports: List[PortForwarding] = field(default_factory=list) + + # _marks: Collection[Union[MarkDecorator, Mark]] = field( + # default_factory=list + # ) + + @property + def values(self) -> Tuple[Self]: + return (self,) + + @property + def marks(self) -> Collection[Union[MarkDecorator, Mark]]: + marks = tuple(self._marks) + for ctr in self.containers: + marks += tuple(ctr.marks) + return marks + + @property + def id(self) -> str: + return "Pod with containers: " + ",".join( + str(c) for c in self.containers + ) - #: ports exposed by the pod - forwarded_ports: List[PortForwarding] = field(default_factory=list) + def __bool__(self) -> bool: + return True @dataclass(frozen=True) diff --git a/tests/test_container.py b/tests/test_container.py index a1908c3..eb7a33c 100644 --- a/tests/test_container.py +++ b/tests/test_container.py @@ -14,6 +14,7 @@ from . import images +@pytest.mark.xfail(reason="API change") def test_derived_container_fails_without_base() -> None: """Ensure that a DerivedContainer cannot be instantiated without providing the base parameter. diff --git a/tests/test_marks.py b/tests/test_marks.py new file mode 100644 index 0000000..78a6b60 --- /dev/null +++ b/tests/test_marks.py @@ -0,0 +1,99 @@ +import pytest +from _pytest.mark import ParameterSet +from pytest_container.container import Container +from pytest_container.container import ContainerBase +from pytest_container.container import DerivedContainer +from pytest_container.pod import Pod + +from tests.images import LEAP_URL + +LEAP_WITH_MARK = Container(url=LEAP_URL, _marks=[pytest.mark.secretleapmark]) + +DERIVED_ON_LEAP_WITH_MARK = DerivedContainer(base=LEAP_WITH_MARK) + +SECOND_DERIVED_ON_LEAP = DerivedContainer( + base=DERIVED_ON_LEAP_WITH_MARK, _marks=[pytest.mark.othersecretmark] +) + +INDEPENDENT_OTHER_LEAP = Container( + url=LEAP_URL, _marks=[pytest.mark.othersecretmark] +) + +UNMARKED_POD = Pod(containers=[LEAP_WITH_MARK, INDEPENDENT_OTHER_LEAP]) + +MARKED_POD = Pod( + containers=[LEAP_WITH_MARK, INDEPENDENT_OTHER_LEAP], + marks=[pytest.mark.secretpodmark], +) + + +def test_marks() -> None: + assert list(LEAP_WITH_MARK.marks) == [pytest.mark.secretleapmark] + assert list(DERIVED_ON_LEAP_WITH_MARK.marks) == [ + pytest.mark.secretleapmark + ] + assert list(SECOND_DERIVED_ON_LEAP.marks) == [ + pytest.mark.othersecretmark, + pytest.mark.secretleapmark, + ] + assert not DerivedContainer( + base=LEAP_URL, containerfile="ENV HOME=/root" + ).marks + + pod_marks = UNMARKED_POD.marks + assert ( + len(pod_marks) == 2 + and pytest.mark.othersecretmark in pod_marks + and pytest.mark.secretleapmark in pod_marks + ) + + pod_marks = MARKED_POD.marks + assert ( + len(pod_marks) == 3 + and pytest.mark.othersecretmark in pod_marks + and pytest.mark.secretleapmark in pod_marks + and pytest.mark.secretpodmark in pod_marks + ) + + +@pytest.mark.parametrize( + "ctr", + [ + LEAP_WITH_MARK, + DERIVED_ON_LEAP_WITH_MARK, + SECOND_DERIVED_ON_LEAP, + INDEPENDENT_OTHER_LEAP, + ], +) +def test_container_is_pytest_param(ctr) -> None: + + assert isinstance(ctr, ParameterSet) + assert isinstance(ctr, (Container, DerivedContainer)) + + +@pytest.mark.parametrize( + "ctr", + [ + LEAP_WITH_MARK, + DERIVED_ON_LEAP_WITH_MARK, + SECOND_DERIVED_ON_LEAP, + INDEPENDENT_OTHER_LEAP, + ], +) +def test_container_is_truthy(ctr: ContainerBase) -> None: + """Regression test that we don't accidentally inherit __bool__ from tuple + and the container is False by default. + + """ + assert ctr + + +@pytest.mark.parametrize("pd", [MARKED_POD, UNMARKED_POD]) +def test_pod_is_pytest_param(pd: Pod) -> None: + assert isinstance(pd, ParameterSet) + assert isinstance(pd, Pod) + + +@pytest.mark.parametrize("pd", [MARKED_POD, UNMARKED_POD]) +def test_pod_is_truthy(pd: Pod) -> None: + assert pd