From b9c8245c49e077120bd7817ac0109dec9274f4cd Mon Sep 17 00:00:00 2001
From: Broque Thomas <26755000+Nezreka@users.noreply.github.com>
Date: Thu, 30 Apr 2026 14:07:00 -0700
Subject: [PATCH] Stop config retry tests from writing to the real DB
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The fixture used the wrong env var name (SOULSYNC_DB_PATH) when trying
to redirect ConfigManager at a tmp directory. ConfigManager actually
reads DATABASE_PATH (config/settings.py:49), so the test ConfigManager
loaded — and then saved — at the user's real database/music_library.db.
The retry stub in test_lock_errors_during_retries_log_at_debug_not_error
calls the real _save_to_database after its mocked failures, which then
clobbered the encrypted app_config row with the test fixture's stub
payload {"plex": {"base_url": "http://example.test"}}.

Three layers of fix so this can't happen again:
- Use the correct env var (DATABASE_PATH).
- Pin mgr.database_path / mgr.config_path on the instance after
  construction, so the test fixture's tmp paths win even if
  ConfigManager's resolution logic changes.
- Assert the resolved database_path is rooted under tmp_path before
  returning the fixture, so the test refuses to run if it would touch
  a non-tmp DB.
---
 tests/test_config_save_retry.py | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/tests/test_config_save_retry.py b/tests/test_config_save_retry.py
index 7214849e..de8c1268 100644
--- a/tests/test_config_save_retry.py
+++ b/tests/test_config_save_retry.py
@@ -24,14 +24,29 @@ from config.settings import ConfigManager
 
 @pytest.fixture
 def manager(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> ConfigManager:
-    """Build a ConfigManager rooted at a tmp dir so every test starts clean."""
+    """Build a ConfigManager rooted at a tmp dir so every test starts clean.
+
+    CRITICAL: ConfigManager reads ``DATABASE_PATH`` (not ``SOULSYNC_DB_PATH``)
+    when picking the DB location. Setting the wrong env var here would let
+    tests reach the real ``database/music_library.db`` and clobber the
+    user's encrypted credentials. The ``database_path`` is also pinned
+    directly on the instance after construction as a defense-in-depth check
+    in case ConfigManager's resolution logic ever changes.
+    """
     config_path = tmp_path / "config.json"
     db_path = tmp_path / "database" / "music_library.db"
     monkeypatch.setenv("SOULSYNC_CONFIG_PATH", str(config_path))
-    monkeypatch.setenv("SOULSYNC_DB_PATH", str(db_path))
+    monkeypatch.setenv("DATABASE_PATH", str(db_path))
     mgr = ConfigManager(str(config_path))
+    # Defense-in-depth: pin the path on the instance so even if ConfigManager
+    # ignored the env var, the DB writes still land in the tmp directory.
+    mgr.database_path = db_path
+    mgr.config_path = config_path
     # Replace whatever was loaded with a known payload so we can assert on it
     mgr.config_data = {"plex": {"base_url": "http://example.test"}}
+    assert str(mgr.database_path).startswith(str(tmp_path)), (
+        "Test fixture would write to a non-tmp DB — refusing to run"
+    )
     return mgr