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.
This commit is contained in:
parent
c56511f69f
commit
8e9a0c367a
12 changed files with 50 additions and 52 deletions
|
|
@ -6,19 +6,19 @@ from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def app_client(tmp_path):
|
def app_client(tmp_path, monkeypatch):
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
os.environ["JWT_SECRET_KEY"] = "test-secret"
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret")
|
||||||
from app.main import create_app
|
from app.main import create_app
|
||||||
app = create_app()
|
app = create_app()
|
||||||
return TestClient(app)
|
return TestClient(app)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def seeded_client(tmp_path):
|
def seeded_client(tmp_path, monkeypatch):
|
||||||
"""Client with a pre-created admin user and JWT token."""
|
"""Client with a pre-created admin user and JWT token."""
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
os.environ["JWT_SECRET_KEY"] = "test-secret"
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret")
|
||||||
from app.main import create_app
|
from app.main import create_app
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
from src.repositories.users import UserRepository
|
from src.repositories.users import UserRepository
|
||||||
|
|
|
||||||
|
|
@ -6,9 +6,9 @@ from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def client(tmp_path):
|
def client(tmp_path, monkeypatch):
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
|
||||||
|
|
||||||
from app.main import create_app
|
from app.main import create_app
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
|
|
|
||||||
|
|
@ -6,10 +6,10 @@ from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def client(tmp_path):
|
def client(tmp_path, monkeypatch):
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
|
||||||
os.environ["SCRIPT_TIMEOUT"] = "10"
|
monkeypatch.setenv("SCRIPT_TIMEOUT", "10")
|
||||||
|
|
||||||
from app.main import create_app
|
from app.main import create_app
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
|
|
|
||||||
|
|
@ -6,9 +6,9 @@ from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def client(tmp_path):
|
def client(tmp_path, monkeypatch):
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
|
||||||
|
|
||||||
from app.main import create_app
|
from app.main import create_app
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
|
|
|
||||||
|
|
@ -6,20 +6,20 @@ from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def fresh_client(tmp_path):
|
def fresh_client(tmp_path, monkeypatch):
|
||||||
"""Client with EMPTY database — no users."""
|
"""Client with EMPTY database — no users."""
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
|
||||||
from app.main import create_app
|
from app.main import create_app
|
||||||
app = create_app()
|
app = create_app()
|
||||||
return TestClient(app)
|
return TestClient(app)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def seeded_client(tmp_path):
|
def seeded_client(tmp_path, monkeypatch):
|
||||||
"""Client with one existing user."""
|
"""Client with one existing user."""
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
|
||||||
from app.main import create_app
|
from app.main import create_app
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
from src.repositories.users import UserRepository
|
from src.repositories.users import UserRepository
|
||||||
|
|
|
||||||
|
|
@ -34,8 +34,8 @@ class TestGetSystemDb:
|
||||||
finally:
|
finally:
|
||||||
conn.close()
|
conn.close()
|
||||||
|
|
||||||
def test_idempotent(self, tmp_path):
|
def test_idempotent(self, tmp_path, monkeypatch):
|
||||||
_setup_data_dir(tmp_path)
|
_setup_data_dir(tmp_path, monkeypatch)
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
|
|
||||||
conn = get_system_db()
|
conn = get_system_db()
|
||||||
|
|
@ -53,8 +53,8 @@ class TestGetSystemDb:
|
||||||
|
|
||||||
|
|
||||||
class TestGetSchemaVersion:
|
class TestGetSchemaVersion:
|
||||||
def test_returns_version(self, tmp_path):
|
def test_returns_version(self, tmp_path, monkeypatch):
|
||||||
_setup_data_dir(tmp_path)
|
_setup_data_dir(tmp_path, monkeypatch)
|
||||||
from src.db import get_schema_version, get_system_db
|
from src.db import get_schema_version, get_system_db
|
||||||
|
|
||||||
conn = get_system_db()
|
conn = get_system_db()
|
||||||
|
|
@ -63,8 +63,8 @@ class TestGetSchemaVersion:
|
||||||
finally:
|
finally:
|
||||||
conn.close()
|
conn.close()
|
||||||
|
|
||||||
def test_returns_zero_for_empty_db(self, tmp_path):
|
def test_returns_zero_for_empty_db(self, tmp_path, monkeypatch):
|
||||||
_setup_data_dir(tmp_path)
|
_setup_data_dir(tmp_path, monkeypatch)
|
||||||
from src.db import get_schema_version
|
from src.db import get_schema_version
|
||||||
|
|
||||||
conn = duckdb.connect(str(tmp_path / "empty.duckdb"))
|
conn = duckdb.connect(str(tmp_path / "empty.duckdb"))
|
||||||
|
|
@ -75,9 +75,9 @@ class TestGetSchemaVersion:
|
||||||
|
|
||||||
|
|
||||||
class TestV1ToV2Migration:
|
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."""
|
"""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
|
import duckdb as _duckdb
|
||||||
|
|
||||||
# Create a v1 database manually
|
# Create a v1 database manually
|
||||||
|
|
@ -133,8 +133,8 @@ class TestV1ToV2Migration:
|
||||||
|
|
||||||
|
|
||||||
class TestGetAnalyticsDb:
|
class TestGetAnalyticsDb:
|
||||||
def test_creates_db(self, tmp_path):
|
def test_creates_db(self, tmp_path, monkeypatch):
|
||||||
_setup_data_dir(tmp_path)
|
_setup_data_dir(tmp_path, monkeypatch)
|
||||||
from src.db import get_analytics_db
|
from src.db import get_analytics_db
|
||||||
|
|
||||||
conn = get_analytics_db()
|
conn = get_analytics_db()
|
||||||
|
|
@ -145,9 +145,9 @@ class TestGetAnalyticsDb:
|
||||||
|
|
||||||
|
|
||||||
class TestGetAnalyticsDbReadonly:
|
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."""
|
"""Directories with SQL-injection chars in their name are skipped."""
|
||||||
_setup_data_dir(tmp_path)
|
_setup_data_dir(tmp_path, monkeypatch)
|
||||||
import importlib
|
import importlib
|
||||||
import src.db as db_module
|
import src.db as db_module
|
||||||
importlib.reload(db_module)
|
importlib.reload(db_module)
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,7 @@ import pytest
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def migration_env(tmp_path):
|
def migration_env(tmp_path, monkeypatch):
|
||||||
"""Create temp dir with sample JSON files mimicking production layout."""
|
"""Create temp dir with sample JSON files mimicking production layout."""
|
||||||
data_dir = tmp_path / "data"
|
data_dir = tmp_path / "data"
|
||||||
(data_dir / "notifications").mkdir(parents=True)
|
(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)
|
return str(data_dir)
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -79,11 +79,11 @@ def test_migration_idempotent(migration_env):
|
||||||
assert stats2["sync_state"] == 2
|
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."""
|
"""Migration should handle missing JSON files gracefully."""
|
||||||
data_dir = tmp_path / "empty_data"
|
data_dir = tmp_path / "empty_data"
|
||||||
data_dir.mkdir()
|
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
|
from scripts.migrate_json_to_duckdb import migrate_all
|
||||||
stats = migrate_all(str(data_dir))
|
stats = migrate_all(str(data_dir))
|
||||||
assert stats["sync_state"] == 0
|
assert stats["sync_state"] == 0
|
||||||
|
|
|
||||||
|
|
@ -8,9 +8,9 @@ import pytest
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def setup_env(tmp_path):
|
def setup_env(tmp_path, monkeypatch):
|
||||||
"""Set up DATA_DIR and return paths."""
|
"""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 = tmp_path / "extracts"
|
||||||
extracts_dir.mkdir()
|
extracts_dir.mkdir()
|
||||||
analytics_dir = tmp_path / "analytics"
|
analytics_dir = tmp_path / "analytics"
|
||||||
|
|
@ -22,8 +22,6 @@ def setup_env(tmp_path):
|
||||||
"extracts_dir": extracts_dir,
|
"extracts_dir": extracts_dir,
|
||||||
"analytics_db": str(analytics_dir / "server.duckdb"),
|
"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]):
|
def _create_mock_extract(extracts_dir: Path, source_name: str, tables: list[dict]):
|
||||||
|
|
|
||||||
|
|
@ -5,8 +5,8 @@ import pytest
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def db_conn(tmp_path):
|
def db_conn(tmp_path, monkeypatch):
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
conn = get_system_db()
|
conn = get_system_db()
|
||||||
yield conn
|
yield conn
|
||||||
|
|
|
||||||
|
|
@ -5,8 +5,8 @@ import pytest
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def setup_db(tmp_path):
|
def setup_db(tmp_path, monkeypatch):
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
from src.repositories.users import UserRepository
|
from src.repositories.users import UserRepository
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -5,8 +5,8 @@ import pytest
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def db_conn(tmp_path):
|
def db_conn(tmp_path, monkeypatch):
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
conn = get_system_db()
|
conn = get_system_db()
|
||||||
yield conn
|
yield conn
|
||||||
|
|
|
||||||
|
|
@ -8,10 +8,10 @@ from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def client(tmp_path):
|
def client(tmp_path, monkeypatch):
|
||||||
os.environ["DATA_DIR"] = str(tmp_path)
|
monkeypatch.setenv("DATA_DIR", str(tmp_path))
|
||||||
os.environ["JWT_SECRET_KEY"] = "test-secret-32chars-minimum!!!!!"
|
monkeypatch.setenv("JWT_SECRET_KEY", "test-secret-32chars-minimum!!!!!")
|
||||||
os.environ["SCRIPT_TIMEOUT"] = "5"
|
monkeypatch.setenv("SCRIPT_TIMEOUT", "5")
|
||||||
|
|
||||||
from app.main import create_app
|
from app.main import create_app
|
||||||
from src.db import get_system_db
|
from src.db import get_system_db
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue