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/11/27 07:49:30 UTC
[GitHub] [airflow] Taragolis opened a new pull request, #27948: Replace `unittests` in providers-apache tests by pure `pytest`
Taragolis opened a new pull request, #27948:
URL: https://github.com/apache/airflow/pull/27948
Migrate all Apache providers tests to `pytest`.
All changes are more or less straightforward:
- Get rid of `unittests.TestCase` class and **TestCase.assert*** methods
- Replace decorator `parameterized.expand` by `pytest.mark.parametrize`. I
- replace `requests_mock.mock` decorator by `requests_mock` fixture
- Convert `TestCase.subTest` to parametrize tests
- Convert classes **setUp*** and **tearDown*** to [appropriate pytest methods](https://docs.pytest.org/en/6.2.x/xunit_setup.html#classic-xunit-style-setup)
_See additional findings 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] potiuk commented on a diff in pull request #27948: Replace `unittests` in providers-apache tests by pure `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #27948:
URL: https://github.com/apache/airflow/pull/27948#discussion_r1033021896
##########
tests/providers/apache/drill/operators/test_drill.py:
##########
@@ -32,13 +30,13 @@
@pytest.mark.backend("drill")
-class TestDrillOperator(unittest.TestCase):
- def setUp(self):
Review Comment:
Indeed. I think drill has been missing in action :)
--
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 #27948: Replace `unittests` in providers-apache tests by pure `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #27948:
URL: https://github.com/apache/airflow/pull/27948#discussion_r1033022025
##########
tests/providers/apache/hdfs/sensors/test_hdfs.py:
##########
@@ -97,21 +96,22 @@ def test_legacy_file_does_not_exists(self):
task.execute(None)
-class TestHdfsSensorFolder(unittest.TestCase):
- def setUp(self):
+class TestHdfsSensorFolder:
+ def setup_method(self, method):
self.hook = FakeHDFSHook
- self.log = logging.getLogger()
- self.log.setLevel(logging.DEBUG)
+
+ logger = logging.getLogger(__name__)
+ logger.setLevel(logging.DEBUG)
+ logger.debug("#" * 10)
+ logger.debug("Running test case: %s.%s", self.__class__.__name__, method.__name__)
+ logger.debug("#" * 10)
Review Comment:
Yep. We can get rid of it as a 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] potiuk commented on a diff in pull request #27948: Replace `unittests` in providers-apache tests by pure `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #27948:
URL: https://github.com/apache/airflow/pull/27948#discussion_r1033026270
##########
tests/providers/apache/hive/hooks/test_hive.py:
##########
@@ -55,23 +53,7 @@ def __init__(self):
self.iterable = []
-class TestHiveEnvironment(unittest.TestCase):
- def setUp(self):
- self.next_day = (DEFAULT_DATE + datetime.timedelta(days=1)).isoformat()[:10]
- self.database = "airflow"
- self.partition_by = "ds"
- self.table = "static_babynames_partitioned"
- with mock.patch(
- "airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.get_metastore_client"
- ) as get_metastore_mock, mock.patch(
- "airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.get_connection"
- ):
- get_metastore_mock.return_value = mock.MagicMock()
-
- self.hook = HiveMetastoreHook()
Review Comment:
Yep.
--
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 merged pull request #27948: Replace `unittests` in providers-apache tests by pure `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #27948:
URL: https://github.com/apache/airflow/pull/27948
--
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 #27948: Replace `unittests` in providers-apache tests by pure `pytest`
Posted by GitBox <gi...@apache.org>.
Taragolis commented on code in PR #27948:
URL: https://github.com/apache/airflow/pull/27948#discussion_r1032884418
##########
tests/providers/apache/hdfs/hooks/test_webhdfs.py:
##########
@@ -85,61 +84,39 @@ def test_get_conn_with_schema(self, socket_mock, mock_get_connection, mock_insec
mock_insecure_client.return_value.status.assert_called_once_with("/")
assert conn == mock_insecure_client.return_value
- @patch("airflow.providers.apache.hdfs.hooks.webhdfs.requests.Session", return_value="session")
- @patch("airflow.providers.apache.hdfs.hooks.webhdfs.InsecureClient")
- @patch(
- "airflow.providers.apache.hdfs.hooks.webhdfs.WebHDFSHook.get_connection",
- return_value=Connection(host="host_1.com,host_2.com", login="user"),
+ @pytest.mark.parametrize(
+ "test_connection",
+ [
+ pytest.param(Connection(host="host_1.com,host_2.com", login="user"), id="without-password"),
+ pytest.param(
+ Connection(host="host_1.com,host_2.com", login="user", password="password"),
+ id="with-password",
+ ),
+ ],
Review Comment:
Merge two identical test implementation (`test_get_conn_without_port_schema` and `test_get_conn_with_password_without_port_schema`) into single one
##########
tests/providers/apache/livy/hooks/test_livy.py:
##########
@@ -17,34 +17,55 @@
from __future__ import annotations
Review Comment:
A lot of changes in this module:
1. Make sure that changes in tests-connections reflected in tests
2. Replace all SubTests
3. replace `requests_mock.mock` decorator by `requests_mock` fixture
4. Add additional test parametrisation
##########
tests/providers/apache/hive/hooks/test_hive.py:
##########
@@ -55,23 +53,7 @@ def __init__(self):
self.iterable = []
-class TestHiveEnvironment(unittest.TestCase):
- def setUp(self):
- self.next_day = (DEFAULT_DATE + datetime.timedelta(days=1)).isoformat()[:10]
- self.database = "airflow"
- self.partition_by = "ds"
- self.table = "static_babynames_partitioned"
- with mock.patch(
- "airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.get_metastore_client"
- ) as get_metastore_mock, mock.patch(
- "airflow.providers.apache.hive.hooks.hive.HiveMetastoreHook.get_connection"
- ):
- get_metastore_mock.return_value = mock.MagicMock()
-
- self.hook = HiveMetastoreHook()
Review Comment:
This class only use once in the same module and also have the same name as reusable test class from `tests/providers/apache/hive/__init__.py` which may confuse
##########
tests/providers/apache/hdfs/sensors/test_hdfs.py:
##########
@@ -97,21 +96,22 @@ def test_legacy_file_does_not_exists(self):
task.execute(None)
-class TestHdfsSensorFolder(unittest.TestCase):
- def setUp(self):
+class TestHdfsSensorFolder:
+ def setup_method(self, method):
self.hook = FakeHDFSHook
- self.log = logging.getLogger()
- self.log.setLevel(logging.DEBUG)
+
+ logger = logging.getLogger(__name__)
+ logger.setLevel(logging.DEBUG)
+ logger.debug("#" * 10)
+ logger.debug("Running test case: %s.%s", self.__class__.__name__, method.__name__)
+ logger.debug("#" * 10)
Review Comment:
This part previously exists in tests case methods.
Is a good idea get rid of this logger entirely?
##########
tests/providers/apache/hdfs/sensors/test_hdfs.py:
##########
@@ -200,21 +191,22 @@ def test_should_be_non_empty_directory_fail(self):
task.execute(None)
-class TestHdfsSensorRegex(unittest.TestCase):
- def setUp(self):
+class TestHdfsSensorRegex:
+ def setup_method(self, method):
self.hook = FakeHDFSHook
- self.log = logging.getLogger()
- self.log.setLevel(logging.DEBUG)
+
+ logger = logging.getLogger(__name__)
+ logger.setLevel(logging.DEBUG)
+ logger.debug("#" * 10)
+ logger.debug("Running test case: %s.%s", self.__class__.__name__, method.__name__)
+ logger.debug("#" * 10)
Review Comment:
Same as mention for `TestHdfsSensorFolder`
##########
tests/providers/apache/drill/operators/test_drill.py:
##########
@@ -32,13 +30,13 @@
@pytest.mark.backend("drill")
-class TestDrillOperator(unittest.TestCase):
- def setUp(self):
Review Comment:
`TestDrillOperator` has a bit strange decorator might be some typo or outdated stuff:
- Backend `drill` not exists
- Integration `drill` not exists but might be exists in the past
--
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 #27948: Replace `unittests` in providers-apache tests by pure `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #27948:
URL: https://github.com/apache/airflow/pull/27948#discussion_r1033022109
##########
tests/providers/apache/hdfs/sensors/test_hdfs.py:
##########
@@ -200,21 +191,22 @@ def test_should_be_non_empty_directory_fail(self):
task.execute(None)
-class TestHdfsSensorRegex(unittest.TestCase):
- def setUp(self):
+class TestHdfsSensorRegex:
+ def setup_method(self, method):
self.hook = FakeHDFSHook
- self.log = logging.getLogger()
- self.log.setLevel(logging.DEBUG)
+
+ logger = logging.getLogger(__name__)
+ logger.setLevel(logging.DEBUG)
+ logger.debug("#" * 10)
+ logger.debug("Running test case: %s.%s", self.__class__.__name__, method.__name__)
+ logger.debug("#" * 10)
Review Comment:
Same here. This seems to be a left-over and we should get rid of it as a 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] potiuk commented on pull request #27948: Replace `unittests` in providers-apache tests by pure `pytest`
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27948:
URL: https://github.com/apache/airflow/pull/27948#issuecomment-1328362660
Yeah. I really like the pytest conversions :)
--
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