From 461bc049a058a233afe02c5ae3ba4a9571651e75 Mon Sep 17 00:00:00 2001
From: Sebastian Mohr <sebastian@mohrenclan.de>
Date: Wed, 17 Sep 2025 11:42:07 +0200
Subject: [PATCH] Enhanced custom logger typing and logging tests

---
 beets/logging.py     | 47 ++++++++++++++++++++++++++++++--------------
 test/test_logging.py | 45 +++++++++++++++++++++++++++++-------------
 2 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/beets/logging.py b/beets/logging.py
index fd8b1962f..becfff86f 100644
--- a/beets/logging.py
+++ b/beets/logging.py
@@ -20,6 +20,9 @@ use {}-style formatting and can interpolate keywords arguments to the logging
 calls (`debug`, `info`, etc).
 """
 
+from __future__ import annotations
+
+import logging
 import threading
 from copy import copy
 from logging import (
@@ -34,6 +37,9 @@ from logging import (
     NullHandler,
     StreamHandler,
 )
+from typing import TYPE_CHECKING, Any, Mapping, TypeVar
+
+from typing_extensions import ParamSpec
 
 __all__ = [
     "DEBUG",
@@ -49,8 +55,14 @@ __all__ = [
     "getLogger",
 ]
 
+if TYPE_CHECKING:
+    T = TypeVar("T")
 
-def logsafe(val):
+
+P = ParamSpec("P")
+
+
+def _logsafe(val: T) -> str | T:
     """Coerce `bytes` to `str` to avoid crashes solely due to logging.
 
     This is particularly relevant for bytestring paths. Much of our code
@@ -83,40 +95,45 @@ class StrFormatLogger(Logger):
     """
 
     class _LogMessage:
-        def __init__(self, msg, args, kwargs):
+        def __init__(
+            self,
+            msg: str,
+            args: logging._ArgsType,
+            kwargs: dict[str, Any],
+        ):
             self.msg = msg
             self.args = args
             self.kwargs = kwargs
 
         def __str__(self):
-            args = [logsafe(a) for a in self.args]
-            kwargs = {k: logsafe(v) for (k, v) in self.kwargs.items()}
+            args = [_logsafe(a) for a in self.args]
+            kwargs = {k: _logsafe(v) for (k, v) in self.kwargs.items()}
             return self.msg.format(*args, **kwargs)
 
     def _log(
         self,
-        level,
-        msg,
-        args,
-        exc_info=None,
-        extra=None,
-        stack_info=False,
+        level: int,
+        msg: object,
+        args: logging._ArgsType = (),
+        exc_info: logging._ExcInfoType = None,
+        extra: Mapping[str, Any] | None = None,
+        stack_info: bool = False,
+        stacklevel: int = 1,
         **kwargs,
     ):
         """Log msg.format(*args, **kwargs)"""
-        m = self._LogMessage(msg, args, kwargs)
 
-        stacklevel = kwargs.pop("stacklevel", 1)
-        stacklevel = {"stacklevel": stacklevel}
+        if isinstance(msg, str):
+            msg = self._LogMessage(msg, args, kwargs)
 
         return super()._log(
             level,
-            m,
+            msg,
             (),
             exc_info=exc_info,
             extra=extra,
             stack_info=stack_info,
-            **stacklevel,
+            stacklevel=stacklevel,
         )
 
 
diff --git a/test/test_logging.py b/test/test_logging.py
index b751dd70e..4a62e3a3b 100644
--- a/test/test_logging.py
+++ b/test/test_logging.py
@@ -3,10 +3,9 @@
 import logging as log
 import sys
 import threading
-import unittest
-from io import StringIO
 from types import ModuleType
-from unittest.mock import patch
+
+import pytest
 
 import beets.logging as blog
 from beets import plugins, ui
@@ -14,8 +13,10 @@ from beets.test import _common, helper
 from beets.test.helper import AsIsImporterMixin, ImportTestCase, PluginMixin
 
 
-class LoggingTest(unittest.TestCase):
-    def test_logging_management(self):
+class TestStrFormatLogger:
+    """Tests for the custom str-formatting logger."""
+
+    def test_logger_creation(self):
         l1 = log.getLogger("foo123")
         l2 = blog.getLogger("foo123")
         assert l1 == l2
@@ -35,17 +36,33 @@ class LoggingTest(unittest.TestCase):
         l6 = blog.getLogger()
         assert l1 != l6
 
-    def test_str_format_logging(self):
-        logger = blog.getLogger("baz123")
-        stream = StringIO()
-        handler = log.StreamHandler(stream)
+    @pytest.mark.parametrize(
+        "level", [log.DEBUG, log.INFO, log.WARNING, log.ERROR]
+    )
+    @pytest.mark.parametrize(
+        "msg, args, kwargs, expected",
+        [
+            ("foo {} bar {}", ("oof", "baz"), {}, "foo oof bar baz"),
+            (
+                "foo {bar} baz {foo}",
+                (),
+                {"foo": "oof", "bar": "baz"},
+                "foo baz baz oof",
+            ),
+            ("no args", (), {}, "no args"),
+            ("foo {} bar {baz}", ("oof",), {"baz": "baz"}, "foo oof bar baz"),
+        ],
+    )
+    def test_str_format_logging(
+        self, level, msg, args, kwargs, expected, caplog
+    ):
+        logger = blog.getLogger("test_logger")
+        logger.setLevel(level)
 
-        logger.addHandler(handler)
-        logger.propagate = False
+        with caplog.at_level(level, logger="test_logger"):
+            logger.log(level, msg, *args, **kwargs)
 
-        logger.warning("foo {} {bar}", "oof", bar="baz")
-        handler.flush()
-        assert stream.getvalue(), "foo oof baz"
+        assert str(caplog.records[0].msg) == expected
 
 
 class DummyModule(ModuleType):