michaelmicheal commented on code in PR #22092:
URL: https://github.com/apache/airflow/pull/22092#discussion_r899149802
##########
tests/providers/cncf/kubernetes/operators/test_kubernetes_pod.py:
##########
@@ -270,7 +270,10 @@ def test_image_pull_policy_correctly_set(self):
assert pod.spec.containers[0].image_pull_policy == "Always"
@mock.patch("airflow.providers.cncf.kubernetes.utils.pod_manager.PodManager.delete_pod")
- def test_pod_delete_even_on_launcher_error(self, delete_pod_mock):
+
@mock.patch("airflow.providers.cncf.kubernetes.operators.kubernetes_pod.KubernetesPodOperator.find_pod")
+ def test_pod_delete_even_on_launcher_error(self, find_pod_mock,
delete_pod_mock):
+ remote_pod_mock = MagicMock()
+ find_pod_mock.return_value = remote_pod_mock
Review Comment:
As I understand it, this test asserts that the `delete_pod` method is called
when the pod fails to launch. Without mocking the `find_pod` method,
`remote_pod` is set to `None`.
```python
# finding remote pod to ensure it exists
remote_pod = self.find_pod(self.pod.metadata.namespace, context=context)
```
Since this PR adds logic that only deletes when the `remote_pod` is not
None, the `delete_pod` method is not called and this test fails without mocking
the `find_pod` method.
That being said I think these lines are redundant and I'll remove them.
```python
remote_pod_mock = MagicMock()
find_pod_mock.return_value = remote_pod_mock
```
--
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]
