diff options
author | Shik Chen <shik@google.com> | 2024-07-01 18:51:33 +0800 |
---|---|---|
committer | LUCI <gerrit-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2024-07-02 06:24:31 +0000 |
commit | 9bf8236c24839045787fa284471fab950485285c (patch) | |
tree | c0fb602875d822447a460b08e8e193bd42d7f31c | |
parent | 87f52f308c48c11a99cefcc308a0994abeb2a7ff (diff) | |
download | git-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.py | 4 | ||||
-rw-r--r-- | tests/test_repo_logging.py | 37 |
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 | |||
17 | import contextlib | ||
18 | import io | ||
19 | import logging | ||
16 | import unittest | 20 | import unittest |
17 | from unittest import mock | 21 | from unittest import mock |
18 | 22 | ||
23 | from color import SetDefaultColoring | ||
19 | from error import RepoExitError | 24 | from error import RepoExitError |
20 | from repo_logging import RepoLogger | 25 | from 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 | ) | ||