Merge pull request #553 from OpenCOMPES/fix-552

update tests for env variables

Author: zain-sohail

.cspell/custom-dictionary.txt

@@ -79,6 +79,7 @@ datestring
 ddir
 delaxes
 delayeds
+delenv
 Desy
 Deutsches
 dfield

src/sed/core/config.py

@@ -19,6 +19,7 @@
 package_dir = os.path.dirname(find_spec("sed").origin)
 
 USER_CONFIG_PATH = user_config_path(appname="sed", appauthor="OpenCOMPES", ensure_exists=True)
+ENV_DIR = Path(".env")
 
 # Configure logging
 logger = setup_logging("config")
@@ -295,7 +296,7 @@ def read_env_var(var_name: str) -> str | None:
         return value
 
     # Then check .env in current directory
-    local_vars = _parse_env_file(Path(".env"))
+    local_vars = _parse_env_file(ENV_DIR)
     if var_name in local_vars:
         logger.debug(f"Found {var_name} in ./.env file")
         return local_vars[var_name]

tests/test_config.py

@@ -15,7 +15,6 @@
 from sed.core.config import read_env_var
 from sed.core.config import save_config
 from sed.core.config import save_env_var
-from sed.core.config import USER_CONFIG_PATH
 
 test_dir = os.path.dirname(__file__)
 test_config_dir = Path(f"{test_dir}/data/loader/")
@@ -235,11 +234,15 @@ def test_invalid_config_wrong_values():
     assert "Invalid value 9999 for gid. Group not found." in str(e.value)
 
 
-def test_env_var_read_write(tmp_path, monkeypatch):
-    """Test reading and writing environment variables."""
-    # Mock USER_CONFIG_PATH to use a temporary directory
+@pytest.fixture
+def mock_env_file(tmp_path, monkeypatch):
+    """Mock the .env file for testing"""
     monkeypatch.setattr("sed.core.config.USER_CONFIG_PATH", tmp_path)
+    yield tmp_path
+
 
+def test_env_var_read_write(mock_env_file):  # noqa: ARG001
+    """Test reading and writing environment variables."""
     # Test writing a new variable
     save_env_var("TEST_VAR", "test_value")
     assert read_env_var("TEST_VAR") == "test_value"
@@ -258,16 +261,13 @@ def test_env_var_read_write(tmp_path, monkeypatch):
     assert read_env_var("NON_EXISTENT_VAR") is None
 
 
-def test_env_var_read_no_file(tmp_path, monkeypatch):
+def test_env_var_read_no_file(mock_env_file):  # noqa: ARG001
     """Test reading environment variables when .env file doesn't exist."""
-    # Mock USER_CONFIG_PATH to use an empty temporary directory
-    monkeypatch.setattr("sed.core.config.USER_CONFIG_PATH", tmp_path)
-
     # Test reading from non-existent file
     assert read_env_var("TEST_VAR") is None
 
 
-def test_env_var_special_characters():
+def test_env_var_special_characters(mock_env_file):  # noqa: ARG001
     """Test reading and writing environment variables with special characters."""
     test_cases = {
         "TEST_URL": "http://example.com/path?query=value",
@@ -280,50 +280,42 @@ def test_env_var_special_characters():
         assert read_env_var(var_name) == value
 
 
-@pytest.fixture
-def cleanup_env_files():
-    """Cleanup any .env files before and after tests"""
-    # Clean up any existing .env files
-    for path in [Path(".env"), USER_CONFIG_PATH / ".env"]:
-        if path.exists():
-            path.unlink()
-
-    yield
-
-    # Clean up after tests
-    for path in [Path(".env"), USER_CONFIG_PATH / ".env"]:
-        if path.exists():
-            path.unlink()
-
-
-def test_env_var_precedence(cleanup_env_files):  # noqa: ARG001
+def test_env_var_precedence(mock_env_file, tmp_path, monkeypatch):  # noqa: ARG001
     """Test that environment variables are read in correct order of precedence"""
+    # Create local .env directory if it doesn't exist
+    local_env_dir = tmp_path / "local"
+    local_env_dir.mkdir(exist_ok=True)
+    monkeypatch.setattr("sed.core.config.ENV_DIR", local_env_dir / ".env")
+
     # Set up test values in different locations
     os.environ["TEST_VAR"] = "os_value"
 
-    with open(".env", "w") as f:
-        f.write("TEST_VAR=local_value\n")
-
-    save_env_var("TEST_VAR", "user_value")  # Saves to USER_CONFIG_PATH
+    # Save to user config first (lowest precedence)
+    save_env_var("TEST_VAR", "user_value")
 
-    # Should get OS value first
-    assert read_env_var("TEST_VAR") == "os_value"
+    # Create local .env file (medium precedence)
+    with open(local_env_dir / ".env", "w") as f:
+        f.write("TEST_VAR=local_value\n")
 
-    # Remove from OS env and should get local value
-    del os.environ["TEST_VAR"]
+    # Remove from OS env to test other precedence levels
+    monkeypatch.delenv("TEST_VAR", raising=False)
     assert read_env_var("TEST_VAR") == "local_value"
 
     # Remove local .env and should get user config value
-    Path(".env").unlink()
+    (local_env_dir / ".env").unlink()
     assert read_env_var("TEST_VAR") == "user_value"
 
     # Remove user config and should get None
-    (USER_CONFIG_PATH / ".env").unlink()
+    (mock_env_file / ".env").unlink()
     assert read_env_var("TEST_VAR") is None
 
 
-def test_env_var_save_and_load(cleanup_env_files):  # noqa: ARG001
+def test_env_var_save_and_load(mock_env_file, monkeypatch):  # noqa: ARG001
     """Test saving and loading environment variables"""
+    # Clear any existing OS environment variables
+    monkeypatch.delenv("TEST_VAR", raising=False)
+    monkeypatch.delenv("OTHER_VAR", raising=False)
+
     # Save a variable
     save_env_var("TEST_VAR", "test_value")
 
@@ -336,14 +328,20 @@ def test_env_var_save_and_load(cleanup_env_files):  # noqa: ARG001
     assert read_env_var("OTHER_VAR") == "other_value"
 
 
-def test_env_var_not_found(cleanup_env_files):  # noqa: ARG001
+def test_env_var_not_found(mock_env_file):  # noqa: ARG001
     """Test behavior when environment variable is not found"""
     assert read_env_var("NONEXISTENT_VAR") is None
 
 
-def test_env_file_format(cleanup_env_files):  # noqa: ARG001
+def test_env_file_format(mock_env_file, monkeypatch):  # noqa: ARG001
     """Test that .env file parsing handles different formats correctly"""
-    with open(".env", "w") as f:
+    # Clear any existing OS environment variables
+    monkeypatch.delenv("TEST_VAR", raising=False)
+    monkeypatch.delenv("SPACES_VAR", raising=False)
+    monkeypatch.delenv("EMPTY_VAR", raising=False)
+    monkeypatch.delenv("COMMENT", raising=False)
+
+    with open(mock_env_file / ".env", "w") as f:
         f.write(
             """
                 TEST_VAR=value1