Skip to content

Commit b535ea2

Browse files
authored
fix: flaky garbage collection resulting in testing errors (#423)
# change Fixes #399. Applied a bit of defensive coding and attempted to create some tests for it, however reproducing it with a local dev machine is not easy. I did my best to reproduce the issue with garbage collection in the new test. --------- Co-authored-by: Balint Bartha <[email protected]>
1 parent 3271357 commit b535ea2

File tree

4 files changed

+35
-15
lines changed

4 files changed

+35
-15
lines changed

core/testcontainers/core/container.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import os
2-
from typing import Iterable, Optional, Tuple
2+
from typing import Optional, Tuple
33

44
from docker.models.containers import Container
55

@@ -23,6 +23,7 @@ class DockerContainer:
2323
>>> with DockerContainer("hello-world") as container:
2424
... delay = wait_for_logs(container, "Hello from Docker!")
2525
"""
26+
2627
def __init__(self, image: str, docker_client_kw: Optional[dict] = None, **kwargs) -> None:
2728
self.env = {}
2829
self.ports = {}
@@ -42,7 +43,7 @@ def with_bind_ports(self, container: int, host: int = None) -> 'DockerContainer'
4243
self.ports[container] = host
4344
return self
4445

45-
def with_exposed_ports(self, *ports: Iterable[int]) -> 'DockerContainer':
46+
def with_exposed_ports(self, *ports: int) -> 'DockerContainer':
4647
for port in ports:
4748
self.ports[port] = None
4849
return self
@@ -67,7 +68,7 @@ def start(self) -> 'DockerContainer':
6768
return self
6869

6970
def stop(self, force=True, delete_volume=True) -> None:
70-
self.get_wrapped_container().remove(force=force, v=delete_volume)
71+
self._container.remove(force=force, v=delete_volume)
7172

7273
def __enter__(self) -> 'DockerContainer':
7374
return self.start()
@@ -77,13 +78,14 @@ def __exit__(self, exc_type, exc_val, exc_tb) -> None:
7778

7879
def __del__(self) -> None:
7980
"""
80-
Try to remove the container in all circumstances
81+
__del__ runs when Python attempts to garbage collect the object.
82+
In case of leaky test design, we still attempt to clean up the container.
8183
"""
82-
if self._container is not None:
83-
try:
84+
try:
85+
if self._container is not None:
8486
self.stop()
85-
except: # noqa: E722
86-
pass
87+
finally:
88+
pass
8789

8890
def get_container_host_ip(self) -> str:
8991
# infer from docker host
@@ -143,4 +145,4 @@ def get_logs(self) -> Tuple[str, str]:
143145
def exec(self, command) -> Tuple[int, str]:
144146
if not self._container:
145147
raise ContainerStartException("Container should be started before executing a command")
146-
return self.get_wrapped_container().exec_run(command)
148+
return self._container.exec_run(command)

core/testcontainers/core/docker_client.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,22 @@ class DockerClient:
3939
"""
4040
Thin wrapper around :class:`docker.DockerClient` for a more functional interface.
4141
"""
42+
4243
def __init__(self, **kwargs) -> None:
4344
self.client = docker.from_env(**kwargs)
4445

4546
@ft.wraps(ContainerCollection.run)
46-
def run(self, image: str, command: Union[str, List[str]] = None,
47-
environment: Optional[dict] = None, ports: Optional[dict] = None,
48-
detach: bool = False, stdout: bool = True, stderr: bool = False, remove: bool = False,
49-
**kwargs) -> Container:
47+
def run(
48+
self, image: str,
49+
command: Union[str, List[str]] = None,
50+
environment: Optional[dict] = None,
51+
ports: Optional[dict] = None,
52+
detach: bool = False,
53+
stdout: bool = True,
54+
stderr: bool = False,
55+
remove: bool = False,
56+
**kwargs
57+
) -> Container:
5058
container = self.client.containers.run(
5159
image, command=command, stdout=stdout, stderr=stderr, remove=remove, detach=detach,
5260
environment=environment, ports=ports, **kwargs

core/tests/test_core.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,22 @@
44
from testcontainers.core.waiting_utils import wait_for_logs
55

66

7-
def test_raise_timeout():
7+
def test_timeout_is_raised_when_waiting_for_logs():
88
with pytest.raises(TimeoutError):
99
with DockerContainer("alpine").with_command("sleep 2") as container:
1010
wait_for_logs(container, "Hello from Docker!", timeout=1e-3)
1111

1212

13+
def test_garbage_collection_is_defensive():
14+
# For more info, see https://github.com/testcontainers/testcontainers-python/issues/399
15+
# we simulate garbage collection: start, stop, then call `del`
16+
container = DockerContainer("postgres:latest")
17+
container.start()
18+
container.stop(force=True, delete_volume=True)
19+
delattr(container, "_container")
20+
del container
21+
22+
1323
def test_wait_for_hello():
1424
with DockerContainer("hello-world") as container:
1525
wait_for_logs(container, "Hello from Docker!")

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ priority = "primary"
123123
line-length = 120
124124

125125
[tool.pytest.ini_options]
126-
addopts = "--cov-report=term --tb=short --strict-markers"
126+
addopts = "--cov-report=term --cov-report=html --tb=short --strict-markers"
127127
log_cli = true
128128
log_cli_level = "INFO"
129129

0 commit comments

Comments
 (0)