You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/12/19 11:34:25 UTC
[GitHub] [airflow] Taragolis opened a new pull request, #28459: Migrate remaining Always & CLI tests to `pytest`
Taragolis opened a new pull request, #28459:
URL: https://github.com/apache/airflow/pull/28459
Migrate remaining Always & CLI tests to `pytest` in selected directories
- `tests/always/*`
- `tests/cli/*`
_As usual add findings, info about significant changes and potential follow up in comments_
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1052116651
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
I thought `TestProjectStructure` should be re-worked, I miss the main idea of this test so it would be nice if someone could share the idea about this tests
---
`test_deprecated_packages` - I thought this test not properly worked since actual files replaced by PEP-562 getattr - https://github.com/apache/airflow/pull/26153
---
`test_providers_modules_should_have_tests` - This test broken for a while, generators wrapped into generators and when it actually transformed to set this line broke `expected_test_files` calculation and it always `set([])`
https://github.com/apache/airflow/blob/e377e869da9f0e42ac1e0a615347cf7cd6565d54/tests/always/test_project_structure.py#L87
Rather than raise an error in this test, I add print warning instead of until we fix or rewrite this test logic
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054660574
##########
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:
But if we can find a good way also having similar properties and less code - that's cool
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054698636
##########
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:
Ah yeah. I missed that :( ... sorry, been a bit swamed recently :)
--
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
[GitHub] [airflow] dimberman commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
dimberman commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054644548
##########
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:
There seems to be a lot of repeat code here. Is there any way of compressing this into a generator?
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054684991
##########
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.
In this case we could also test our fixtures/test context managers and helpers functions if we want.
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.
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054700534
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
`test_providers_modules_should_have_tests` after changes by this PR shows this warning
```console
Detect missing tests in providers module.
Modules Files: 566, Current Test Files: 583, Missing Tests Files: 97.
tests/providers/amazon/aws/hooks/test_dms.py
tests/providers/amazon/aws/links/test_base_aws.py
tests/providers/amazon/aws/links/test_batch.py
tests/providers/amazon/aws/links/test_emr.py
tests/providers/amazon/aws/links/test_logs.py
tests/providers/amazon/aws/operators/test_aws_lambda.py
tests/providers/amazon/aws/operators/test_dms.py
tests/providers/amazon/aws/operators/test_emr.py
tests/providers/amazon/aws/operators/test_s3.py
tests/providers/amazon/aws/operators/test_sagemaker.py
tests/providers/amazon/aws/sensors/test_dms.py
tests/providers/amazon/aws/sensors/test_ecs.py
tests/providers/amazon/aws/sensors/test_emr.py
tests/providers/amazon/aws/sensors/test_s3.py
tests/providers/amazon/aws/sensors/test_sagemaker.py
tests/providers/amazon/aws/test_exceptions.py
tests/providers/amazon/aws/utils/test_rds.py
tests/providers/amazon/aws/waiters/test_base_waiter.py
tests/providers/apache/cassandra/hooks/test_cassandra.py
tests/providers/apache/druid/operators/test_druid_check.py
tests/providers/cncf/kubernetes/backcompat/test_backwards_compat_converters.py
tests/providers/cncf/kubernetes/test_python_kubernetes_script.py
tests/providers/cncf/kubernetes/utils/test_xcom_sidecar.py
tests/providers/databricks/hooks/test_databricks_base.py
tests/providers/databricks/utils/test_databricks.py
tests/providers/dbt/cloud/hooks/test_dbt.py
tests/providers/dbt/cloud/operators/test_dbt.py
tests/providers/dbt/cloud/sensors/test_dbt.py
tests/providers/elasticsearch/log/test_es_json_formatter.py
tests/providers/google/cloud/links/test_base.py
tests/providers/google/cloud/links/test_bigquery.py
tests/providers/google/cloud/links/test_bigquery_dts.py
tests/providers/google/cloud/links/test_bigtable.py
tests/providers/google/cloud/links/test_cloud_build.py
tests/providers/google/cloud/links/test_cloud_functions.py
tests/providers/google/cloud/links/test_cloud_memorystore.py
tests/providers/google/cloud/links/test_cloud_sql.py
tests/providers/google/cloud/links/test_cloud_storage_transfer.py
tests/providers/google/cloud/links/test_cloud_tasks.py
tests/providers/google/cloud/links/test_compute.py
tests/providers/google/cloud/links/test_data_loss_prevention.py
tests/providers/google/cloud/links/test_datacatalog.py
tests/providers/google/cloud/links/test_dataflow.py
tests/providers/google/cloud/links/test_dataform.py
tests/providers/google/cloud/links/test_dataplex.py
tests/providers/google/cloud/links/test_dataprep.py
tests/providers/google/cloud/links/test_dataproc.py
tests/providers/google/cloud/links/test_datastore.py
tests/providers/google/cloud/links/test_kubernetes_engine.py
tests/providers/google/cloud/links/test_life_sciences.py
tests/providers/google/cloud/links/test_mlengine.py
tests/providers/google/cloud/links/test_pubsub.py
tests/providers/google/cloud/links/test_spanner.py
tests/providers/google/cloud/links/test_stackdriver.py
tests/providers/google/cloud/links/test_vertex_ai.py
tests/providers/google/cloud/links/test_workflows.py
tests/providers/google/cloud/operators/vertex_ai/test_auto_ml.py
tests/providers/google/cloud/operators/vertex_ai/test_batch_prediction_job.py
tests/providers/google/cloud/operators/vertex_ai/test_custom_job.py
tests/providers/google/cloud/operators/vertex_ai/test_dataset.py
tests/providers/google/cloud/operators/vertex_ai/test_endpoint_service.py
tests/providers/google/cloud/operators/vertex_ai/test_hyperparameter_tuning_job.py
tests/providers/google/cloud/operators/vertex_ai/test_model_service.py
tests/providers/google/cloud/sensors/test_dataform.py
tests/providers/google/cloud/transfers/test_presto_to_gcs.py
tests/providers/google/cloud/transfers/test_trino_to_gcs.py
tests/providers/google/cloud/triggers/test_cloud_composer.py
tests/providers/google/cloud/triggers/test_dataproc.py
tests/providers/google/cloud/utils/test_bigquery.py
tests/providers/google/cloud/utils/test_bigquery_get_data.py
tests/providers/google/cloud/utils/test_dataform.py
tests/providers/google/common/links/test_storage.py
tests/providers/google/common/test_consts.py
tests/providers/google/test_go_module_utils.py
tests/providers/microsoft/azure/hooks/test_batch.py
tests/providers/microsoft/azure/hooks/test_container_instance.py
tests/providers/microsoft/azure/hooks/test_container_registry.py
tests/providers/microsoft/azure/hooks/test_container_volume.py
tests/providers/microsoft/azure/hooks/test_cosmos.py
tests/providers/microsoft/azure/hooks/test_data_factory.py
tests/providers/microsoft/azure/hooks/test_data_lake.py
tests/providers/microsoft/azure/hooks/test_fileshare.py
tests/providers/microsoft/azure/hooks/test_synapse.py
tests/providers/microsoft/azure/operators/test_adls.py
tests/providers/microsoft/azure/operators/test_batch.py
tests/providers/microsoft/azure/operators/test_container_instances.py
tests/providers/microsoft/azure/operators/test_cosmos.py
tests/providers/microsoft/azure/operators/test_data_factory.py
tests/providers/microsoft/azure/operators/test_synapse.py
tests/providers/microsoft/azure/secrets/test_key_vault.py
tests/providers/microsoft/azure/sensors/test_cosmos.py
tests/providers/microsoft/azure/sensors/test_data_factory.py
tests/providers/mongo/sensors/test_mongo.py
tests/providers/presto/transfers/test_gcs_to_presto.py
tests/providers/redis/operators/test_redis_publish.py
tests/providers/redis/sensors/test_redis_key.py
tests/providers/trino/transfers/test_gcs_to_trino.py
```
Some of them about deprecated modules
--
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
[GitHub] [airflow] Taragolis closed pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis closed pull request #28459: Migrate remaining Always & CLI tests to `pytest`
URL: https://github.com/apache/airflow/pull/28459
--
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
[GitHub] [airflow] potiuk commented on pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28459:
URL: https://github.com/apache/airflow/pull/28459#issuecomment-1385342609
@dimberman ? What do you think?
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by 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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1057582689
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
> * test_deprecated_packages -> we should indeed remove, they are no problem any more
Done
> * Test...ProviderProjectStructure - we should remove them - they bring no value and having then for few providers in here makes littel sense
Done, keep only `ProjectStructureTest` because `ProjectStructureTest` depends on it
> * ExampleCoverageTest - similarly
Removed
> * AssetsCoverageTest -> I think this is largely used by Google team. Naybe @bhirsz might tell more about it (though in any case I thik if it is useful for Google it should go to google provider tests.
Keep for a while
> * test_providers_modules_should_have_tests -> is the only one I find useful from those for now but if it's broken, it might be we are far-off and fixing it would be problematic.
Will try to fix (make it run again) as follow up
--
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
[GitHub] [airflow] Taragolis commented on pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on PR #28459:
URL: https://github.com/apache/airflow/pull/28459#issuecomment-1405644763
👀 😢 👀
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054653729
##########
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:
I don't think there is any problem to generate tests cases:
- Functions
- Generators
- Fixtures
- [pytest-cases](https://smarie.github.io/python-pytest-cases/)
Right now general idea migrate all tests to `pytest` it is sill remaining about 100-120 modules (most of them sits into Google Provider).
Also all code generation could contain some bugs and complicated logic
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054720051
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
Yeah. Sounds like a good idea.
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054695454
##########
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:
Yeah I've also suggest in dev-list about separate generic tests parts into package this should also help split providers into separate sub-folders and even in separate repository because in this case all of them would have access to this package (we just nee install them) and all tests configurations which live inside `tests/conftest.py` could be moved into it.
Of course it is in theory and I plan to create PoC
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1052116651
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
I thought `TestProjectStructure` should be re-worked, I miss the main idea of this test so it would be nice if someone could share the idea about this tests
---
`test_deprecated_packages` - I thought this test not properly worked since actual files replaced by PEP-562 getattr - https://github.com/apache/airflow/pull/26153
---
`test_providers_modules_should_have_tests` - This test broken for a while, generators wrapped into generators and when it actually transformed to set this line broke `expected_test_files` calculation and it always `set([])`
https://github.com/apache/airflow/blob/e377e869da9f0e42ac1e0a615347cf7cd6565d54/tests/always/test_project_structure.py#L87
Instead of raise an error in this test I print warning until we fix or rewrite this test logic
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054663312
##########
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:
@dimberman - BTW. Do you have some propoal in mind ? Maybe some example that you can show how it could look like ? I would be interested to see how this could be done nicer ?
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054708003
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
- some of tests moved into tests.integrations
- Amazon links tested in one go in same module
- Some of amazon tests split across different test modules, for example: test_s3_bucket.py + test_s3_bucket_tagging.py + test_s3_file_transform.py + test_s3_list.py + test_s3_list_prefixes.py + test_s3_object.py but module is s3.py (merged version)
So in theory there is no problem to fix some issue, add tests.integration as search path and add temporary exclusion for remaining and turn on this test again
--
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
[GitHub] [airflow] potiuk commented on pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #28459:
URL: https://github.com/apache/airflow/pull/28459#issuecomment-1397453882
LGTM. @dimberman -> do you still have doubts? Or should we merge this one?
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054700534
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
`test_providers_modules_should_have_tests` after changes by this PR shows this warning
```console
Detect missing tests in providers module.
Modules Files: 566, Current Test Files: 583, Missing Tests Files: 97.
tests/providers/amazon/aws/hooks/test_dms.py
tests/providers/amazon/aws/links/test_base_aws.py
tests/providers/amazon/aws/links/test_batch.py
tests/providers/amazon/aws/links/test_emr.py
tests/providers/amazon/aws/links/test_logs.py
tests/providers/amazon/aws/operators/test_aws_lambda.py
tests/providers/amazon/aws/operators/test_dms.py
tests/providers/amazon/aws/operators/test_emr.py
tests/providers/amazon/aws/operators/test_s3.py
tests/providers/amazon/aws/operators/test_sagemaker.py
tests/providers/amazon/aws/sensors/test_dms.py
tests/providers/amazon/aws/sensors/test_ecs.py
tests/providers/amazon/aws/sensors/test_emr.py
tests/providers/amazon/aws/sensors/test_s3.py
tests/providers/amazon/aws/sensors/test_sagemaker.py
tests/providers/amazon/aws/test_exceptions.py
tests/providers/amazon/aws/utils/test_rds.py
tests/providers/amazon/aws/waiters/test_base_waiter.py
tests/providers/apache/cassandra/hooks/test_cassandra.py
tests/providers/apache/druid/operators/test_druid_check.py
tests/providers/cncf/kubernetes/backcompat/test_backwards_compat_converters.py
tests/providers/cncf/kubernetes/test_python_kubernetes_script.py
tests/providers/cncf/kubernetes/utils/test_xcom_sidecar.py
tests/providers/databricks/hooks/test_databricks_base.py
tests/providers/databricks/utils/test_databricks.py
tests/providers/dbt/cloud/hooks/test_dbt.py
tests/providers/dbt/cloud/operators/test_dbt.py
tests/providers/dbt/cloud/sensors/test_dbt.py
tests/providers/elasticsearch/log/test_es_json_formatter.py
tests/providers/google/cloud/links/test_base.py
tests/providers/google/cloud/links/test_bigquery.py
tests/providers/google/cloud/links/test_bigquery_dts.py
tests/providers/google/cloud/links/test_bigtable.py
tests/providers/google/cloud/links/test_cloud_build.py
tests/providers/google/cloud/links/test_cloud_functions.py
tests/providers/google/cloud/links/test_cloud_memorystore.py
tests/providers/google/cloud/links/test_cloud_sql.py
tests/providers/google/cloud/links/test_cloud_storage_transfer.py
tests/providers/google/cloud/links/test_cloud_tasks.py
tests/providers/google/cloud/links/test_compute.py
tests/providers/google/cloud/links/test_data_loss_prevention.py
tests/providers/google/cloud/links/test_datacatalog.py
tests/providers/google/cloud/links/test_dataflow.py
tests/providers/google/cloud/links/test_dataform.py
tests/providers/google/cloud/links/test_dataplex.py
tests/providers/google/cloud/links/test_dataprep.py
tests/providers/google/cloud/links/test_dataproc.py
tests/providers/google/cloud/links/test_datastore.py
tests/providers/google/cloud/links/test_kubernetes_engine.py
tests/providers/google/cloud/links/test_life_sciences.py
tests/providers/google/cloud/links/test_mlengine.py
tests/providers/google/cloud/links/test_pubsub.py
tests/providers/google/cloud/links/test_spanner.py
tests/providers/google/cloud/links/test_stackdriver.py
tests/providers/google/cloud/links/test_vertex_ai.py
tests/providers/google/cloud/links/test_workflows.py
tests/providers/google/cloud/operators/vertex_ai/test_auto_ml.py
tests/providers/google/cloud/operators/vertex_ai/test_batch_prediction_job.py
tests/providers/google/cloud/operators/vertex_ai/test_custom_job.py
tests/providers/google/cloud/operators/vertex_ai/test_dataset.py
tests/providers/google/cloud/operators/vertex_ai/test_endpoint_service.py
tests/providers/google/cloud/operators/vertex_ai/test_hyperparameter_tuning_job.py
tests/providers/google/cloud/operators/vertex_ai/test_model_service.py
tests/providers/google/cloud/sensors/test_dataform.py
tests/providers/google/cloud/transfers/test_presto_to_gcs.py
tests/providers/google/cloud/transfers/test_trino_to_gcs.py
tests/providers/google/cloud/triggers/test_cloud_composer.py
tests/providers/google/cloud/triggers/test_dataproc.py
tests/providers/google/cloud/utils/test_bigquery.py
tests/providers/google/cloud/utils/test_bigquery_get_data.py
tests/providers/google/cloud/utils/test_dataform.py
tests/providers/google/common/links/test_storage.py
tests/providers/google/common/test_consts.py
tests/providers/google/test_go_module_utils.py
tests/providers/microsoft/azure/hooks/test_batch.py
tests/providers/microsoft/azure/hooks/test_container_instance.py
tests/providers/microsoft/azure/hooks/test_container_registry.py
tests/providers/microsoft/azure/hooks/test_container_volume.py
tests/providers/microsoft/azure/hooks/test_cosmos.py
tests/providers/microsoft/azure/hooks/test_data_factory.py
tests/providers/microsoft/azure/hooks/test_data_lake.py
tests/providers/microsoft/azure/hooks/test_fileshare.py
tests/providers/microsoft/azure/hooks/test_synapse.py
tests/providers/microsoft/azure/operators/test_adls.py
tests/providers/microsoft/azure/operators/test_batch.py
tests/providers/microsoft/azure/operators/test_container_instances.py
tests/providers/microsoft/azure/operators/test_cosmos.py
tests/providers/microsoft/azure/operators/test_data_factory.py
tests/providers/microsoft/azure/operators/test_synapse.py
tests/providers/microsoft/azure/secrets/test_key_vault.py
tests/providers/microsoft/azure/sensors/test_cosmos.py
tests/providers/microsoft/azure/sensors/test_data_factory.py
tests/providers/mongo/sensors/test_mongo.py
tests/providers/presto/transfers/test_gcs_to_presto.py
tests/providers/redis/operators/test_redis_publish.py
tests/providers/redis/sensors/test_redis_key.py
tests/providers/trino/transfers/test_gcs_to_trino.py'
```
--
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
[GitHub] [airflow] Taragolis commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054708003
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
- some of tests moved into tests.integrations
- Amazon links tested in one go in same module
- Some of amazon tests split across different test modules, for example: `test_s3_bucket.py` + `test_s3_bucket_tagging.py` + `test_s3_file_transform.py` + `test_s3_list.py` + `test_s3_list_prefixes.py` + `test_s3_object.py` but module named as `s3.py` (merged version)
So in theory there is no problem to fix some issue, add tests.integration as search path and add temporary exclusion for remaining and turn on this test again
--
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
[GitHub] [airflow] potiuk commented on a diff in pull request #28459: Migrate remaining Always & CLI tests to `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28459:
URL: https://github.com/apache/airflow/pull/28459#discussion_r1054681194
##########
tests/always/test_project_structure.py:
##########
@@ -30,68 +30,79 @@
)
-class TestProjectStructure(unittest.TestCase):
+class TestProjectStructure:
Review Comment:
Most of this module come from our past renaming we've doe in [AIP-21](https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-21%3A+Changes+in+import+paths) - and some of them can be removed or simplified or moved elsewhere as they never caught up as "common provider" approach:
* test_deprecated_packages -> we should indeed remove, they are no problem any more
* Test...ProviderProjectStructure - we should remove them - they bring no value and having then for few providers in here makes littel sense
* ExampleCoverageTest - similarly
* AssetsCoverageTest -> I think this is largely used by Google team. Naybe @bhirsz might tell more about it (though in any case I thik if it is useful for Google it should go to google provider tests.
* test_providers_modules_should_have_tests -> is the only one I find useful from those for now but if it's broken, it might be we are far-off and fixing it would be problematic.
--
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