fix(tests): stop conftest leaking temp dirs and being imported twice (#1559)

## Why

A full test run left ~12.4k abandoned directories in `/tmp` and,
separately, silently disabled the per-test isolation `tests/conftest.py`
promises. Both traced back to the same file.

On this machine the leak reached 973k entries in two days of uptime and
exhausted the tmpfs inode table — `mkdir` then fails with `ENOSPC` while
`df` still reports 5 GB free, which breaks every tool that needs a temp
file. `/tmp` is only aged at boot (`q! /tmp … 10d`), so nothing reclaims
it in between.

## What changed

**Temp dirs are released again.** `_reset_decky_mock_paths` created two
`tempfile.mkdtemp()` directories per test and never removed them; three
more leaked per process at import time. `mkdtemp` leaves the directory
behind by design. The per-test pair is now dropped on teardown, the
import-time trio via `atexit` — a session-scoped fixture would not fire
for a run that collects no tests.

**`conftest.py` is no longer imported twice.** It puts `tests/` on
`sys.path`, so `from conftest import ...` in 13 modules resolved to a
*second* module object beside pytest's `tests.conftest`. Both ran the
module body, and the later one won `sys.modules["decky"] = mock_decky`.
Since fixtures are registered only from pytest's copy, the autouse
fixture refreshed a mock nothing observed, while production code reading
`decky.DECKY_PLUGIN_SETTINGS_DIR` saw the shadow's import-time directory
for the whole session:

```
tests/adapters alone            -> fresh dir per test
the same file + tests/services  -> one shared dir for every test
```

Behaviour that depends on the test selection. Nothing relied on it yet —
the only test reaching `Plugin._main()` patches `main.bootstrap`, so the
paths never drive real I/O — so this was a trap waiting for the first
persistence test, not an active failure. The shared constructors moved
to `tests/_factories.py`, matching the helper modules the suite already
uses (`tests/contract/_harness.py`, `tests/services/saves/_helpers.py`).

Switching to `prepend` import mode would also fix the root cause, but is
unavailable: 17 test basenames repeat across directories and only 6 sit
in packages.

**Dead config removed.** `pytest.ini` and `[tool.pytest.ini_options]`
both declared `asyncio_mode`; `pytest.ini` wins, so the pyproject block
never took effect.

## Not moved to `tmp_path`

Deliberate. Adapter tests assert on the exact contents of their
`tmp_path` (`listdir(...) == []`), and `tests/contract/_harness.py`
already owns `tmp_path/settings` and `tmp_path/runtime`. `tmp_path`
would be the idiomatic choice in the abstract and the wrong one here.

## Verification

- 6210 Python tests pass; the now-*effective* isolation breaks none of
them, confirming the defect was latent
- `/tmp` delta is 0 after a full run, a single file, a subset with
direct conftest imports, and a path collecting no tests (each previously
leaked)
- `mise run gate` green end to end

docs: N/A — test infrastructure and tooling only; no architecture,
data-flow, or user-facing behaviour change.

Author: danielcopper

pyproject.toml

@@ -63,6 +63,3 @@ select = [
 [tool.ruff.lint.isort]
 known-first-party = ["lib", "domain", "services", "adapters"]
 known-third-party = ["_vendor", "decky"]
-
-[tool.pytest.ini_options]
-asyncio_mode = "auto"

tests/_factories.py

@@ -0,0 +1,83 @@
+"""Construction helpers shared across the suite.
+
+These live outside ``conftest.py`` on purpose. A conftest is imported by
+pytest under its own module name; importing it a second time by plain name
+(`from conftest import ...`) creates a *separate* module object that re-runs
+the module body — including ``sys.modules["decky"] = mock_decky``, which
+would then shadow the mock whose paths the autouse fixtures refresh.
+Anything a test module needs to import belongs here instead.
+
+Import as ``from _factories import _make_retry`` — ``tests/`` is on the path
+via the root conftest, the same way ``fakes/`` is reached.
+"""
+
+from typing import Any
+from unittest.mock import MagicMock
+
+
+def _no_retry(fn, *a, **kw):
+    """Pass-through Retry side_effect: invoke the wrapped callable once, no backoff."""
+    return fn(*a, **kw)
+
+
+def _make_retry():
+    """Build a Retry ``MagicMock`` that runs ``with_retry`` callables exactly once
+    and reports every exception as non-retryable. Used everywhere services
+    take a ``Retry`` Protocol injection in tests."""
+    retry = MagicMock()
+    retry.with_retry.side_effect = _no_retry
+    retry.is_retryable.return_value = False
+    return retry
+
+
+def _make_testable_plugin():
+    """Return a TestablePlugin instance with test-only attributes declared.
+
+    Pre-populates ``_migration_service`` with a non-pending MagicMock so the
+    ``@migration_blocked`` decorator passes through (it requires the service and
+    raises RuntimeError if it is unwired) in tests that don't otherwise wire
+    migration state. Tests that exercise the block can override
+    ``is_retrodeck_migration_pending`` per-test.
+
+    Also pre-wires a no-op ``_debug_logger`` so any service that consumes
+    ``Plugin._log_debug`` (which forwards through ``_debug_logger``) works
+    out of the box. Tests that want to assert on debug-log behaviour can
+    override ``_debug_logger`` after construction (e.g. with the real
+    ``SettingsAwareDebugLogger`` bound to a settings dict they control).
+    """
+    # Import here to ensure decky mock is already installed
+    from main import Plugin
+
+    class TestablePlugin(Plugin):
+        """Plugin subclass that declares test-only attributes for type safety.
+
+        Genuinely test-fixture-only attributes live here: ``_fake_api``,
+        ``_resolve_system``, ``_save_settings``, plus the Unit-of-Work
+        handles tests seed and assert against (``_uow``, ``_uow_factory``)
+        and the per-test ``_tmp_path`` scratch dir. Test-fixture handles
+        shared with production wiring (``_state``, ``_http_adapter``, ...)
+        are declared on ``Plugin`` itself as ``Any``-typed annotation slots
+        so test-only construction paths type-check uniformly.
+        ``_save_settings`` is a test-only handle for the settings dict tests
+        thread into ``SaveService`` / ``PlaytimeService``; production threads
+        its settings store as ``self.settings``, never under this name.
+        """
+
+        _fake_api: Any
+        _resolve_system: Any
+        _save_settings: Any
+        _uow: Any
+        _uow_factory: Any
+        _tmp_path: Any
+        _core_info: Any
+        _platform_core_reader: Any
+        _active_core: Any
+        _m3u_supported: Any
+        _renderer_rss: Any
+        _renderer_gc: Any
+
+    instance = TestablePlugin()
+    instance._migration_service = MagicMock()
+    instance._migration_service.is_retrodeck_migration_pending.return_value = False
+    instance._debug_logger = lambda msg: None
+    return instance

tests/conftest.py

@@ -1,8 +1,9 @@
+import atexit
 import logging
 import os
+import shutil
 import sys
 import tempfile
-from typing import Any
 from unittest.mock import AsyncMock, MagicMock
 
 import pytest
@@ -27,87 +28,42 @@
 sys.path.insert(0, _tests_root)
 
 
+# Import-time temp dirs, so the mock module is complete before anything imports
+# main. The settings/runtime pair is replaced per test by `_reset_decky_mock_paths`;
+# the log dir lives for the whole process.
+_process_settings_dir = tempfile.mkdtemp()
+_process_runtime_dir = tempfile.mkdtemp()
+_process_log_dir = tempfile.mkdtemp()
+
+
+@atexit.register
+def _cleanup_process_temp_dirs() -> None:
+    """Drop the import-time temp dirs — ``mkdtemp`` never cleans up after itself.
+
+    Deliberately ``atexit`` rather than a session-scoped fixture: a fixture
+    only tears down once at least one test is collected, so a run that
+    collects nothing — pointing pytest at a directory that holds only
+    fixtures or vectors — would leak all three. atexit also stays correct
+    if this module is ever executed more than once, since each execution
+    releases exactly what it created.
+    """
+    for path in (_process_settings_dir, _process_runtime_dir, _process_log_dir):
+        shutil.rmtree(path, ignore_errors=True)
+
+
 # Create mock decky module before any imports of main
 mock_decky = MagicMock()
 mock_decky.DECKY_PLUGIN_DIR = _project_root
-mock_decky.DECKY_PLUGIN_SETTINGS_DIR = tempfile.mkdtemp()
-mock_decky.DECKY_PLUGIN_RUNTIME_DIR = tempfile.mkdtemp()
-mock_decky.DECKY_PLUGIN_LOG_DIR = tempfile.mkdtemp()
+mock_decky.DECKY_PLUGIN_SETTINGS_DIR = _process_settings_dir
+mock_decky.DECKY_PLUGIN_RUNTIME_DIR = _process_runtime_dir
+mock_decky.DECKY_PLUGIN_LOG_DIR = _process_log_dir
 mock_decky.DECKY_USER_HOME = os.path.expanduser("~")
 mock_decky.logger = logging.getLogger("test_romm")
 mock_decky.emit = AsyncMock()
 
 sys.modules["decky"] = mock_decky
 
 
-def _no_retry(fn, *a, **kw):
-    """Pass-through Retry side_effect: invoke the wrapped callable once, no backoff."""
-    return fn(*a, **kw)
-
-
-def _make_retry():
-    """Build a Retry ``MagicMock`` that runs ``with_retry`` callables exactly once
-    and reports every exception as non-retryable. Used everywhere services
-    take a ``Retry`` Protocol injection in tests."""
-    retry = MagicMock()
-    retry.with_retry.side_effect = _no_retry
-    retry.is_retryable.return_value = False
-    return retry
-
-
-def _make_testable_plugin():
-    """Return a TestablePlugin instance with test-only attributes declared.
-
-    Pre-populates ``_migration_service`` with a non-pending MagicMock so the
-    ``@migration_blocked`` decorator passes through (it requires the service and
-    raises RuntimeError if it is unwired) in tests that don't otherwise wire
-    migration state. Tests that exercise the block can override
-    ``is_retrodeck_migration_pending`` per-test.
-
-    Also pre-wires a no-op ``_debug_logger`` so any service that consumes
-    ``Plugin._log_debug`` (which forwards through ``_debug_logger``) works
-    out of the box. Tests that want to assert on debug-log behaviour can
-    override ``_debug_logger`` after construction (e.g. with the real
-    ``SettingsAwareDebugLogger`` bound to a settings dict they control).
-    """
-    # Import here to ensure decky mock is already installed
-    from main import Plugin
-
-    class TestablePlugin(Plugin):
-        """Plugin subclass that declares test-only attributes for type safety.
-
-        Genuinely test-fixture-only attributes live here: ``_fake_api``,
-        ``_resolve_system``, ``_save_settings``, plus the Unit-of-Work
-        handles tests seed and assert against (``_uow``, ``_uow_factory``)
-        and the per-test ``_tmp_path`` scratch dir. Test-fixture handles
-        shared with production wiring (``_state``, ``_http_adapter``, ...)
-        are declared on ``Plugin`` itself as ``Any``-typed annotation slots
-        so test-only construction paths type-check uniformly.
-        ``_save_settings`` is a test-only handle for the settings dict tests
-        thread into ``SaveService`` / ``PlaytimeService``; production threads
-        its settings store as ``self.settings``, never under this name.
-        """
-
-        _fake_api: Any
-        _resolve_system: Any
-        _save_settings: Any
-        _uow: Any
-        _uow_factory: Any
-        _tmp_path: Any
-        _core_info: Any
-        _platform_core_reader: Any
-        _active_core: Any
-        _m3u_supported: Any
-        _renderer_rss: Any
-        _renderer_gc: Any
-
-    instance = TestablePlugin()
-    instance._migration_service = MagicMock()
-    instance._migration_service.is_retrodeck_migration_pending.return_value = False
-    instance._debug_logger = lambda msg: None
-    return instance
-
-
 @pytest.fixture
 def fake_romm_api():
     """Function-scoped ``FakeRommApi`` instance.
@@ -141,10 +97,21 @@ def _reset_decky_mock_paths():
 
     Fresh ``DECKY_PLUGIN_SETTINGS_DIR`` and ``DECKY_PLUGIN_RUNTIME_DIR``
     per test prevents cross-test pollution from persistence-touching
-    tests.
+    tests. Both are removed again on teardown — ``mkdtemp`` leaves the
+    directory behind by design, and two leaked dirs per test across a
+    suite this size exhaust the tmpfs inode table in days, not months.
+
+    They deliberately do not live under ``tmp_path``: adapter tests assert
+    on the exact contents of their ``tmp_path`` (``listdir(...) == []``),
+    and ``tests/contract`` already owns ``tmp_path/settings`` and
+    ``tmp_path/runtime``.
     """
     mock_decky.DECKY_USER_HOME = os.path.expanduser("~")
     mock_decky.DECKY_PLUGIN_DIR = _project_root
-    mock_decky.DECKY_PLUGIN_SETTINGS_DIR = tempfile.mkdtemp()
-    mock_decky.DECKY_PLUGIN_RUNTIME_DIR = tempfile.mkdtemp()
+    settings_dir = tempfile.mkdtemp()
+    runtime_dir = tempfile.mkdtemp()
+    mock_decky.DECKY_PLUGIN_SETTINGS_DIR = settings_dir
+    mock_decky.DECKY_PLUGIN_RUNTIME_DIR = runtime_dir
     yield
+    shutil.rmtree(settings_dir, ignore_errors=True)
+    shutil.rmtree(runtime_dir, ignore_errors=True)

tests/services/library/conftest.py

@@ -15,7 +15,7 @@
 import pytest
 
 # conftest.py patches decky before this import; use _make_testable_plugin for test-only attrs
-from conftest import _make_testable_plugin
+from _factories import _make_testable_plugin
 from fakes.fake_core_info_provider import FakeCoreInfoProvider
 from fakes.fake_disc_resolver import FakeDiscResolver
 from fakes.fake_platform_core_reader import FakePlatformCoreReader

tests/services/saves/_helpers.py

@@ -6,7 +6,7 @@
 from datetime import UTC, datetime
 from typing import Any
 
-from conftest import _make_retry
+from _factories import _make_retry
 from fakes.fake_active_core_resolver import FakeActiveCoreResolver
 from fakes.fake_hostname_reader import FakeHostnameReader
 from fakes.fake_machine_id_reader import FakeMachineIdReader

tests/services/saves/sync_engine/test_devices.py

@@ -8,7 +8,7 @@
 import logging
 
 import pytest
-from conftest import _make_retry
+from _factories import _make_retry
 from fakes.fake_hostname_reader import FakeHostnameReader
 from fakes.fake_machine_id_reader import FakeMachineIdReader
 from fakes.fake_save_api import FakeSaveApi

tests/services/test_achievements.py

@@ -5,7 +5,7 @@
 import pytest
 
 # conftest.py patches decky before this import; use _make_testable_plugin for test-only attrs
-from conftest import _make_testable_plugin
+from _factories import _make_testable_plugin
 from fakes.fake_active_core_resolver import FakeActiveCoreResolver
 from fakes.fake_disc_resolver import FakeDiscResolver
 from fakes.fake_renderer_gc import FakeRendererGc

tests/services/test_downloads.py

@@ -8,7 +8,7 @@
 import pytest
 
 # conftest.py patches decky before this import; use _make_testable_plugin for test-only attrs
-from conftest import _make_testable_plugin
+from _factories import _make_testable_plugin
 from fakes.fake_core_info_provider import FakeCoreInfoProvider
 from fakes.fake_disc_resolver import FakeDiscResolver
 from fakes.fake_platform_core_reader import FakePlatformCoreReader

tests/services/test_firmware.py

@@ -8,7 +8,7 @@
 import pytest
 
 # conftest.py patches decky before this import; use _make_testable_plugin for test-only attrs
-from conftest import _make_testable_plugin
+from _factories import _make_testable_plugin
 from fakes.fake_active_core_resolver import FakeActiveCoreResolver
 from fakes.fake_core_info_provider import FakeCoreInfoProvider
 from fakes.fake_disc_resolver import FakeDiscResolver

tests/services/test_game_detail.py

@@ -6,7 +6,7 @@
 import pytest
 
 # conftest.py patches decky before this import; use _make_testable_plugin for test-only attrs
-from conftest import _make_retry, _make_testable_plugin
+from _factories import _make_retry, _make_testable_plugin
 from fakes.fake_active_core_resolver import FakeActiveCoreResolver
 from fakes.fake_core_info_provider import FakeCoreInfoProvider
 from fakes.fake_disc_resolver import FakeDiscResolver