henry3260 commented on code in PR #60258:
URL: https://github.com/apache/airflow/pull/60258#discussion_r2672010403
##########
shared/logging/tests/logging/test_percent_formatter.py:
##########
@@ -29,3 +29,21 @@ def test_no_callsite(self):
formatted = fmter(mock.Mock(name="Logger"), "info", {"event": "our
msg"})
assert formatted == "(unknown file):0 our msg"
+
+ def test_percent_formatter_handles_none_values(self):
+ """Test that PercentFormatRender handles None values gracefully."""
+ fmter = PercentFormatRender("%(filename)s:%(lineno)d %(message)s")
+
+ # Mock logger and event_dict with None values
+ logger = mock.Mock(name="Logger")
+ event_dict = {
+ "event": "test message",
+ "filename": None,
+ "lineno": None,
+ }
Review Comment:
> Is this accurately representing the event dict that is created, or would
it be this?
>
> ```python
> event_dict = {
> "event": "test message",
> }
> ```
Actually, I believe the current test case `("lineno": None)` is the accurate
representation because the original traceback showed a TypeError, not a
KeyError."
"If the key were missing from the event dict (as in your snippet), the
string formatting would raise a KeyError. The fact that we see a TypeError
confirms that the key exists in the dictionary but holds a None value. So I
think explicitly setting it to None in the test is correct. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
