[GitHub] [airflow] dstandish commented on a change in pull request #19695: Add config and context params to KubernetesHook

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

dstandish commented on a change in pull request #19695:
URL: https://github.com/apache/airflow/pull/19695#discussion_r752822542



##########
File path: tests/providers/cncf/kubernetes/hooks/test_kubernetes.py
##########
@@ -34,100 +32,124 @@
 from tests.test_utils.db import clear_db_connections
 
 
-class TestKubernetesHook(unittest.TestCase):
+class TestKubernetesHook:
     @classmethod
-    def setUpClass(cls) -> None:
-        db.merge_conn(
-            Connection(
-                conn_id='kubernetes_in_cluster',
-                conn_type='kubernetes',
-                extra=json.dumps({'extra__kubernetes__in_cluster': True}),
-            )
-        )
-        db.merge_conn(
-            Connection(
-                conn_id='kubernetes_kube_config',
-                conn_type='kubernetes',
-                extra=json.dumps({'extra__kubernetes__kube_config': '{"test": "kube"}'}),
-            )
-        )
-        db.merge_conn(
-            Connection(
-                conn_id='kubernetes_kube_config_path',
-                conn_type='kubernetes',
-                extra=json.dumps({'extra__kubernetes__kube_config_path': 'path/to/file'}),
-            )
-        )
-        db.merge_conn(
-            Connection(conn_id='kubernetes_default_kube_config', conn_type='kubernetes', extra=json.dumps({}))
-        )
-        db.merge_conn(
-            Connection(
-                conn_id='kubernetes_with_namespace',
-                conn_type='kubernetes',
-                extra=json.dumps({'extra__kubernetes__namespace': 'mock_namespace'}),
-            )
-        )
+    def setup_class(cls) -> None:
+        for conn_id, extra in [
+            ('in_cluster', {'extra__kubernetes__in_cluster': True}),
+            ('kube_config', {'extra__kubernetes__kube_config': '{"test": "kube"}'}),
+            ('kube_config_path', {'extra__kubernetes__kube_config_path': 'path/to/file'}),
+            ('in_cluster_empty', {'extra__kubernetes__in_cluster': ''}),
+            ('kube_config_empty', {'extra__kubernetes__kube_config': ''}),
+            ('kube_config_path_empty', {'extra__kubernetes__kube_config_path': ''}),
+            ('with_namespace', {'extra__kubernetes__namespace': 'mock_namespace'}),
+            ('default_kube_config', {}),
+        ]:
+            db.merge_conn(Connection(conn_type='kubernetes', conn_id=conn_id, extra=json.dumps(extra)))
 
     @classmethod
-    def tearDownClass(cls) -> None:
+    def teardown_class(cls) -> None:
         clear_db_connections()
 
+    @pytest.mark.parametrize(
+        'in_cluster_param, conn_id, in_cluster_called',
+        (
+            pytest.param(None, 'in_cluster', True),
+            pytest.param(True, 'in_cluster', True),
+            pytest.param(False, 'in_cluster', False),
+            pytest.param(None, 'in_cluster_empty', False),
+            pytest.param(True, 'in_cluster_empty', True),
+            pytest.param(False, 'in_cluster_empty', False),
+        ),
+    )
     @patch("kubernetes.config.incluster_config.InClusterConfigLoader")
-    def test_in_cluster_connection(self, mock_kube_config_loader):
-        kubernetes_hook = KubernetesHook(conn_id='kubernetes_in_cluster')
+    def test_in_cluster_connection(
+        self, mock_kube_config_loader, in_cluster_param, conn_id, in_cluster_called
+    ):
+        kubernetes_hook = KubernetesHook(conn_id=conn_id, in_cluster=in_cluster_param)
         api_conn = kubernetes_hook.get_conn()
-        mock_kube_config_loader.assert_called_once()
+        if in_cluster_called:
+            mock_kube_config_loader.assert_called_once()
+        else:
+            mock_kube_config_loader.assert_not_called()
         assert isinstance(api_conn, kubernetes.client.api_client.ApiClient)
 
+    @pytest.mark.parametrize(
+        'config_path_param, conn_id, call_path',
+        (
+            pytest.param(None, 'kube_config_path', 'path/to/file'),
+            pytest.param('/my/path/override', 'kube_config_path', '/my/path/override'),
+            pytest.param(None, 'kube_config_path_empty', '~/.kube/config'),
+            pytest.param('/my/path/override', 'kube_config_path_empty', '/my/path/override'),
+        ),
+    )
     @patch("kubernetes.config.kube_config.KubeConfigLoader")
     @patch("kubernetes.config.kube_config.KubeConfigMerger")
-    def test_kube_config_path(self, mock_kube_config_loader, mock_kube_config_merger):
-        kubernetes_hook = KubernetesHook(conn_id='kubernetes_kube_config_path')
+    def test_kube_config_path(
+        self, mock_kube_config_merger, mock_kube_config_loader, config_path_param, conn_id, call_path
+    ):
+        kubernetes_hook = KubernetesHook(conn_id=conn_id, config_file=config_path_param)
         api_conn = kubernetes_hook.get_conn()
-        mock_kube_config_loader.assert_called_once_with("path/to/file")

Review comment:
       it seems that merger and loader were backwards in some of these tests




-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org