summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShik Chen <shik@google.com>2024-07-01 18:51:33 +0800
committerLUCI <gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com>2024-07-02 06:24:31 +0000
commit9bf8236c24839045787fa284471fab950485285c (patch)
treec0fb602875d822447a460b08e8e193bd42d7f31c
parent87f52f308c48c11a99cefcc308a0994abeb2a7ff (diff)
downloadgit-repo-9bf8236c24839045787fa284471fab950485285c.tar.gz
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>
-rw-r--r--repo_logging.py4
-rw-r--r--tests/test_repo_logging.py37
2 files changed, 39 insertions, 2 deletions
diff --git a/repo_logging.py b/repo_logging.py
index 20a53429..639382a2 100644
--- a/repo_logging.py
+++ b/repo_logging.py
@@ -39,8 +39,8 @@ class _LogColoring(Coloring):
39 39
40 def __init__(self, config): 40 def __init__(self, config):
41 super().__init__(config, "logs") 41 super().__init__(config, "logs")
42 self.error = self.colorer("error", fg="red") 42 self.error = self.nofmt_colorer("error", fg="red")
43 self.warning = self.colorer("warn", fg="yellow") 43 self.warning = self.nofmt_colorer("warn", fg="yellow")
44 self.levelMap = { 44 self.levelMap = {
45 "WARNING": self.warning, 45 "WARNING": self.warning,
46 "ERROR": self.error, 46 "ERROR": self.error,
diff --git a/tests/test_repo_logging.py b/tests/test_repo_logging.py
index 0f6a3355..e072039e 100644
--- a/tests/test_repo_logging.py
+++ b/tests/test_repo_logging.py
@@ -13,9 +13,14 @@
13# limitations under the License. 13# limitations under the License.
14 14
15"""Unit test for repo_logging module.""" 15"""Unit test for repo_logging module."""
16
17import contextlib
18import io
19import logging
16import unittest 20import unittest
17from unittest import mock 21from unittest import mock
18 22
23from color import SetDefaultColoring
19from error import RepoExitError 24from error import RepoExitError
20from repo_logging import RepoLogger 25from repo_logging import RepoLogger
21 26
@@ -62,3 +67,35 @@ class TestRepoLogger(unittest.TestCase):
62 mock.call("Repo command failed: %s", "RepoExitError"), 67 mock.call("Repo command failed: %s", "RepoExitError"),
63 ] 68 ]
64 ) 69 )
70
71 def test_log_with_format_string(self):
72 """Test different log levels with format strings."""
73
74 # Set color output to "always" for consistent test results.
75 # This ensures the logger's behavior is uniform across different
76 # environments and git configurations.
77 SetDefaultColoring("always")
78
79 # Regex pattern to match optional ANSI color codes.
80 # \033 - Escape character
81 # \[ - Opening square bracket
82 # [0-9;]* - Zero or more digits or semicolons
83 # m - Ending 'm' character
84 # ? - Makes the entire group optional
85 opt_color = r"(\033\[[0-9;]*m)?"
86
87 for level in (logging.INFO, logging.WARN, logging.ERROR):
88 name = logging.getLevelName(level)
89
90 with self.subTest(level=level, name=name):
91 output = io.StringIO()
92
93 with contextlib.redirect_stderr(output):
94 logger = RepoLogger(__name__)
95 logger.log(level, "%s", "100% pass")
96
97 self.assertRegex(
98 output.getvalue().strip(),
99 f"^{opt_color}100% pass{opt_color}$",
100 f"failed for level {name}",
101 )