Repository: ClearcodeHQ/pytest-rabbitmq (language: python)

Issue:
RabbitMQ fills the temporary directory – pytest-rabbitmq doesn't clean up after itself
### What action do you want to perform
Repeatedly run a test suite which uses the `rabbitmq_proc` fixture.

### What are the results
RabbitMQ runs out of disk space.

### What are the expected results
RabbitMQ should run fine and not leave huge amounts of data in temporary filesystems.

---

The reason for this seems to be that `pytest-rabbitmq` uses Python's [tempfile.gettempdir()](https://docs.python.org/3/library/tempfile.html#tempfile.gettempdir) to get a directory for the RabbitMQ server to store its data (see [src/pytest_rabbitmq/factories/process.py:98](https://github.com/ClearcodeHQ/pytest-rabbitmq/blob/825537961f692a61bf2ad136801dfafa45458713/src/pytest_rabbitmq/factories/process.py#L98)).

On my system, `gettempdir()` returns `/run/user/1000` which has 3.2 GiB of total space. However, after I've run my test suite for a half dozen times while sometimes debugging for a couple of hours, that filesystem has been filled with `rabbitmq.<pid>` directories. They typically eat 0.5 GiB each.

The problem wasn't easy to spot, and `pytest-rabbitmq` could help by either cleaning up the data directory or letting me know loud and clear that it left lots of data on disk.


Discussion / hints:
Here's the fixture I use as a work-around for this problem:
```python
@pytest.fixture
def rabbitmq_server(rabbitmq_proc):
    tempdir = Path(rabbitmq_proc._envvars["RABBITMQ_MNESIA_BASE"]).parent
    try:
        # `aiopika` uses the `AMQP_PORT` environment variable, patch that:
        with patch.dict(os.environ, {"AMQP_PORT": str(rabbitmq_proc.port)}):
            yield
    finally:
        rmtree(tempdir, ignore_errors=True)
```

I guess it would be more appropriate to somehow make sure `rabbitmq_proc` has first finished, and only then do the clean-up. But that would be easier done in and by `pytest-rabbitmq` itself.
We could attempt to get the directory size and print message I suppose. We can also I think pretty format that message at the end of the tests 🤔 

I'd want to also migrate the temporary path handling to tmp_path_factory for this.

Fix patch:
diff --git a/newsfragments/266.depr.rst b/newsfragments/266.depr.rst
new file mode 100644
index 0000000..9eb7bb1
--- /dev/null
+++ b/newsfragments/266.depr.rst
@@ -0,0 +1,1 @@
+Deprecate `rabbitmq_logsdir` and `--rabbitmq-logsdir` config options.
\ No newline at end of file
diff --git a/newsfragments/266.feature.rst b/newsfragments/266.feature.rst
new file mode 100644
index 0000000..e6f59f5
--- /dev/null
+++ b/newsfragments/266.feature.rst
@@ -0,0 +1,2 @@
+Use `tmp_path_factory` instead of gettempdir() manually.
+This will allow cleaning of a temporary files.
\ No newline at end of file
diff --git a/pyproject.toml b/pyproject.toml
index 8a8784f..5aaca5a 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -85,6 +85,10 @@ issue_format="`#{issue} <https://https://github.com/ClearcodeHQ/pytest-rabbitmq/
 name = "Features"
 showcontent = true
 
+[tool.towncrier.fragment.depr]
+name = "Deprecations"
+showcontent = true
+
 [tool.towncrier.fragment.bugfix]
 name = "Bugfixes"
 showcontent = true


Test patch:
diff --git a/pytest_rabbitmq/factories/executor.py b/pytest_rabbitmq/factories/executor.py
index 1a1087c..3a4e988 100644
--- a/pytest_rabbitmq/factories/executor.py
+++ b/pytest_rabbitmq/factories/executor.py
@@ -1,6 +1,7 @@
 """RabbitMQ Executor."""
 import re
 import subprocess
+from pathlib import Path
 from typing import List, Optional
 
 from mirakuru import TCPExecutor
@@ -17,9 +18,9 @@ def __init__(
         host: str,
         port: int,
         rabbit_ctl: str,
-        logpath: str,
-        path: str,
-        plugin_path: str,
+        logpath: Path,
+        path: Path,
+        plugin_path: Path,
         node_name: Optional[str] = None,
     ) -> None:  # pylint:disable=too-many-arguments
         """Initialize RabbitMQ executor.
@@ -27,16 +28,15 @@ def __init__(
         :param command: rabbitmq-server location
         :param host: host where rabbitmq will be accessible
         :param port: port under which rabbitmq runs
-        :param str rabbit_ctl: rabbitctl location
-        :param str logpath:
-        :param str path: Path containing rabbitmq'a mnesia na plugins
-        :param str node_name: RabbitMQ node name
-        :param kwargs: see TCPExecutor for description
+        :param rabbit_ctl: rabbitctl location
+        :param logpath:
+        :param path: Path containing rabbitmq'a mnesia na plugins
+        :param node_name: RabbitMQ node name
         """
         envvars = {
-            "RABBITMQ_LOG_BASE": logpath + f"/rabbit-server.{port}.log",
-            "RABBITMQ_MNESIA_BASE": path + "mnesia",
-            "RABBITMQ_ENABLED_PLUGINS_FILE": plugin_path + "/plugins",
+            "RABBITMQ_LOG_BASE": str(logpath / f"rabbit-server.{port}.log"),
+            "RABBITMQ_MNESIA_BASE": str(path / "mnesia"),
+            "RABBITMQ_ENABLED_PLUGINS_FILE": str(plugin_path / "plugins"),
             "RABBITMQ_NODE_PORT": str(port),
             # Use the port number in node name, so multiple instances started
             # at different ports will work separately instead of clustering.
diff --git a/pytest_rabbitmq/factories/process.py b/pytest_rabbitmq/factories/process.py
index b12371d..f68f2eb 100644
--- a/pytest_rabbitmq/factories/process.py
+++ b/pytest_rabbitmq/factories/process.py
@@ -17,14 +17,14 @@
 # along with pytest-rabbitmq.  If not, see <http://www.gnu.org/licenses/>.
 """RabbitMQ process fixture factory."""
 
-import os
-from tempfile import gettempdir
+from pathlib import Path
 from typing import Any, Callable, Generator, Optional, TypedDict
+from warnings import warn
 
 import pytest
 from mirakuru.exceptions import ProcessExitedWithError
 from port_for import get_port
-from pytest import FixtureRequest
+from pytest import FixtureRequest, TempPathFactory
 
 from pytest_rabbitmq.factories.executor import RabbitMqExecutor
 
@@ -34,11 +34,11 @@ class RabbitMQConfig(TypedDict):
 
     host: str
     port: Optional[int]
-    logsdir: str
+    logsdir: Optional[Path]
     server: str
     ctl: str
     node: str
-    plugindir: str
+    plugindir: Path
 
 
 def get_config(request: FixtureRequest) -> RabbitMQConfig:
@@ -52,11 +52,11 @@ def get_conf_option(option: str) -> Any:
     config: RabbitMQConfig = {
         "host": get_conf_option("host"),
         "port": int(port) if port else None,
-        "logsdir": get_conf_option("logsdir"),
+        "logsdir": Path(get_conf_option("logsdir")),
         "server": get_conf_option("server"),
         "ctl": get_conf_option("ctl"),
         "node": get_conf_option("node"),
-        "plugindir": get_conf_option("plugindir"),
+        "plugindir": Path(get_conf_option("plugindir")),
     }
     return config
 
@@ -67,31 +67,33 @@ def rabbitmq_proc(
     port: Optional[int] = -1,
     node: Optional[str] = None,
     ctl: Optional[str] = None,
-    logsdir: Optional[str] = None,
-    plugindir: Optional[str] = None,
-) -> Callable[[FixtureRequest], Generator[RabbitMqExecutor, None, None]]:
+    logsdir: Optional[Path] = None,
+    plugindir: Optional[Path] = None,
+) -> Callable[[FixtureRequest, TempPathFactory], Generator[RabbitMqExecutor, None, None]]:
     """Fixture factory for RabbitMQ process.
 
-    :param str server: path to rabbitmq-server command
-    :param str host: server host
-    :param str|int|tuple|set|list port:
+    :param server: path to rabbitmq-server command
+    :param host: server host
+    :param port:
         exact port (e.g. '8000', 8000)
         randomly selected port (None) - any random available port
         [(2000,3000)] or (2000,3000) - random available port from a given range
         [{4002,4003}] or {4002,4003} - random of 4002 or 4003 ports
         [(2000,3000), {4002,4003}] -random of given range and set
-    :param str node: RabbitMQ node name used for setting environment
+    :param node: RabbitMQ node name used for setting environment
                           variable RABBITMQ_NODENAME (the default depends
                           on the port number, so multiple nodes are not
                           clustered)
-    :param str ctl: path to rabbitmqctl file
-    :param str logsdir: path to log directory
+    :param ctl: path to rabbitmqctl file
+    :param logsdir: path to log directory
 
     :returns pytest fixture with RabbitMQ process executor
     """
 
     @pytest.fixture(scope="session")
-    def rabbitmq_proc_fixture(request: FixtureRequest) -> Generator[RabbitMqExecutor, None, None]:
+    def rabbitmq_proc_fixture(
+        request: FixtureRequest, tmp_path_factory: TempPathFactory
+    ) -> Generator[RabbitMqExecutor, None, None]:
         """Fixture for RabbitMQ process.
 
         #. Get config.
@@ -117,11 +119,20 @@ def rabbitmq_proc_fixture(request: FixtureRequest) -> Generator[RabbitMqExecutor
         rabbit_port = get_port(port) or get_port(config["port"])
         assert rabbit_port
 
-        rabbit_path = os.path.join(gettempdir(), f"rabbitmq.{rabbit_port}/")
-        rabbit_plugin_path = plugindir or config["plugindir"] or rabbit_path
+        tmpdir = tmp_path_factory.mktemp(f"pytest-rabbitmq-{request.fixturename}")
+
+        rabbit_plugin_path = plugindir or config["plugindir"]
 
         rabbit_logpath = config["logsdir"] or logsdir
-        assert rabbit_logpath
+        if rabbit_logpath:
+            warn(
+                f"rabbitmq_logsdir and --rabbitmq-logsdir config option is "
+                f"deprecated, and will be dropped in future releases. "
+                f"All fixture related data resides within {tmpdir}",
+                DeprecationWarning,
+            )
+        if not rabbit_logpath:
+            rabbit_logpath = tmpdir / "logs"
 
         rabbit_executor = RabbitMqExecutor(
             rabbit_server,
@@ -129,7 +140,7 @@ def rabbitmq_proc_fixture(request: FixtureRequest) -> Generator[RabbitMqExecutor
             rabbit_port,
             rabbit_ctl,
             logpath=rabbit_logpath,
-            path=rabbit_path,
+            path=tmpdir,
             plugin_path=rabbit_plugin_path,
             node_name=node or config["node"],
         )
diff --git a/pytest_rabbitmq/plugin.py b/pytest_rabbitmq/plugin.py
index d1e43ea..ff09857 100644
--- a/pytest_rabbitmq/plugin.py
+++ b/pytest_rabbitmq/plugin.py
@@ -53,7 +53,7 @@ def pytest_addoption(parser: Parser) -> None:
     parser.addini(
         name="rabbitmq_logsdir",
         help=_help_logsdir,
-        default=gettempdir(),
+        default=None,
     )
     parser.addini(
         name="rabbitmq_plugindir",
diff --git a/tests/conftest.py b/tests/conftest.py
index a14b1e1..44f1007 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -1,4 +1,6 @@
 """Tests main conftest file."""
+from pathlib import Path
+
 from pytest_rabbitmq import factories
 from pytest_rabbitmq.plugin import *  # noqa: F403
 
@@ -9,5 +11,5 @@
 rabbitmq_rand = factories.rabbitmq("rabbitmq_rand_proc")
 rabbitmq_rand_proc2 = factories.rabbitmq_proc(port=None)
 rabbitmq_rand_proc3 = factories.rabbitmq_proc(port=None)
-rabbitmq_plugindir = factories.rabbitmq_proc(plugindir="/etc")
+rabbitmq_plugindir = factories.rabbitmq_proc(plugindir=Path("/etc"))
 # pylint:enable=invalid-name