[GitHub] [airflow] potiuk commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`

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

potiuk commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054659263


##########
tests/always/test_connection.py:
##########
@@ -38,38 +36,214 @@
 ConnectionParts = namedtuple("ConnectionParts", ["conn_type", "login", "password", "host", "port", "schema"])
 
 
-class UriTestCaseConfig:
-    def __init__(
-        self,
-        test_conn_uri: str,
-        test_conn_attributes: dict,
-        description: str,
-    ):
-        """
-
-        :param test_conn_uri: URI that we use to create connection
-        :param test_conn_attributes: we expect a connection object created with `test_uri` to have these
-        attributes
-        :param description: human-friendly name appended to parameterized test
-        """
-        self.test_uri = test_conn_uri
-        self.test_conn_attributes = test_conn_attributes
-        self.description = description
-
-    @staticmethod
-    def uri_test_name(func, num, param):
-        return f"{func.__name__}_{num}_{param.args[0].description.replace(' ', '_')}"
-
-
-class TestConnection(unittest.TestCase):
-    def setUp(self):
+TEST_FROM_URI_PARAMS = [
+    # test_conn_uri: URI that we use to create connection
+    # test_conn_attributes: we expect a connection object created with `test_uri` to have these attributes
+    pytest.param(
+        "scheme://user:password@host%2Flocation:1234/schema",
+        {
+            "conn_type": "scheme",
+            "host": "host/location",
+            "schema": "schema",
+            "login": "user",
+            "password": "password",
+            "port": 1234,
+            "extra": None,
+        },
+        id="without extras",
+    ),
+    pytest.param(
+        "scheme://user:password@host%2Flocation:1234/schema?extra1=a%20value&extra2=%2Fpath%2F",
+        {
+            "conn_type": "scheme",
+            "host": "host/location",
+            "schema": "schema",
+            "login": "user",
+            "password": "password",
+            "port": 1234,
+            "extra_dejson": {"extra1": "a value", "extra2": "/path/"},
+        },
+        id="with extras",
+    ),
+    pytest.param(
+        "scheme://user:password@host%2Flocation:1234/schema?__extra__=single+value",
+        {
+            "conn_type": "scheme",
+            "host": "host/location",
+            "schema": "schema",
+            "login": "user",
+            "password": "password",
+            "port": 1234,
+            "extra": "single value",
+        },
+        id="with extras single value",

Review Comment:
   We can always reorganize and refactor those tests later - l;et's not do both changing the type of tests and refactoring the way how they are defined in one PR
   
   This is also a question of DRY vs DAMP - I  think in many cases for tests DRY does not work that well. if you have to look at several places (inevitable with DRY) to understand what is the expected test input and output, it's worse than if you have the inputs and outputs together (and no common code that you also have to look-up). It's really nice to see all the cases with input/output  clearly grouped together.



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