Merge pull request #235 from calculquebec/hotfix/logger_permission_error

fix: handle logger permission errors and move default logpath to cwd

Author: Scirelgar

pennylane_calculquebec/logger.py

@@ -2,17 +2,33 @@
 import os
 from pennylane_calculquebec._version import __version__
 
-ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+DEFAULT_LOG_PATH = os.path.join(
+    os.getcwd(),
+    "pennylane_calculquebec.log",
+)
 LOG_PATH = os.environ.get(
-    "PLCQ_LOG_PATH", os.path.join(ROOT_DIR, "pennylane_calculquebec.log")
+    "PLCQ_LOG_PATH",
+    DEFAULT_LOG_PATH,
 )
 
 logger = logging.getLogger("pennylane_calculquebec")
 logger.setLevel(logging.INFO)
 
-handler = logging.FileHandler(LOG_PATH, mode="a", encoding="utf-8")
+try:
+    handler = logging.FileHandler(LOG_PATH, mode="a", encoding="utf-8")
+
+except OSError as e:
+    logging.warning(
+        "Unable to open log file '%s' for writing due to a %s: %s. "
+        "Falling back to console logging (StreamHandler).",
+        LOG_PATH,
+        type(e).__name__,
+        e,
+    )
+    handler = logging.StreamHandler()
+
 formatter = logging.Formatter(
-    f"%(asctime)s - The plugin version is {__version__} | Incident: %(message)s",
+    f"%(asctime)s [%(levelname)s] Version {__version__} | Message: %(message)s",
     datefmt="%Y-%m-%d %H:%M:%S",
 )
 handler.setFormatter(formatter)

tests/test_logger.py

@@ -1,40 +1,74 @@
+import importlib
 import os
-import tempfile
 import logging
-from pennylane_calculquebec.logger import logger  # Replace with actual import
+from unittest.mock import patch
+import pytest
+from pennylane_calculquebec import logger as logger_module
 
 
-def test_logger_creates_virtual_folder_and_logs_message(monkeypatch):
-    with tempfile.TemporaryDirectory() as tmp_dir:
-        log_path = os.path.join(tmp_dir, "virtual", "pennylane_calculquebec.log")
+@pytest.fixture
+def clean_logger():
+    """Fixture to ensure logger is in a clean state and handlers are closed."""
+    # Close existing handlers to release file locks
+    for h in logger_module.logger.handlers[:]:
+        logger_module.logger.removeHandler(h)
+        h.close()
+    yield
+    # Cleanup after test
+    for h in logger_module.logger.handlers[:]:
+        logger_module.logger.removeHandler(h)
+        h.close()
 
-        # Patch environment variable
-        monkeypatch.setenv("PLCQ_LOG_PATH", log_path)
 
-        # Ensure log directory exists
-        os.makedirs(os.path.dirname(log_path), exist_ok=True)
+def test_logger_write_permission_denied(clean_logger, caplog):
+    """Test that logger falls back to StreamHandler if FileHandler fails due to permissions."""
+    with patch("logging.FileHandler", side_effect=PermissionError("Permission denied")):
+        importlib.reload(logger_module)
 
-        # Remove any existing handlers
-        for h in logger.handlers[:]:
-            logger.removeHandler(h)
-            h.close()  # <-- Close existing handlers
+    handlers = logger_module.logger.handlers
+    # Check if a StreamHandler is present (and it's not a FileHandler which is a subclass)
+    assert any(type(h) is logging.StreamHandler for h in handlers)
+    assert "Unable to open log file" in caplog.text
+    assert "PermissionError" in caplog.text
 
-        # Set up new FileHandler
-        handler = logging.FileHandler(log_path, mode="a", encoding="utf-8")
-        formatter = logging.Formatter("Test | %(asctime)s | Incident: %(message)s")
-        handler.setFormatter(formatter)
-        logger.addHandler(handler)
 
-        # Log test message
-        test_message = "Simulated error"
-        logger.error(test_message)
+def test_logger_file_not_found(clean_logger, caplog):
+    """Test that logger falls back to StreamHandler if FileHandler fails due to file not found."""
+    with patch("logging.FileHandler", side_effect=FileNotFoundError("File not found")):
+        importlib.reload(logger_module)
 
-        # Verify file created and message written
-        assert os.path.exists(log_path)
-        with open(log_path, "r", encoding="utf-8") as f:
-            contents = f.read()
-            assert test_message in contents
+    handlers = logger_module.logger.handlers
+    # Check if a StreamHandler is present (and it's not a FileHandler which is a subclass)
+    assert any(type(h) is logging.StreamHandler for h in handlers)
+    assert "Unable to open log file" in caplog.text
+    assert "FileNotFoundError" in caplog.text
 
-        # Explicitly close the handler to release the file lock
-        logger.removeHandler(handler)
-        handler.close()
+
+def test_logger_create_logfile_in_set_env_var(clean_logger, tmp_path):
+    """Test that logger uses the path specified in PLCQ_LOG_PATH environment variable."""
+    log_path = str(tmp_path / "custom_env.log")
+
+    with patch.dict(os.environ, {"PLCQ_LOG_PATH": log_path}):
+        importlib.reload(logger_module)
+
+    logger_module.logger.info("test message env var")
+
+    assert os.path.exists(log_path)
+    with open(log_path, "r", encoding="utf-8") as f:
+        assert "test message env var" in f.read()
+
+
+def test_logger_create_logfile_in_cwd(clean_logger, tmp_path, monkeypatch):
+    """Test that logger defaults to creating a log file in the current working directory."""
+    # Use monkeypatch to safely change CWD and environment
+    monkeypatch.chdir(tmp_path)
+    monkeypatch.delenv("PLCQ_LOG_PATH", raising=False)
+
+    importlib.reload(logger_module)
+
+    expected_path = os.path.join(os.getcwd(), "pennylane_calculquebec.log")
+    logger_module.logger.info("test message cwd")
+
+    assert os.path.exists(expected_path)
+    with open(expected_path, "r", encoding="utf-8") as f:
+        assert "test message cwd" in f.read()