From 8e9a0c367a31c5cce86368cfcd7a18527ceef268 Mon Sep 17 00:00:00 2001
From: ZdenekSrotyr <zdenek.srotyr@keboola.com>
Date: Thu, 9 Apr 2026 07:11:36 +0200
Subject: [PATCH] fix: replace os.environ direct assignment with
 monkeypatch.setenv in test fixtures

Prevents environment variable leaking between tests. All DATA_DIR,
JWT_SECRET_KEY, and SCRIPT_TIMEOUT assignments in fixtures now use
monkeypatch.setenv() which auto-reverts after each test. Removes
manual os.environ.pop() cleanup lines.
---
 tests/test_api.py            | 12 ++++++------
 tests/test_api_complete.py   |  6 +++---
 tests/test_api_scripts.py    |  8 ++++----
 tests/test_auth_providers.py |  6 +++---
 tests/test_bootstrap.py      | 12 ++++++------
 tests/test_db.py             | 24 ++++++++++++------------
 tests/test_migration.py      |  8 ++++----
 tests/test_orchestrator.py   |  6 ++----
 tests/test_permissions.py    |  4 ++--
 tests/test_rbac.py           |  4 ++--
 tests/test_repositories.py   |  4 ++--
 tests/test_security.py       |  8 ++++----
 12 files changed, 50 insertions(+), 52 deletions(-)

diff --git a/tests/test_api.py b/tests/test_api.py
index 4c8ee07..973f0af 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -6,19 +6,19 @@ from fastapi.testclient import TestClient
 
 
 @pytest.fixture
-def app_client(tmp_path):
-    os.environ["DATA_DIR"] = str(tmp_path)
-    os.environ["JWT_SECRET_KEY"] = "test-secret"
+def app_client(tmp_path, monkeypatch):
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
+    monkeypatch.setenv("JWT_SECRET_KEY", "test-secret")
     from app.main import create_app
     app = create_app()
     return TestClient(app)
 
 
 @pytest.fixture
-def seeded_client(tmp_path):
+def seeded_client(tmp_path, monkeypatch):
     """Client with a pre-created admin user and JWT token."""
-    os.environ["DATA_DIR"] = str(tmp_path)
-    os.environ["JWT_SECRET_KEY"] = "test-secret"
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
+    monkeypatch.setenv("JWT_SECRET_KEY", "test-secret")
     from app.main import create_app
     from src.db import get_system_db
     from src.repositories.users import UserRepository
diff --git a/tests/test_api_complete.py b/tests/test_api_complete.py
index 315a935..04979a3 100644
--- a/tests/test_api_complete.py
+++ b/tests/test_api_complete.py
@@ -6,9 +6,9 @@ from fastapi.testclient import TestClient
 
 
 @pytest.fixture
-def client(tmp_path):
-    os.environ["DATA_DIR"] = str(tmp_path)
-    os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
+def client(tmp_path, monkeypatch):
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
+    monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
 
     from app.main import create_app
     from src.db import get_system_db
diff --git a/tests/test_api_scripts.py b/tests/test_api_scripts.py
index 2bdeaba..f52c1ef 100644
--- a/tests/test_api_scripts.py
+++ b/tests/test_api_scripts.py
@@ -6,10 +6,10 @@ from fastapi.testclient import TestClient
 
 
 @pytest.fixture
-def client(tmp_path):
-    os.environ["DATA_DIR"] = str(tmp_path)
-    os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
-    os.environ["SCRIPT_TIMEOUT"] = "10"
+def client(tmp_path, monkeypatch):
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
+    monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
+    monkeypatch.setenv("SCRIPT_TIMEOUT", "10")
 
     from app.main import create_app
     from src.db import get_system_db
diff --git a/tests/test_auth_providers.py b/tests/test_auth_providers.py
index d160177..be4f43f 100644
--- a/tests/test_auth_providers.py
+++ b/tests/test_auth_providers.py
@@ -6,9 +6,9 @@ from fastapi.testclient import TestClient
 
 
 @pytest.fixture
-def client(tmp_path):
-    os.environ["DATA_DIR"] = str(tmp_path)
-    os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
+def client(tmp_path, monkeypatch):
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
+    monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
 
     from app.main import create_app
     from src.db import get_system_db
diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py
index 6dfa713..4613ab4 100644
--- a/tests/test_bootstrap.py
+++ b/tests/test_bootstrap.py
@@ -6,20 +6,20 @@ from fastapi.testclient import TestClient
 
 
 @pytest.fixture
-def fresh_client(tmp_path):
+def fresh_client(tmp_path, monkeypatch):
     """Client with EMPTY database — no users."""
-    os.environ["DATA_DIR"] = str(tmp_path)
-    os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
+    monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
     from app.main import create_app
     app = create_app()
     return TestClient(app)
 
 
 @pytest.fixture
-def seeded_client(tmp_path):
+def seeded_client(tmp_path, monkeypatch):
     """Client with one existing user."""
-    os.environ["DATA_DIR"] = str(tmp_path)
-    os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
+    monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
     from app.main import create_app
     from src.db import get_system_db
     from src.repositories.users import UserRepository
diff --git a/tests/test_db.py b/tests/test_db.py
index 61bc4dc..32d625f 100644
--- a/tests/test_db.py
+++ b/tests/test_db.py
@@ -34,8 +34,8 @@ class TestGetSystemDb:
         finally:
             conn.close()
 
-    def test_idempotent(self, tmp_path):
-        _setup_data_dir(tmp_path)
+    def test_idempotent(self, tmp_path, monkeypatch):
+        _setup_data_dir(tmp_path, monkeypatch)
         from src.db import get_system_db
 
         conn = get_system_db()
@@ -53,8 +53,8 @@ class TestGetSystemDb:
 
 
 class TestGetSchemaVersion:
-    def test_returns_version(self, tmp_path):
-        _setup_data_dir(tmp_path)
+    def test_returns_version(self, tmp_path, monkeypatch):
+        _setup_data_dir(tmp_path, monkeypatch)
         from src.db import get_schema_version, get_system_db
 
         conn = get_system_db()
@@ -63,8 +63,8 @@ class TestGetSchemaVersion:
         finally:
             conn.close()
 
-    def test_returns_zero_for_empty_db(self, tmp_path):
-        _setup_data_dir(tmp_path)
+    def test_returns_zero_for_empty_db(self, tmp_path, monkeypatch):
+        _setup_data_dir(tmp_path, monkeypatch)
         from src.db import get_schema_version
 
         conn = duckdb.connect(str(tmp_path / "empty.duckdb"))
@@ -75,9 +75,9 @@ class TestGetSchemaVersion:
 
 
 class TestV1ToV2Migration:
-    def test_migration_adds_source_columns(self, tmp_path):
+    def test_migration_adds_source_columns(self, tmp_path, monkeypatch):
         """Simulate a v1 database and verify v2 migration adds new columns."""
-        _setup_data_dir(tmp_path)
+        _setup_data_dir(tmp_path, monkeypatch)
         import duckdb as _duckdb
 
         # Create a v1 database manually
@@ -133,8 +133,8 @@ class TestV1ToV2Migration:
 
 
 class TestGetAnalyticsDb:
-    def test_creates_db(self, tmp_path):
-        _setup_data_dir(tmp_path)
+    def test_creates_db(self, tmp_path, monkeypatch):
+        _setup_data_dir(tmp_path, monkeypatch)
         from src.db import get_analytics_db
 
         conn = get_analytics_db()
@@ -145,9 +145,9 @@ class TestGetAnalyticsDb:
 
 
 class TestGetAnalyticsDbReadonly:
-    def test_analytics_readonly_rejects_malicious_dir_name(self, tmp_path):
+    def test_analytics_readonly_rejects_malicious_dir_name(self, tmp_path, monkeypatch):
         """Directories with SQL-injection chars in their name are skipped."""
-        _setup_data_dir(tmp_path)
+        _setup_data_dir(tmp_path, monkeypatch)
         import importlib
         import src.db as db_module
         importlib.reload(db_module)
diff --git a/tests/test_migration.py b/tests/test_migration.py
index 7802b72..24a71ea 100644
--- a/tests/test_migration.py
+++ b/tests/test_migration.py
@@ -6,7 +6,7 @@ import pytest
 
 
 @pytest.fixture
-def migration_env(tmp_path):
+def migration_env(tmp_path, monkeypatch):
     """Create temp dir with sample JSON files mimicking production layout."""
     data_dir = tmp_path / "data"
     (data_dir / "notifications").mkdir(parents=True)
@@ -52,7 +52,7 @@ def migration_env(tmp_path):
         }
     }))
 
-    os.environ["DATA_DIR"] = str(data_dir)
+    monkeypatch.setenv("DATA_DIR", str(data_dir))
     return str(data_dir)
 
 
@@ -79,11 +79,11 @@ def test_migration_idempotent(migration_env):
     assert stats2["sync_state"] == 2
 
 
-def test_migration_with_missing_files(tmp_path):
+def test_migration_with_missing_files(tmp_path, monkeypatch):
     """Migration should handle missing JSON files gracefully."""
     data_dir = tmp_path / "empty_data"
     data_dir.mkdir()
-    os.environ["DATA_DIR"] = str(data_dir)
+    monkeypatch.setenv("DATA_DIR", str(data_dir))
     from scripts.migrate_json_to_duckdb import migrate_all
     stats = migrate_all(str(data_dir))
     assert stats["sync_state"] == 0
diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py
index d54507a..504bef3 100644
--- a/tests/test_orchestrator.py
+++ b/tests/test_orchestrator.py
@@ -8,9 +8,9 @@ import pytest
 
 
 @pytest.fixture
-def setup_env(tmp_path):
+def setup_env(tmp_path, monkeypatch):
     """Set up DATA_DIR and return paths."""
-    os.environ["DATA_DIR"] = str(tmp_path)
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
     extracts_dir = tmp_path / "extracts"
     extracts_dir.mkdir()
     analytics_dir = tmp_path / "analytics"
@@ -22,8 +22,6 @@ def setup_env(tmp_path):
         "extracts_dir": extracts_dir,
         "analytics_db": str(analytics_dir / "server.duckdb"),
     }
-    # Clean up env var to avoid leaking between tests
-    os.environ.pop("DATA_DIR", None)
 
 
 def _create_mock_extract(extracts_dir: Path, source_name: str, tables: list[dict]):
diff --git a/tests/test_permissions.py b/tests/test_permissions.py
index 976d777..1dcc07c 100644
--- a/tests/test_permissions.py
+++ b/tests/test_permissions.py
@@ -5,8 +5,8 @@ import pytest
 
 
 @pytest.fixture
-def db_conn(tmp_path):
-    os.environ["DATA_DIR"] = str(tmp_path)
+def db_conn(tmp_path, monkeypatch):
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
     from src.db import get_system_db
     conn = get_system_db()
     yield conn
diff --git a/tests/test_rbac.py b/tests/test_rbac.py
index 45fa6ec..0410546 100644
--- a/tests/test_rbac.py
+++ b/tests/test_rbac.py
@@ -5,8 +5,8 @@ import pytest
 
 
 @pytest.fixture
-def setup_db(tmp_path):
-    os.environ["DATA_DIR"] = str(tmp_path)
+def setup_db(tmp_path, monkeypatch):
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
     from src.db import get_system_db
     from src.repositories.users import UserRepository
 
diff --git a/tests/test_repositories.py b/tests/test_repositories.py
index 920fbec..4417fe3 100644
--- a/tests/test_repositories.py
+++ b/tests/test_repositories.py
@@ -5,8 +5,8 @@ import pytest
 
 
 @pytest.fixture
-def db_conn(tmp_path):
-    os.environ["DATA_DIR"] = str(tmp_path)
+def db_conn(tmp_path, monkeypatch):
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
     from src.db import get_system_db
     conn = get_system_db()
     yield conn
diff --git a/tests/test_security.py b/tests/test_security.py
index 95829b3..4265106 100644
--- a/tests/test_security.py
+++ b/tests/test_security.py
@@ -8,10 +8,10 @@ from fastapi.testclient import TestClient
 
 
 @pytest.fixture
-def client(tmp_path):
-    os.environ["DATA_DIR"] = str(tmp_path)
-    os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
-    os.environ["SCRIPT_TIMEOUT"] = "5"
+def client(tmp_path, monkeypatch):
+    monkeypatch.setenv("DATA_DIR", str(tmp_path))
+    monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
+    monkeypatch.setenv("SCRIPT_TIMEOUT", "5")
 
     from app.main import create_app
     from src.db import get_system_db