potiuk commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054691773
##########
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:
> About DRY, right now a lot of providers tests usually do the the same
things:
>
> * Mock Airflow Connections
> * Mock their clients
> * Create task and execute them
> Most of this stuff could be achived by couple fixtures or context
managers. Unfortunetly right now structure of internal framework split across
tests package. My suggestion move all generic stuff into separate package
module and inject them as `pytest` plugin.
Yep. It would be good if we can do some generalisation. But we have to take
into account that we plan at some point of time to split providers out -
possibly even to a separate repo per provider - and for sure we want to
restructure the providers to become independent standalone "sub-folders" as an
interim step -
https://lists.apache.org/thread/3s5tn1wnvo0cw9vofwmbjl0rkyvhrtbx and there any
common code might become challenging. Pytest plugins might actually help with
it.
> If we talk about this part it is a bit tricky generate some stuff because
within this test cases we tests our Connection model and how it transform URI
-> Connection.
Yeah I also find it a but challenging how to do it "dry-er" vs loosing some
of hte DAMPness.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]