Taragolis commented on code in PR #27696:
URL: https://github.com/apache/airflow/pull/27696#discussion_r1023859753
##########
tests/test_utils/asserts.py:
##########
@@ -19,22 +19,38 @@
import logging
import re
import traceback
+import warnings
from collections import Counter
from contextlib import contextmanager
+from typing import TYPE_CHECKING
from sqlalchemy import event
# Long import to not create a copy of the reference, but to refer to one place.
import airflow.settings
+if TYPE_CHECKING:
+ from unittest import TestCase
+
log = logging.getLogger(__name__)
-def assert_equal_ignore_multiple_spaces(case, first, second, msg=None):
+def assert_equal_ignore_multiple_spaces(case: TestCase | None, first, second,
msg=None):
def _trim(s):
return re.sub(r"\s+", " ", s.strip())
- return case.assertEqual(_trim(first), _trim(second), msg)
+ if case:
+ warnings.warn(
+ "Passing `case` has no effect and will be remove in the future. "
+ "Please set to `None` for avoid this warning.",
+ FutureWarning,
+ stacklevel=3,
+ )
+
+ if not msg:
+ assert _trim(first) == _trim(second)
+ else:
+ assert _trim(first) == _trim(second), msg
Review Comment:
Replace usage `unittests.TestCase.assertEqual` by assert.
Because some of tests use this helper (google, amazon, apache) I add warning
as a reminder for further migration to `pytest`.
##########
tests/providers/sftp/hooks/test_sftp.py:
##########
@@ -180,11 +178,11 @@ def test_no_host_key_check_default(self, get_connection):
@mock.patch("airflow.providers.sftp.hooks.sftp.SFTPHook.get_connection")
def test_no_host_key_check_enabled(self, get_connection):
- connection = Connection(login="login", host="host",
extra='{"no_host_key_check": false}')
+ connection = Connection(login="login", host="host",
extra='{"no_host_key_check": true}')
get_connection.return_value = connection
hook = SFTPHook()
- assert hook.no_host_key_check is False
+ assert hook.no_host_key_check is True
Review Comment:
Fix this test.
It might be a typo because it had same implementations as
`test_no_host_key_check_disabled` initially
##########
tests/providers/sftp/hooks/test_sftp.py:
##########
@@ -385,41 +385,45 @@ def test_invalid_ssh_hook(self, mock_get_connection):
with pytest.raises(AirflowException, match="ssh_hook must be an
instance of SSHHook"):
connection = Connection(conn_id="sftp_default", login="root",
host="localhost")
mock_get_connection.return_value = connection
- SFTPHook(ssh_hook="invalid_hook") # type: ignore
+ with pytest.warns(DeprecationWarning, match=r"Parameter `ssh_hook`
is deprecated.*"):
+ SFTPHook(ssh_hook="invalid_hook")
@mock.patch("airflow.providers.ssh.hooks.ssh.SSHHook.get_connection")
def test_valid_ssh_hook(self, mock_get_connection):
connection = Connection(conn_id="sftp_test", login="root",
host="localhost")
mock_get_connection.return_value = connection
- hook = SFTPHook(ssh_hook=SSHHook(ssh_conn_id="sftp_test"))
+ with pytest.warns(DeprecationWarning, match=r"Parameter `ssh_hook` is
deprecated.*"):
+ hook = SFTPHook(ssh_hook=SSHHook(ssh_conn_id="sftp_test"))
assert hook.ssh_conn_id == "sftp_test"
assert isinstance(hook.get_conn(), paramiko.SFTPClient)
def test_get_suffix_pattern_match(self):
output = self.hook.get_file_by_pattern(TMP_PATH, "*.txt")
- self.assertTrue(output, TMP_FILE_FOR_TESTS)
+ # In CI files might have different name, so we check that file found
rather than actual name
+ assert output, TMP_FILE_FOR_TESTS
Review Comment:
Just for now I keep same logic for this test: "Returns any file without
check expected".
Seems like in CI and local breeze there are different set of files as result
```python
assert output == TMP_FILE_FOR_TESTS
```
Pass in the local environment and failed in the CI with error
```text
E AssertionError: assert 'remote_path.txt' == 'test_file.txt'
E - test_file.txt
E + remote_path.txt
```
--
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]
