Replace `unittests` in providers tests by pure `pytest` [Wave-3] #27696

Taragolis · 2022-11-15T17:51:27Z

Follow-up: #26831, #26992

Attempt to replace unittests.TestCase by native pytest classes into providers tests except this:

google (146 modules)
amazon (88 modules)
apache (33 modules)
microsoft (27 modules)

All changes are straight forward:

Get rid of unittests.TestCase class and TestCase.assert* methods
Replace decorator parameterized.expand by pytest.mark.parametrize. I cant see any benefits if compare to builtin pytest.
Rename ClassNameTest to TestClassName
replace requests_mock.mock decorator by requests_mock fixture

uranusjr

Didn’t read everything but nothing pops up from a few sampling.

Taragolis

Add info about change logic in tests or tests helpers

Taragolis · 2022-11-16T11:13:17Z

tests/test_utils/asserts.py

+    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


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.

Taragolis · 2022-11-16T11:20:06Z

tests/providers/sftp/hooks/test_sftp.py

        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


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

assert output == TMP_FILE_FOR_TESTS

Pass in the local environment and failed in the CI with error

E AssertionError: assert 'remote_path.txt' == 'test_file.txt' E - test_file.txt E + remote_path.txt

Taragolis · 2022-11-16T11:22:01Z

tests/providers/sftp/hooks/test_sftp.py

    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


Fix this test.
It might be a typo because it had same implementations as test_no_host_key_check_disabled initially

Taragolis · 2022-11-17T07:25:06Z

tests/providers/common/sql/sensors/test_sql.py

-    @unittest.skipIf(
-        "AIRFLOW_RUNALL_TESTS" not in os.environ, "Skipped because AIRFLOW_RUNALL_TESTS is not set"
+    @pytest.mark.skipif(
+        "AIRFLOW_RUNALL_TESTS" not in os.environ, reason="Skipped because AIRFLOW_RUNALL_TESTS is not set"


I have no idea is variable AIRFLOW_RUNALL_TESTS set anywhere because I couldn't find any reference outside of tests in main. This variable also check in some other tests

I found that initially this env added by this PR #7468 and info about it should store into the https://issues.apache.org/jira/browse/AIRFLOW-6847 (unfortunately I do not have access).

When I tried set export AIRFLOW_RUNALL_TESTS=x in breeze and run tests I've always get same error, regardless of whether migrated to pytest or not

ERROR airflow.task:taskinstance.py:1749 Task failed with exception Traceback (most recent call last): File "/usr/local/lib/python3.7/site-packages/urllib3/connection.py", line 175, in _new_conn (self._dns_host, self.port), self.timeout, **extra_kw File "/usr/local/lib/python3.7/site-packages/urllib3/util/connection.py", line 95, in create_connection raise err File "/usr/local/lib/python3.7/site-packages/urllib3/util/connection.py", line 85, in create_connection sock.connect(sa) ConnectionRefusedError: [Errno 111] Connection refused During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/usr/local/lib/python3.7/site-packages/urllib3/connectionpool.py", line 710, in urlopen chunked=chunked, File "/usr/local/lib/python3.7/site-packages/urllib3/connectionpool.py", line 398, in _make_request conn.request(method, url, **httplib_request_kw) File "/usr/local/lib/python3.7/site-packages/urllib3/connection.py", line 239, in request super(HTTPConnection, self).request(method, url, body=body, headers=headers) File "/usr/local/lib/python3.7/http/client.py", line 1281, in request self._send_request(method, url, body, headers, encode_chunked) File "/usr/local/lib/python3.7/http/client.py", line 1327, in _send_request self.endheaders(body, encode_chunked=encode_chunked) File "/usr/local/lib/python3.7/http/client.py", line 1276, in endheaders self._send_output(message_body, encode_chunked=encode_chunked) File "/usr/local/lib/python3.7/http/client.py", line 1036, in _send_output self.send(msg) File "/usr/local/lib/python3.7/http/client.py", line 976, in send self.connect() File "/usr/local/lib/python3.7/site-packages/urllib3/connection.py", line 205, in connect conn = self._new_conn() File "/usr/local/lib/python3.7/site-packages/urllib3/connection.py", line 187, in _new_conn self, "Failed to establish a new connection: %s" % e urllib3.exceptions.NewConnectionError: <urllib3.connection.HTTPConnection object at 0x7f4a91d33e10>: Failed to establish a new connection: [Errno 111] Connection refused During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/usr/local/lib/python3.7/site-packages/requests/adapters.py", line 499, in send timeout=timeout, File "/usr/local/lib/python3.7/site-packages/urllib3/connectionpool.py", line 788, in urlopen method, url, error=e, _pool=self, _stacktrace=sys.exc_info()[2] File "/usr/local/lib/python3.7/site-packages/urllib3/util/retry.py", line 592, in increment raise MaxRetryError(_pool, url, error or ResponseError(cause)) urllib3.exceptions.MaxRetryError: HTTPConnectionPool(host='localhost', port=3400): Max retries exceeded with url: /v1/statement (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7f4a91d33e10>: Failed to establish a new connection: [Errno 111] Connection refused')) During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/opt/airflow/airflow/sensors/base.py", line 200, in execute poke_return = self.poke(context) File "/opt/airflow/airflow/providers/common/sql/sensors/sql.py", line 92, in poke records = hook.get_records(self.sql, self.parameters) File "/opt/airflow/airflow/providers/presto/hooks/presto.py", line 152, in get_records return super().get_records(self.strip_sql_string(sql), parameters) File "/opt/airflow/airflow/providers/common/sql/hooks/sql.py", line 196, in get_records return self.run(sql=sql, parameters=parameters, handler=fetch_all_handler) File "/opt/airflow/airflow/providers/presto/hooks/presto.py", line 196, in run return_last=return_last, File "/opt/airflow/airflow/providers/common/sql/hooks/sql.py", line 266, in run self._run_command(cur, sql_statement, parameters) File "/opt/airflow/airflow/providers/common/sql/hooks/sql.py", line 291, in _run_command cur.execute(sql_statement) File "/usr/local/lib/python3.7/site-packages/prestodb/dbapi.py", line 236, in execute result = self._query.execute() File "/usr/local/lib/python3.7/site-packages/prestodb/client.py", line 542, in execute response = self._request.post(self._sql) File "/usr/local/lib/python3.7/site-packages/prestodb/client.py", line 356, in post proxies=PROXIES, File "/usr/local/lib/python3.7/site-packages/prestodb/exceptions.py", line 130, in decorated raise error File "/usr/local/lib/python3.7/site-packages/prestodb/exceptions.py", line 117, in decorated result = func(*args, **kwargs) File "/usr/local/lib/python3.7/site-packages/requests/sessions.py", line 635, in post return self.request("POST", url, data=data, json=json, **kwargs) File "/usr/local/lib/python3.7/site-packages/requests/sessions.py", line 587, in request resp = self.send(prep, **send_kwargs) File "/usr/local/lib/python3.7/site-packages/requests/sessions.py", line 701, in send r = adapter.send(request, **kwargs) File "/usr/local/lib/python3.7/site-packages/requests/adapters.py", line 565, in send raise ConnectionError(e, request=request) requests.exceptions.ConnectionError: HTTPConnectionPool(host='localhost', port=3400): Max retries exceeded with url: /v1/statement (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7f4a91d33e10>: Failed to establish a new connection: [Errno 111] Connection refused'))

Is it possible that this kind of tests are actually broken and just not run in CI?

It's very likely

) (cherry picked from commit 518fd84)

Replace unittests in providers tests by pure pytest [Wave-3]

d554922

Taragolis requested a review from eladkal as a code owner November 15, 2022 17:51

boring-cyborg bot added area:logging area:providers provider:common-sql provider:databricks labels Nov 15, 2022

Taragolis added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Nov 15, 2022

Taragolis added 2 commits November 15, 2022 22:52

Fix SFTPHook tests

054f104

Fix MySqlHook Tests

88ae05f

uranusjr approved these changes Nov 16, 2022

View reviewed changes

Taragolis commented Nov 16, 2022

View reviewed changes

Taragolis added 3 commits November 16, 2022 21:11

Fix TestRedisPublishOperator

d294af6

get rid of TestHiveEnvironment

8020464

Remove import unittest

8b989a6

Taragolis commented Nov 17, 2022

View reviewed changes

Taragolis mentioned this pull request Nov 17, 2022

Replace unittests in providers-microsoft tests by pure pytest [Wave-4] #27735

Merged

change setup to setup_method

e56fa90

uranusjr approved these changes Nov 25, 2022

View reviewed changes

potiuk approved these changes Nov 26, 2022

View reviewed changes

potiuk merged commit 518fd84 into apache:main Nov 26, 2022

Taragolis deleted the providers-test-pytests-wave-1 branch November 26, 2022 11:58

ephraimbuddy pushed a commit that referenced this pull request Jan 13, 2023

Replace unittests in providers tests by pure pytest [Wave-3] (#27696

350b4a1

) (cherry picked from commit 518fd84)

ephraimbuddy pushed a commit that referenced this pull request Jan 14, 2023

Replace unittests in providers tests by pure pytest [Wave-3] (#27696

6c01117

) (cherry picked from commit 518fd84)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Replace `unittests` in providers tests by pure `pytest` [Wave-3] #27696

Replace `unittests` in providers tests by pure `pytest` [Wave-3] #27696

Uh oh!

Taragolis commented Nov 15, 2022 •

edited

Loading

Uh oh!

uranusjr left a comment

Uh oh!

Taragolis left a comment

Uh oh!

Taragolis Nov 16, 2022

Uh oh!

Taragolis Nov 16, 2022

Uh oh!

Taragolis Nov 16, 2022

Uh oh!

Taragolis Nov 17, 2022

Uh oh!

potiuk Nov 17, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Replace unittests in providers tests by pure pytest [Wave-3] #27696

Replace unittests in providers tests by pure pytest [Wave-3] #27696

Uh oh!

Conversation

Taragolis commented Nov 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

uranusjr left a comment

Choose a reason for hiding this comment

Uh oh!

Taragolis left a comment

Choose a reason for hiding this comment

Uh oh!

Taragolis Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

Taragolis Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

Taragolis Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

Taragolis Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

potiuk Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Replace `unittests` in providers tests by pure `pytest` [Wave-3] #27696

Replace `unittests` in providers tests by pure `pytest` [Wave-3] #27696

Taragolis commented Nov 15, 2022 •

edited

Loading