Skip to content

Commit

Permalink
logging: Fix log formatting with colored output
Browse files Browse the repository at this point in the history
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 <[email protected]>
Commit-Queue: Shik Chen <[email protected]>
Reviewed-by: Mike Frysinger <[email protected]>
  • Loading branch information
ShikChen authored and LUCI committed Jul 2, 2024
1 parent 87f52f3 commit 9bf8236
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
4 changes: 2 additions & 2 deletions repo_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
37 changes: 37 additions & 0 deletions tests/test_repo_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -62,3 +67,35 @@ def test_log_aggregated_errors_logs_single_error(self, mock_error):
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}",
)

0 comments on commit 9bf8236

Please sign in to comment.