[GitHub] [airflow] mik-laj edited a comment on issue #5958: [AIRFLOW-5335] Simplify GCSHook test

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services