[jira] [Resolved] (AIRFLOW-5720) don't call _get_connections_from_db in TestCloudSqlDatabaseHook

["Kamil Bregula (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/AIRFLOW-5720?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kamil Bregula resolved AIRFLOW-5720.
------------------------------------
    Resolution: Won't Fix

Resolved in different way: https://github.com/apache/airflow/pull/6390#issuecomment-554045581

> don't call _get_connections_from_db in TestCloudSqlDatabaseHook
> ---------------------------------------------------------------
>
>                 Key: AIRFLOW-5720
>                 URL: https://issues.apache.org/jira/browse/AIRFLOW-5720
>             Project: Apache Airflow
>          Issue Type: New Feature
>          Components: gcp
>    Affects Versions: 1.10.5
>            Reporter: Daniel Standish
>            Assignee: Daniel Standish
>            Priority: Major
>
> Issues with this test class:
> *tests are mocking lower-level than they need to*
> Tests were mocking {{airflow.hook.BaseHook.get_connections}}.
> Instead they can mock {{airflow.gcp.hooks.cloud_sql.CloudSqlDatabaseHook.get_connection}} which is more direct.
> *should not reference private method*
> This is an impediment to refactoring of connections / creds.
> *Tests had complexity that did not add a benefit*
> They all had this bit:
> {code:python}
>         self._setup_connections(get_connections, uri)
>         gcp_conn_id = 'google_cloud_default'
>         hook = CloudSqlDatabaseHook(
>             default_gcp_project_id=BaseHook.get_connection(gcp_conn_id).extra_dejson.get(
>                 'extra__google_cloud_platform__project')
>         )
> {code}
> {{_setup_connections}} was like this:
> {code:python}
>     @staticmethod
>     def _setup_connections(get_connections, uri):
>         gcp_connection = mock.MagicMock()
>         gcp_connection.extra_dejson = mock.MagicMock()
>         gcp_connection.extra_dejson.get.return_value = 'empty_project'
>         cloudsql_connection = Connection()
>         cloudsql_connection.parse_from_uri(uri)
>         cloudsql_connection2 = Connection()
>         cloudsql_connection2.parse_from_uri(uri)
>         get_connections.side_effect = [[gcp_connection], [cloudsql_connection],
>                                        [cloudsql_connection2]]
> {code}
> Issues here are as follows.
> 1. no test ever used the third side effect
> 2. the first side effect does not help us; {{default_gcp_project_id}} is irrelevant
> Only one of the three connections in {{_setup_connections}} has any impact on the test.
> The call of {{BaseHook.get_connection}} only serves to discard the first connection in mock side effect list, {{gcp_connection}}.  
> The second connection is the one that matters, and it is returned when {{CloudSqlDatabaseHook}} calls `self.get_connection` during init.  Since it is a mock side effect, it doesn't matter what value is given for conn_id.  So the {{CloudSqlDatabaseHook}} init param {{default_gcp_project_id}} has no consequence.  And because it has no consequence, we should not supply a value  for it because this is misleading.
> We should not have extra code that does not serve a purpose because it makes it harder to understand what's actually happening.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)