mik-laj edited a comment on issue #5958: [AIRFLOW-5335] Simplify GCSHook test
URL: https://github.com/apache/airflow/pull/5958#issuecomment-526510218
 
 
   ```python
       @mock.patch(
        
BASE_STRING.format("GoogleCloudBaseHook._get_credentials_and_project_id"), 
        return_value=("CREDENTIALS", "PROJECT_ID")
       )
       @mock.patch('google.cloud.storage.Client')
       def test_storage_client_creation(self, mock_client, 
mock_get_credentials_and_project_id):
           hook = gcs_hook.GoogleCloudStorageHook()
           result = hook.get_conn()
           # test that Storage Client is called with required arguments
           mock_client.assert_called_once_with(
               client_info=hook.client_info,
               credentials="CREDENTIALS",
               project="PROJECT_ID")
           self.assertEqual(mock_client.return_value, result)
   ```
   I looked deeper and I have more comments about this code. I think we should 
mock another method because it calls the external library. I think it's worth 
adding assertions for the result. The code is clearer when only one method of 
mocking is used - the decorator. Using context manager and decorator in one 
test is pointless.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to