[GitHub] [airflow] turbaszek commented on a change in pull request #8450: Simplify mocking in BigQueryHook tests

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

turbaszek commented on a change in pull request #8450: Simplify mocking in BigQueryHook tests
URL: https://github.com/apache/airflow/pull/8450#discussion_r410901706
 
 

 ##########
 File path: tests/providers/google/cloud/hooks/test_bigquery.py
 ##########
 @@ -37,152 +36,128 @@
 LOCATION = 'europe-north1'
 
 
-class TestBigQueryHookMethods(unittest.TestCase):
-    @mock.patch(
-        'airflow.providers.google.common.hooks.base_google.GoogleBaseHook._get_credentials_and_project_id',
-        return_value=(CREDENTIALS, PROJECT_ID)
-    )
+class _BigQueryBaseTestClass(unittest.TestCase):
+    def setUp(self) -> None:
+        class MockedBigQueryHook(BigQueryHook):
+            def _get_credentials_and_project_id(self):
+                return CREDENTIALS, PROJECT_ID
+
+        self.hook = MockedBigQueryHook()
+
+
+class TestBigQueryHookMethods(_BigQueryBaseTestClass):
     @mock.patch("airflow.providers.google.cloud.hooks.bigquery.BigQueryConnection")
     @mock.patch("airflow.providers.google.cloud.hooks.bigquery.BigQueryHook._authorize")
     @mock.patch("airflow.providers.google.cloud.hooks.bigquery.build")
     def test_bigquery_client_creation(
-        self, mock_build, mock_authorize, mock_bigquery_connection, mock_get_creds_and_proj_id
+        self, mock_build, mock_authorize, mock_bigquery_connection
     ):
-        bq_hook = hook.BigQueryHook()
-        result = bq_hook.get_conn()
+        result = self.hook.get_conn()
         mock_build.assert_called_once_with(
             'bigquery', 'v2', http=mock_authorize.return_value, cache_discovery=False
         )
         mock_bigquery_connection.assert_called_once_with(
             service=mock_build.return_value,
             project_id=PROJECT_ID,
-            hook=bq_hook,
-            use_legacy_sql=bq_hook.use_legacy_sql,
-            location=bq_hook.location,
-            num_retries=bq_hook.num_retries
+            hook=self.hook,
+            use_legacy_sql=self.hook.use_legacy_sql,
+            location=self.hook.location,
+            num_retries=self.hook.num_retries
         )
         self.assertEqual(mock_bigquery_connection.return_value, result)
 
-    @mock.patch(
-        'airflow.providers.google.common.hooks.base_google.GoogleBaseHook._get_credentials_and_project_id',
-        return_value=(CREDENTIALS, PROJECT_ID)
-    )
     @mock.patch("airflow.providers.google.common.hooks.base_google.GoogleBaseHook.__init__")
     def test_bigquery_bigquery_conn_id_deprecation_warning(
-        self, mock_base_hook_init, mock_get_creds_and_proj_id
+        self, mock_base_hook_init,
     ):
         bigquery_conn_id = "bigquery conn id"
         warning_message = "The bigquery_conn_id parameter has been deprecated. " \
                           "You should pass the gcp_conn_id parameter."
         with self.assertWarns(DeprecationWarning) as warn:
-            hook.BigQueryHook(bigquery_conn_id=bigquery_conn_id)
+            BigQueryHook(bigquery_conn_id=bigquery_conn_id)
             mock_base_hook_init.assert_called_once_with(delegate_to=None, gcp_conn_id='bigquery conn id')
         self.assertEqual(warning_message, str(warn.warning))
 
-    @mock.patch(
-        'airflow.providers.google.common.hooks.base_google.GoogleBaseHook._get_credentials_and_project_id',
-        return_value=(CREDENTIALS, PROJECT_ID)
-    )
-    def test_bigquery_insert_rows_not_implemented(self, mock_get_creds_and_proj_id):
-        bq_hook = hook.BigQueryHook()
+    @mock.patch("airflow.providers.google.cloud.hooks.bigquery.BigQueryHook.get_service")
+    @mock.patch("airflow.providers.google.cloud.hooks.bigquery.BigQueryHook.run_with_configuration")
+    def test_location_propagates_properly(
+        self, run_with_config, mock_get_service
+    ):
+        self.assertIsNone(self.hook.location)
+        self.hook.run_query(sql='select 1', location='US')
+        assert run_with_config.call_count == 1
+        self.assertEqual(self.hook.location, 'US')
 
 Review comment:
   On hook level - yes. On test level I don't think so because setup creates new hook before every test. 

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