logging: Fix log formatting with colored output

The log message is already formatted before being passed to the colorer.
To avoid the exception "TypeError: not enough arguments for format
string", we should use the `nofmt_colorer` instead.

This bug occurs only when the formatted string still contains '%'
character. The following snippet can reproduce the bug:

```
from repo_logging import RepoLogger
RepoLogger(__name__).error("%s", "100% failed")
```

Change-Id: I4e3977b3d21aec4e0deb95fc1c6dd1e59272d695
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/432017
Tested-by: Shik Chen <shik@google.com>
Commit-Queue: Shik Chen <shik@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
This commit is contained in:
Shik Chen 2024-07-01 18:51:33 +08:00 committed by LUCI
parent 87f52f308c
commit 9bf8236c24
2 changed files with 39 additions and 2 deletions

View File

@ -39,8 +39,8 @@ class _LogColoring(Coloring):
def __init__(self, config):
super().__init__(config, "logs")
self.error = self.colorer("error", fg="red")
self.warning = self.colorer("warn", fg="yellow")
self.error = self.nofmt_colorer("error", fg="red")
self.warning = self.nofmt_colorer("warn", fg="yellow")
self.levelMap = {
"WARNING": self.warning,
"ERROR": self.error,

View File

@ -13,9 +13,14 @@
# limitations under the License.
"""Unit test for repo_logging module."""
import contextlib
import io
import logging
import unittest
from unittest import mock
from color import SetDefaultColoring
from error import RepoExitError
from repo_logging import RepoLogger
@ -62,3 +67,35 @@ class TestRepoLogger(unittest.TestCase):
mock.call("Repo command failed: %s", "RepoExitError"),
]
)
def test_log_with_format_string(self):
"""Test different log levels with format strings."""
# Set color output to "always" for consistent test results.
# This ensures the logger's behavior is uniform across different
# environments and git configurations.
SetDefaultColoring("always")
# Regex pattern to match optional ANSI color codes.
# \033 - Escape character
# \[ - Opening square bracket
# [0-9;]* - Zero or more digits or semicolons
# m - Ending 'm' character
# ? - Makes the entire group optional
opt_color = r"(\033\[[0-9;]*m)?"
for level in (logging.INFO, logging.WARN, logging.ERROR):
name = logging.getLevelName(level)
with self.subTest(level=level, name=name):
output = io.StringIO()
with contextlib.redirect_stderr(output):
logger = RepoLogger(__name__)
logger.log(level, "%s", "100% pass")
self.assertRegex(
output.getvalue().strip(),
f"^{opt_color}100% pass{opt_color}$",
f"failed for level {name}",
)