Upgrade pytest to v8

[hussein-awala]

trafficstars

[hussein-awala]

depends on: https://github.com/pytest-dev/pytest-asyncio/pull/765

[hussein-awala]

It will be fixed in the next release https://github.com/pytest-dev/pytest-asyncio/pull/776

[potiuk]

Hey @hussein-awala -> the big problem is pytest-httpx. The latest version we can install in Python 3.8 is 0.23.1 (0.24 and above is Python >= 3.9. The 0.23.1 has Pytest < 8 so pip simply cannot find solution - there is no matching pytest-httpx that we could install in Python 3.8 image. There are likely other problems once we solve this one, but this is a basic prerequisite.

[potiuk]

There is a exactly 1(!) test that uses httpx_mock - so I think we should remove pytest-httpx and implement the test differently

test_info_command.py:

    def test_show_info_anonymize_fileio(self, httpx_mock, setup_parser):

[potiuk]

PR removing pytest-httpx here: https://github.com/apache/airflow/pull/37334 - once we merge it , we can try again.

[potiuk]

For the future @hussein-awala -

One easy way to test if a new dependency is not conflicting is quite simple:

1. Update pyproject toml
2. Enter Breeze
3. Run pip install -e ".[all]" YOUR_DEPENDENCY==VERSION - where YOUR_DEPENDENCY==VERSION is the version you think should be installed by breeze. In this case:

pip install -e ".[all]" pytest==8.0.0

This should find if there is any conflict, much quicker (because you specifically choose dependency version and pip will not try to backtrack and find other combinations, it will simply tell you what's wrong.

In this case the message was:

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
pytest-httpx 0.21.3 requires pytest<8.0,>=6.0, but you have pytest 8.0.0 which is incompatible.

This is how I found out pytest-httpx is problematic.

[hussein-awala]

Thanks Jarek for this detailed explanation and for the tip to test the future changes.

[hussein-awala]

I'm still trying to understand why tests/utils/log/test_colored_log.py::test_format_time_uses_tz_aware fails in Breeze the first time I run it, but it works locally and in Breeze if I run it more than once in the same terminal 🤔

Changing the logging level and moving the logger to a fixture did not resolve the issue. I need to thoroughly check the pytest changelog to understand what's going on.

[potiuk]

@hussein-awala : change patching: airflow.utils.log.timezone_aware.TimezoneAware.formatTime -> airflow.utils.log.colored_log.TimezoneAware.formatTime ?

[hussein-awala]

yes, I tried that too, but it didn't work.

[potiuk]

This one is strange :)

[Taragolis]

Try to add format which include asctime attribute into format into the formatter https://github.com/apache/airflow/blob/1aa91a482630f389005a9a36b0f038e675163411/tests/utils/log/test_colored_log.py#L35

Something like that

    h.setFormatter(CustomTTYColoredFormatter(fmt="%(asctime)s: %(log_color)s%(message)s%(reset)s"))

[hussein-awala]

Try to add format which include asctime attribute into format into the formatter

https://github.com/apache/airflow/blob/1aa91a482630f389005a9a36b0f038e675163411/tests/utils/log/test_colored_log.py#L35

Something like that

    h.setFormatter(CustomTTYColoredFormatter(fmt="%(asctime)s: %(log_color)s%(message)s%(reset)s"))

It seems to work with this, which is more strange to me 🤔 Let's wait for the CI

[potiuk]

It seems to work with this, which is more strange to me 🤔 Let's wait for the CI

Ah - interesting, that would mean that the default logging configuration is not initialized the first time you enter them (because airflow configuration is not writen to airflow.cfg yet.

[potiuk]

Or smth equally racy

[Taragolis]

One of the possible reason changes in collection order: https://github.com/pytest-dev/pytest/issues/7777

Other changes in pytest 8: https://docs.pytest.org/en/stable/changelog.html

[potiuk]

Nice and looks green :)

[potiuk]

TOO QUICK :)

[Taragolis]

Seems like, configs from providers do not loaded for this tests.

[Taragolis]

Worthwhile to try it again with 8.1.1, it might fix some issues.

[Taragolis]

Looks promising only couple of tests:

1. pytest.warns(expected_warning=None) I miss that one when did https://github.com/apache/airflow/pull/38027
2. FAB tests

[potiuk]

Getting closer :)

[Taragolis]

I could reproduce FAB tests failure in pytest 7 if I run this module in random order

root@d9cbab8b3875:/opt/airflow# pip install pytest-random-order

root@d9cbab8b3875:/opt/airflow# pytest tests/providers/fab/auth_manager/test_security.py --random-order
...
======================================================================= 3 failed, 56 passed, 20 warnings in 7.76s =======================================================================

root@d9cbab8b3875:/opt/airflow# pytest tests/providers/fab/auth_manager/test_security.py --random-order

======================================================================= 7 failed, 52 passed, 20 warnings in 7.91s =======================================================================

root@d9cbab8b3875:/opt/airflow# pytest tests/providers/fab/auth_manager/test_security.py
...
============================================================================ 59 passed, 20 warnings in 7.81s ============================================================================

100% there is some side effect which make tests work in this case, in pytest 8 the order slightly different so that is why the error happen into the CI

[hussein-awala]

https://github.com/apache/airflow/pull/37151/commits/789c99aa46d34307e6adf2dd291ebe8faa2c3062 could fix it while keeping the test logic correct

[hussein-awala]

I also fixed it by changing the fixtures scope from module to function, but it increased the test processing time. We need to add a teardown block somewhere to clean the db.

[Taragolis]

I also fixed it by changing the fixtures scope from module to function, but it increased the test processing time. We need to add a teardown block somewhere to clean the db.

I've tried to do the same things locally, but still have a side effects in random order

@pytest.fixture(autouse=True)
def clear_db_after_suite(app):
    clear_db_runs()
    clear_db_dags()
    delete_users(app)
    delete_roles(app)
    yield
    delete_users(app)
    delete_roles(app)
    clear_db_runs()
    clear_db_dags()

for example this two test in that order effect to each other

pytest "tests/providers/fab/auth_manager/test_security.py::TestHasAccessDagDecorator::test_dag_id_consistency[a-None-None-a-False]" tests/providers/fab/auth_manager/test_security.py::test_permissions_work_for_dags_with_dot_in_dagname

[Taragolis]

But yeah if fix help we just need to create a task for a properly fix this test module

[github-actions[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.