dabla commented on code in PR #58698:
URL: https://github.com/apache/airflow/pull/58698#discussion_r2563578830


##########
providers/google/tests/unit/google/cloud/hooks/test_datafusion.py:
##########
@@ -340,42 +340,34 @@ def 
test_list_pipelines_should_fail_if_status_not_200(self, mock_request, hook):
     @mock.patch(HOOK_STR.format("DataFusionHook._cdap_request"))
     def test_start_pipeline(self, mock_request, hook):
         run_id = 1234
-        mock_request.return_value = mock.MagicMock(status=200, 
data=f'[{{"runId":{run_id}}}]')
+        mock_request.return_value = mock.MagicMock(status=200, 
data=f'{{"runId":{run_id}}}')

Review Comment:
   Great PR, love the way how you refactored it.  Just one little improvement 
which wouldn't hurt is to spec the MagicMock with 
google.auth.transport.requests.Request, that way static checks can see if 
mocked methods actually exists for that class.



##########
providers/google/tests/unit/google/cloud/hooks/test_datafusion.py:
##########
@@ -409,16 +395,29 @@ def 
test_start_pipeline_should_fail_if_status_not_200(self, mock_request, hook):
             hook.start_pipeline(
                 pipeline_name=PIPELINE_NAME, instance_url=INSTANCE_URL, 
runtime_args=RUNTIME_ARGS
             )
-        body = [
-            {
-                "appId": PIPELINE_NAME,
-                "programType": "workflow",
-                "programId": "DataPipelineWorkflow",
-                "runtimeargs": RUNTIME_ARGS,
-            }
-        ]
         mock_request.assert_called_once_with(
-            url=f"{INSTANCE_URL}/v3/namespaces/default/start", method="POST", 
body=body
+            
url=f"{INSTANCE_URL}/v3/namespaces/default/apps/{PIPELINE_NAME}/workflows/DataPipelineWorkflow/start",
+            method="POST",
+            body=RUNTIME_ARGS,
+        )
+
+    @mock.patch(HOOK_STR.format("DataFusionHook._cdap_request"))
+    def test_start_pipeline_should_fail_if_no_run_id(self, mock_request, hook):
+        """Test that start_pipeline fails gracefully when response doesn't 
contain runId."""
+        error_response = '{"error": "Invalid runtime arguments"}'
+        mock_request.return_value = mock.MagicMock(status=200, 
data=error_response)

Review Comment:
   Same here.



-- 
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]

Reply via email to