diff --git a/libp2p/utils/logging.py b/libp2p/utils/logging.py index acc67373..a9da0d65 100644 --- a/libp2p/utils/logging.py +++ b/libp2p/utils/logging.py @@ -18,6 +18,9 @@ log_queue: "queue.Queue[Any]" = queue.Queue() # Store the current listener to stop it on exit _current_listener: logging.handlers.QueueListener | None = None +# Store the handlers for proper cleanup +_current_handlers: list[logging.Handler] = [] + # Event to track when the listener is ready _listener_ready = threading.Event() @@ -92,7 +95,7 @@ def setup_logging() -> None: - Child loggers inherit their parent's level unless explicitly set - The root libp2p logger controls the default level """ - global _current_listener, _listener_ready + global _current_listener, _listener_ready, _current_handlers # Reset the event _listener_ready.clear() @@ -101,6 +104,12 @@ def setup_logging() -> None: if _current_listener is not None: _current_listener.stop() _current_listener = None + + # Close and clear existing handlers + for handler in _current_handlers: + if isinstance(handler, logging.FileHandler): + handler.close() + _current_handlers.clear() # Get the log level from environment variable debug_str = os.environ.get("LIBP2P_DEBUG", "") @@ -189,6 +198,9 @@ def setup_logging() -> None: logger.setLevel(level) logger.propagate = False # Prevent message duplication + # Store handlers globally for cleanup + _current_handlers.extend(handlers) + # Start the listener AFTER configuring all loggers _current_listener = logging.handlers.QueueListener( log_queue, *handlers, respect_handler_level=True @@ -203,7 +215,13 @@ def setup_logging() -> None: @atexit.register def cleanup_logging() -> None: """Clean up logging resources on exit.""" - global _current_listener + global _current_listener, _current_handlers if _current_listener is not None: _current_listener.stop() _current_listener = None + + # Close all file handlers to ensure proper cleanup on Windows + for handler in _current_handlers: + if isinstance(handler, logging.FileHandler): + handler.close() + _current_handlers.clear() diff --git a/tests/utils/test_logging.py b/tests/utils/test_logging.py index 603af5e1..05c76ec2 100644 --- a/tests/utils/test_logging.py +++ b/tests/utils/test_logging.py @@ -15,6 +15,7 @@ import pytest import trio from libp2p.utils.logging import ( + _current_handlers, _current_listener, _listener_ready, log_queue, @@ -24,13 +25,19 @@ from libp2p.utils.logging import ( def _reset_logging(): """Reset all logging state.""" - global _current_listener, _listener_ready + global _current_listener, _listener_ready, _current_handlers # Stop existing listener if any if _current_listener is not None: _current_listener.stop() _current_listener = None + # Close all file handlers to ensure proper cleanup on Windows + for handler in _current_handlers: + if isinstance(handler, logging.FileHandler): + handler.close() + _current_handlers.clear() + # Reset the event _listener_ready = threading.Event() @@ -173,6 +180,15 @@ async def test_custom_log_file(clean_env): # Stop the listener to ensure all messages are written if _current_listener is not None: _current_listener.stop() + + # Give a moment for the listener to fully stop + await trio.sleep(0.05) + + # Close all file handlers to release the file + for handler in _current_handlers: + if isinstance(handler, logging.FileHandler): + handler.flush() # Ensure all writes are flushed + handler.close() # Check if the file exists and contains our message assert log_file.exists() @@ -185,17 +201,14 @@ async def test_default_log_file(clean_env): """Test logging to the default file path.""" os.environ["LIBP2P_DEBUG"] = "INFO" - with patch("libp2p.utils.logging.datetime") as mock_datetime: - # Mock the timestamp to have a predictable filename - mock_datetime.now.return_value.strftime.return_value = "20240101_120000" - + with patch("libp2p.utils.paths.create_temp_file") as mock_create_temp: + # Mock the temp file creation to return a predictable path + mock_temp_file = Path(tempfile.gettempdir()) / "test_py-libp2p_20240101_120000.log" + mock_create_temp.return_value = mock_temp_file + # Remove the log file if it exists - if os.name == "nt": # Windows - log_file = Path("C:/Windows/Temp/20240101_120000_py-libp2p.log") - else: # Unix-like - log_file = Path("/tmp/20240101_120000_py-libp2p.log") - log_file.unlink(missing_ok=True) - + mock_temp_file.unlink(missing_ok=True) + setup_logging() # Wait for the listener to be ready @@ -210,10 +223,19 @@ async def test_default_log_file(clean_env): # Stop the listener to ensure all messages are written if _current_listener is not None: _current_listener.stop() + + # Give a moment for the listener to fully stop + await trio.sleep(0.05) + + # Close all file handlers to release the file + for handler in _current_handlers: + if isinstance(handler, logging.FileHandler): + handler.flush() # Ensure all writes are flushed + handler.close() - # Check the default log file - if log_file.exists(): # Only check content if we have write permission - content = log_file.read_text() + # Check the mocked temp file + if mock_temp_file.exists(): + content = mock_temp_file.read_text() assert "Test message" in content diff --git a/tests/utils/test_paths.py b/tests/utils/test_paths.py index e5247cdb..a8eb4ed9 100644 --- a/tests/utils/test_paths.py +++ b/tests/utils/test_paths.py @@ -6,6 +6,8 @@ import os from pathlib import Path import tempfile +import pytest + from libp2p.utils.paths import ( create_temp_file, ensure_dir_exists, @@ -217,6 +219,12 @@ class TestCrossPlatformCompatibility: def test_config_dir_platform_specific_windows(self, monkeypatch): """Test config directory respects Windows conventions.""" + import platform + + # Only run this test on Windows systems + if platform.system() != "Windows": + pytest.skip("This test only runs on Windows systems") + monkeypatch.setattr("os.name", "nt") monkeypatch.setenv("APPDATA", "C:\\Users\\Test\\AppData\\Roaming") config_dir = get_config_dir()