From 0e7b3f0ae20547773533cb263a850ae87559ff9d Mon Sep 17 00:00:00 2001 From: Balint Bartha Date: Tue, 13 Feb 2024 21:49:19 +0100 Subject: [PATCH 1/3] fix: flaky garbage collection resulting in testing errors --- core/testcontainers/core/container.py | 18 ++++++++++-------- core/testcontainers/core/docker_client.py | 4 +++- core/tests/test_core.py | 13 ++++++++++++- pyproject.toml | 2 +- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/core/testcontainers/core/container.py b/core/testcontainers/core/container.py index c3825b935..1e593a254 100644 --- a/core/testcontainers/core/container.py +++ b/core/testcontainers/core/container.py @@ -23,6 +23,7 @@ class DockerContainer: >>> with DockerContainer("hello-world") as container: ... delay = wait_for_logs(container, "Hello from Docker!") """ + def __init__(self, image: str, docker_client_kw: Optional[dict] = None, **kwargs) -> None: self.env = {} self.ports = {} @@ -42,7 +43,7 @@ def with_bind_ports(self, container: int, host: int = None) -> 'DockerContainer' self.ports[container] = host return self - def with_exposed_ports(self, *ports: Iterable[int]) -> 'DockerContainer': + def with_exposed_ports(self, *ports: int) -> 'DockerContainer': for port in ports: self.ports[port] = None return self @@ -67,7 +68,7 @@ def start(self) -> 'DockerContainer': return self def stop(self, force=True, delete_volume=True) -> None: - self.get_wrapped_container().remove(force=force, v=delete_volume) + self._container.remove(force=force, v=delete_volume) def __enter__(self) -> 'DockerContainer': return self.start() @@ -77,13 +78,14 @@ def __exit__(self, exc_type, exc_val, exc_tb) -> None: def __del__(self) -> None: """ - Try to remove the container in all circumstances + __del__ runs when Python attempts to garbage collect the object. + In case of leaky test design, we still attempt to clean up the container. """ - if self._container is not None: - try: + try: + if self._container is not None: self.stop() - except: # noqa: E722 - pass + finally: + pass def get_container_host_ip(self) -> str: # infer from docker host @@ -143,4 +145,4 @@ def get_logs(self) -> Tuple[str, str]: def exec(self, command) -> Tuple[int, str]: if not self._container: raise ContainerStartException("Container should be started before executing a command") - return self.get_wrapped_container().exec_run(command) + return self._container.exec_run(command) diff --git a/core/testcontainers/core/docker_client.py b/core/testcontainers/core/docker_client.py index 228bfd1c1..086205844 100644 --- a/core/testcontainers/core/docker_client.py +++ b/core/testcontainers/core/docker_client.py @@ -39,6 +39,7 @@ class DockerClient: """ Thin wrapper around :class:`docker.DockerClient` for a more functional interface. """ + def __init__(self, **kwargs) -> None: self.client = docker.from_env(**kwargs) @@ -46,7 +47,8 @@ def __init__(self, **kwargs) -> None: def run(self, image: str, command: Union[str, List[str]] = None, environment: Optional[dict] = None, ports: Optional[dict] = None, detach: bool = False, stdout: bool = True, stderr: bool = False, remove: bool = False, - **kwargs) -> Container: + **kwargs + ) -> Container: container = self.client.containers.run( image, command=command, stdout=stdout, stderr=stderr, remove=remove, detach=detach, environment=environment, ports=ports, **kwargs diff --git a/core/tests/test_core.py b/core/tests/test_core.py index 5a6663502..b578a7bee 100644 --- a/core/tests/test_core.py +++ b/core/tests/test_core.py @@ -1,15 +1,26 @@ import pytest from testcontainers.core.container import DockerContainer +from testcontainers.core.exceptions import ContainerStartException from testcontainers.core.waiting_utils import wait_for_logs -def test_raise_timeout(): +def test_timeout_is_raised_when_waiting_for_logs(): with pytest.raises(TimeoutError): with DockerContainer("alpine").with_command("sleep 2") as container: wait_for_logs(container, "Hello from Docker!", timeout=1e-3) +def test_garbage_collection_is_defensive(): + # For more info, see https://github.com/testcontainers/testcontainers-python/issues/399 + # we simulate garbage collection: start, stop, then call `del` + container = DockerContainer("postgres:latest") + container.start() + container.stop(force=True, delete_volume=True) + delattr(container, "_container") + del container + + def test_wait_for_hello(): with DockerContainer("hello-world") as container: wait_for_logs(container, "Hello from Docker!") diff --git a/pyproject.toml b/pyproject.toml index 9bc87dfeb..9139e3b25 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -123,7 +123,7 @@ priority = "primary" line-length = 120 [tool.pytest.ini_options] -addopts = "--cov-report=term --tb=short --strict-markers" +addopts = "--cov-report=term --cov-report=html --tb=short --strict-markers" log_cli = true log_cli_level = "INFO" From ff85a49551c347161112defbdc554da6e37cb576 Mon Sep 17 00:00:00 2001 From: Balint Bartha Date: Tue, 13 Feb 2024 21:54:14 +0100 Subject: [PATCH 2/3] fix: linter --- core/tests/test_core.py | 1 - 1 file changed, 1 deletion(-) diff --git a/core/tests/test_core.py b/core/tests/test_core.py index b578a7bee..01e9c97c8 100644 --- a/core/tests/test_core.py +++ b/core/tests/test_core.py @@ -1,7 +1,6 @@ import pytest from testcontainers.core.container import DockerContainer -from testcontainers.core.exceptions import ContainerStartException from testcontainers.core.waiting_utils import wait_for_logs From f77e0e7da847d3ba5c021a2b8594e6a9c347ab2e Mon Sep 17 00:00:00 2001 From: Balint Bartha Date: Tue, 13 Feb 2024 21:56:09 +0100 Subject: [PATCH 3/3] fix: more linting issues --- core/testcontainers/core/container.py | 2 +- core/testcontainers/core/docker_client.py | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/core/testcontainers/core/container.py b/core/testcontainers/core/container.py index 1e593a254..4caed7e3c 100644 --- a/core/testcontainers/core/container.py +++ b/core/testcontainers/core/container.py @@ -1,5 +1,5 @@ import os -from typing import Iterable, Optional, Tuple +from typing import Optional, Tuple from docker.models.containers import Container diff --git a/core/testcontainers/core/docker_client.py b/core/testcontainers/core/docker_client.py index 086205844..fb54838fa 100644 --- a/core/testcontainers/core/docker_client.py +++ b/core/testcontainers/core/docker_client.py @@ -44,9 +44,15 @@ def __init__(self, **kwargs) -> None: self.client = docker.from_env(**kwargs) @ft.wraps(ContainerCollection.run) - def run(self, image: str, command: Union[str, List[str]] = None, - environment: Optional[dict] = None, ports: Optional[dict] = None, - detach: bool = False, stdout: bool = True, stderr: bool = False, remove: bool = False, + def run( + self, image: str, + command: Union[str, List[str]] = None, + environment: Optional[dict] = None, + ports: Optional[dict] = None, + detach: bool = False, + stdout: bool = True, + stderr: bool = False, + remove: bool = False, **kwargs ) -> Container: container = self.client.containers.run(