[GitHub] [airflow] jedcunningham commented on a diff in pull request #29052: Fix KubernetesPodOperator xcom push when `get_logs=False`

["jedcunningham (via GitHub)" <gi...@apache.org>]

jedcunningham commented on code in PR #29052:
URL: https://github.com/apache/airflow/pull/29052#discussion_r1117814554


##########
tests/providers/cncf/kubernetes/utils/test_pod_manager.py:
##########
@@ -346,6 +351,24 @@ def test_pod_manager_get_client_call_deprecation(self):
                 "and make kube_client a required argument."
             )
 
+    @pytest.mark.parametrize(
+        "container_state, is_terminated", [("waiting", False), ("running", False), ("terminated", True)]
+    )
+    def test_container_is_terminated_with_waiting_state(self, container_state, is_terminated):
+        container_status = MagicMock()
+        container_status.configure_mock(
+            **{
+                "name": "base",
+                "state.waiting": True if container_state == "waiting" else None,
+                "state.running": True if container_state == "running" else None,
+                "state.terminated": True if container_state == "terminated" else None,
+            }
+        )
+        pod_info = MagicMock()
+        pod_info.status.container_statuses = [container_status]
+        is_terminated = container_is_terminated(pod_info, "base")
+        assert is_terminated == is_terminated

Review Comment:
   ```suggestion
           assert container_is_terminated(pod_info, "base") == is_terminated
   ```
   
   You are overwriting the `is_terminated` you are parameterizing! (I personally like using `expected_` for expectations that are parameterized, but feel free to do it however you'd like)



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org