Taragolis commented on code in PR #28039:
URL: https://github.com/apache/airflow/pull/28039#discussion_r1037814650
##########
tests/providers/amazon/aws/hooks/test_batch_client.py:
##########
@@ -299,16 +300,17 @@ def test_job_splunk_logs(self):
}
]
}
- with self.assertLogs(level="WARNING") as capture_logs:
+ with caplog.at_level(level=logging.WARNING):
+ caplog.clear()
assert self.batch_client.get_job_awslogs_info(JOB_ID) is None
- assert len(capture_logs.records) == 1
+ assert len(caplog.messages) == 1
-class TestBatchClientDelays(unittest.TestCase):
+class TestBatchClientDelays:
@mock.patch.dict("os.environ", AWS_DEFAULT_REGION=AWS_REGION)
@mock.patch.dict("os.environ", AWS_ACCESS_KEY_ID=AWS_ACCESS_KEY_ID)
@mock.patch.dict("os.environ", AWS_SECRET_ACCESS_KEY=AWS_SECRET_ACCESS_KEY)
- def setUp(self):
+ def setup_method(self, method):
Review Comment:
> Is it also needed for mocks that "generate" no arguments, like the
@mock.patch.dict ?
And even with this case we need 2 arguments
```
args =
(<tests.providers.amazon.aws.hooks.test_batch_client.TestBatchClientDelays
object at 0x1071f0f70>, <bound method TestB...tDelays.test_init of
<tests.providers.amazon.aws.hooks.test_batch_client.TestBatchClientDelays
object at 0x1071f0f70>>)
kw = {}
@wraps(f)
def _inner(*args, **kw):
self._patch_dict()
try:
> return f(*args, **kw)
E TypeError: setup_method() takes 1 positional argument but 2 were
given
```
##########
tests/providers/amazon/aws/hooks/test_glue_crawler.py:
##########
@@ -83,18 +82,17 @@
}
-class TestGlueCrawlerHook(unittest.TestCase):
- @classmethod
- def setUp(cls):
- cls.hook = GlueCrawlerHook(aws_conn_id="aws_default")
+class TestGlueCrawlerHook:
+ def setup_method(self):
+ self.hook = GlueCrawlerHook(aws_conn_id="aws_default")
def test_init(self):
- self.assertEqual(self.hook.aws_conn_id, "aws_default")
+ assert self.hook.aws_conn_id == "aws_default"
@mock.patch.object(GlueCrawlerHook, "get_conn")
def test_has_crawler(self, mock_get_conn):
response = self.hook.has_crawler(mock_crawler_name)
- self.assertEqual(response, True)
+ assert response
Review Comment:
Good point 👍
##########
tests/providers/amazon/aws/hooks/test_glacier.py:
##########
@@ -47,25 +47,21 @@ def test_retrieve_inventory_should_return_job_id(self,
mock_conn):
assert job_id == result
@mock.patch("airflow.providers.amazon.aws.hooks.glacier.GlacierHook.get_conn")
- def test_retrieve_inventory_should_log_mgs(self, mock_conn):
+ def test_retrieve_inventory_should_log_mgs(self, mock_conn, caplog):
# given
Review Comment:
Right now main target just migrate to `pytests`.
It is good point to change something from the past 😉
To be honest a lot of test need to rewrite later anyway
##########
tests/providers/amazon/aws/hooks/test_datasync.py:
##########
@@ -50,21 +49,13 @@ def test_get_conn(self):
@mock_datasync
@mock.patch.object(DataSyncHook, "get_conn")
-class TestDataSyncHookMocked(unittest.TestCase):
- def __init__(self, *args, **kwargs):
- super().__init__(*args, **kwargs)
- self.source_server_hostname = "host"
- self.source_subdirectory = "somewhere"
- self.destination_bucket_name = "my_bucket"
- self.destination_bucket_dir = "dir"
+class TestDataSyncHookMocked:
+ source_server_hostname = "host"
+ source_subdirectory = "somewhere"
+ destination_bucket_name = "my_bucket"
+ destination_bucket_dir = "dir"
- self.client = None
- self.hook = None
- self.source_location_arn = None
- self.destination_location_arn = None
- self.task_arn = None
-
- def setUp(self):
+ def setup_method(self, method):
Review Comment:
Original @vandonr-amz
> Ah yes I imagine because of the order of arguments, but I imagine in
test_datasync it's not needed ?
Actually this arg needed because we decorated entire class
```
test setup failed
args =
(<tests.providers.amazon.aws.hooks.test_datasync.TestDataSyncHookMocked object
at 0x109fdf4c0>, <bound method TestData...HookMocked.test_init of
<tests.providers.amazon.aws.hooks.test_datasync.TestDataSyncHookMocked object
at 0x109fdf4c0>>)
kwargs = {}
def wrapper(*args, **kwargs):
self.start(reset=reset)
try:
> result = func(*args, **kwargs)
E TypeError: setup_method() takes 1 positional argument but 2 were
given
```
##########
tests/providers/amazon/aws/hooks/test_batch_waiters.py:
##########
@@ -317,12 +316,12 @@ def test_batch_job_waiting(aws_clients, aws_region,
job_queue_name, job_definiti
assert job_status == "SUCCEEDED"
-class TestBatchWaiters(unittest.TestCase):
+class TestBatchWaiters:
@mock.patch.dict("os.environ", AWS_DEFAULT_REGION=AWS_REGION)
@mock.patch.dict("os.environ", AWS_ACCESS_KEY_ID=AWS_ACCESS_KEY_ID)
@mock.patch.dict("os.environ", AWS_SECRET_ACCESS_KEY=AWS_SECRET_ACCESS_KEY)
@mock.patch("airflow.providers.amazon.aws.hooks.batch_client.AwsBaseHook.get_client_type")
- def setUp(self, get_client_type_mock):
+ def setup_method(self, method, get_client_type_mock):
Review Comment:
Original @vandonr-amz
> and nit: I like using _ as name for the arguments that are unused, it's a
clear way to mark them as "only here for the compiler to be happy"
This mostly as reminder what the [actual argument
here](https://docs.pytest.org/en/6.2.x/xunit_setup.html#method-and-function-level-setup-teardown).
--
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]
