ferruzzi commented on code in PR #56578:
URL: https://github.com/apache/airflow/pull/56578#discussion_r2434008684
##########
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/hooks/test_kubernetes.py:
##########
@@ -760,9 +760,11 @@ def test_check_kueue_deployment_running(self,
mock_get_deployment_status, mock_l
@mock.patch(HOOK_MODULE + ".KubernetesHook.log")
@mock.patch(HOOK_MODULE + ".KubernetesHook.get_deployment_status")
def test_check_kueue_deployment_raise_exception(self,
mock_get_deployment_status, mock_log):
- mock_get_deployment_status.side_effect = ValueError
+ mock_get_deployment_status.side_effect = ValueError(
+ "Exception occurred while checking for Deployment status."
+ )
- with pytest.raises(ValueError):
+ with pytest.raises(ValueError, match="Exception occurred while
checking for Deployment status."):
Review Comment:
Right, but the way this test is written, we're just saying
```
a = 5
assert a = 5
```
Seems there should be a better way. But maybe not. I guess we're mocking
one layer down at `get_deployment_status` and calling
`check_kueue_deployment_running` so maybe it's still a valid test.
I would still prefer to see the message string synchronized, maybe set it as
a global in the hook file and import it, that way if it ever gets changed, the
test still accurately reflects the code.
--
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]
