Dev-iL commented on code in PR #75016:
URL: https://github.com/apache/airflow/pull/75016#discussion_r4308316891
##########
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/utils/test_pod_manager.py:
##########
@@ -467,6 +468,21 @@ def test_read_pod_events_without_resource_version(self):
resource_version_match=None,
)
+ @pytest.mark.asyncio
+ @mock.patch.object(PodManager, "read_pod_events", autospec=True,
side_effect=ApiException(status=504))
+ async def test_watch_pod_events_stops_when_reading_events_fails(self,
mock_read, caplog):
Review Comment:
nit/suggestion: the failure test at `utils/test_pod_manager.py:473` raises
from `read_pod_events`, so it would still pass if the `try` block stopped
before event processing. Add a case where retrieval succeeds and
`log_pod_event` raises, then assert that the watcher warns and returns. Cover
the async manager with the same failure condition to protect the full
best-effort boundary.
##########
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/triggers/test_pod.py:
##########
@@ -584,6 +593,34 @@ async def
test_run_loop_returns_failed_when_pod_failed_and_base_container_waits(
}
)
+ @pytest.mark.asyncio
+ @pytest.mark.parametrize(
+ ("log_events_on_startup", "expected_calls"),
+ [(True, 1), (False, 0)],
+ )
+ @mock.patch(f"{TRIGGER_PATH}.define_pod_container_state")
Review Comment:
nit/suggestion: the new patches at `triggers/test_pod.py:601` omit
`autospec`, and the new pod result uses an unrestricted `MagicMock`. Add
`autospec=True` to the patched methods and a pod spec to the returned mock so
the test catches incompatible method calls and misspelled pod attributes. Apply
the same convention to the other mocks introduced by this PR, as required by
`AGENTS.md`.
--
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]
