Remove requirement for test_ prefix for pytest test modules

[potiuk]

Currently we had a requirement to use test_ prefix for modules for all
our tests, however this means that we could have missed some of the
tests from pytest collection when they were placed in a module without
the test_ prefix. This happened already: see #30311 and #30306

The previous attempts to remove it failed, because cassandra tests
seemed to be incompatible with pytest collection when we allowed all
Python files, also there were a few names that caused the pytest
collection to fail with selecting all Python files for collection.

The move is accompanied by converting pytest configuration to
pyproject.toml.

After long investigation, it turned out that the cause of cassandra
failures was pytest assert rewrite with collided with Cython-related
type_codes.py definition of types.

See datastax/python-driver#1142 for PR
that is aimed to fix it on cassandra side, in the meantime we
are manually patching the type_codes.py file from Cassandra by adding
PYTEST_DONT_REWRITE to its docstring when building the CI image for
testing.

Another error was using run_module method which also suffers from
assert rewriting incompatibilities but this could be fixed by using
run_path instead. pytest-dev/pytest#9007

Yet another assert-rewrite problem was that test_airflow_context
also failed with --assert=rewrite turned on when python_files
were not filtered. No solution was found on how we can disable
the rewrite (apparently it happens for yaml.py file but adding
PYTEST_DONT_REWRITE to yaml.py and related files did not help,
so we had to extract the test to separate test type)

Also test in docker_tests/kubernetes_tests should be run with working
directory being in their folders in order to apply pyproject.toml from
their directores (without specifying asyncio mode).

The following changes are applied:

• conversion of pytest configuration from pytest.ini to
  pyproject.toml (for main project, docker_tests and breeze)

• addedd pyproject.toml to breeze, docker_tests, kubernetes_tests
  to reflect they need different configuration for pytest (lack
  of pytest-asyncio for example)

• adding automated patching of Cassandra type_codes.py with
  PYTEST_DONT_REWRITE docstring marker

• add pyproject.toml to docker context in order to include it in
  sources of airflow in the image

• changed working directory for k8s suite of breeze commands to be
  kubernetes_tests

• remove pytest.ini from docker compose mounting and automated removal
  in entrypoin in case old image is used

• rename few breeze function to not start with "test_" so that they
  are not collected as test methods

• remove few test dags from collecton by pytest by name

• replace run_module with run_path usage in EKS test and test_www.

• CI workflow is updated to use docker_tests as working directory
  for those tests

• perf dags were moved out of the tests/test_utils dir to dev
  directory.

• PlainAsserts test type was added and the test_airflow_context
  test is run in this separate test type when running parallel
  tests by default.

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[potiuk]

Before: 15202 tests collected in 123.14s (0:02:03)

After: 15223 tests collected in 144.51s (0:02:24)

[potiuk]

cc: @ashb - I think you attempted to do it before, I think I found a solution.

[potiuk]

Seems we have a few errors to handle before we merge that one :)

[potiuk]

Greeting, Jarek. Just want to clarify why we want to change this behaviour for pytest tests? Personally for me this is a natural to have test_ prefix or _test suffix for test modules, and also this is default pytest naming convention

If some tests not discoverable by the reason that python modules doesn't have test_ prefix might be better just rename this modules? There is plenty other rules which might turn test into undiscoverable.

But it is possible that some other reason exists and I'm just out of the context.

Yes. As explained in the PR - we already hit this problem because we missed the fact that test_ was missing from test modules during review.

This happened already: see #30311 and #30306

Those tests were not running when you run them in CI - simply because pytest collection rules misses them entirely because of lack of test_*. But we have no idea that it happens. They are just silently ignored (by design). The problem is that if you run the tests locally during development, and run them by pytest <test_file>.py they will run (this is why it is more important than other rules that make tests not discoverable). So as someone who contributes the tests, you are completely not aware of that (and reviewer might easily miss it).

As result our CI tests do NOT test those tests for regression. And again - this already happened with databricks and smtp tests - so it will inevitably happen again unless we protect against this - this is not a hypothetical problem.

We could of course add some pre-commits (and @hussein-awala attempted to detect such silently skipped tests in #30306 (comment) , however that was very simplistic and not fool-proof. I think the only way to avoid any such mishaps in the future is to add all files in "tests" dir to be pytest-collectable.

[potiuk]

(all right I also fixed k8s tests to have their own pyproject.toml).

[potiuk]

And BTW - another issue raised to pytest pytest-dev/pytest#10845

[potiuk]

Also @Taragolis on top of the risks involved by accidentally skipping tests and risking regressions - one more point https://docs.pytest.org/en/7.2.x/example/pythoncollection.html#customizing-test-collection explain that you "might" want to collect all python files and go to a great length in explaining how to do it and how to avoid some pitfalls, so this is not entirely unusual to change it if pytest maintainers decided to explain it.

[Taragolis]

Not only module name could be the reason that tests are not collected. The also other naming conventions also could be a reason, and we also have it in the past when test class doesn't start with Test*

We have additional check that tests exists for appropriate airflow modules, however it broken for a very long time and some of the modules doesn't follow this naming convention.

I do not have any concern about scan all files if it safe, I really do not know how it affects to conftest.py and other modules which use only as a helpers but could have some side effects.

May be just better follow naming conventions (this sample only cover regular airflow parts but not breeze, k8s and charts test) and check it again:

1. airflow.awesome.module -> tests.awesome.test_module or tests.integration.awesome.test_module
2. airflow.providers.awesome.module -> tests.providers.awesome.test_module or tests.providers.integration.awesome.test_module

[potiuk]

Not only module name could be the reason that tests are not collected. The also other naming conventions also could be a reason, and we also have it in the past when test class doesn't start with Test*

Yep. But missing test_* in module name is far more difficult to catch by contributor.

If you don't have Test* name (unlike module name) you won't be able to run this test in your IDE or command line. That's the big difference because not following test_ module name might (and it already happened with smtp and datbricks) be not noticed by the contributors when adding their tests. Most people will run tests either via IDE or by runing pytest tests/your_test_path/your_test_module.py to test if they are working. This is also nicely supported by auto-completion in command line (and this auto-completion is based on shell autocompletion of file names so it does not know how to filter out files that do not follow test_ prefixs.. Auto-completing test class/test name with the cumbersome "::" syntax is not working. And even if somebody used it - it still does not run.

I just made an experiment. I brought back "test_*.py" in my PR and I run run this:

pytest tests/providers/smtp/hooks/smtp.py
/home/jarek/.pyenv/versions/3.7/envs/airflow-3.7/lib/python3.7/site-packages/pytest_asyncio/plugin.py:177: DeprecationWarning: You're using an outdated version of pytest. Newer releases of pytest-asyncio will not be compatible with this pytest version. Please update pytest to version 7 or later.
  DeprecationWarning,
========================================================= test session starts =========================================================
platform linux -- Python 3.7.16, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /home/jarek/.pyenv/versions/3.7/envs/airflow-3.7/bin/python3
cachedir: .pytest_cache
rootdir: /home/jarek/code/airflow, configfile: pyproject.toml
plugins: rerunfailures-9.1.1, requests-mock-1.10.0, timeouts-1.2.1, xdist-3.1.0, instafail-0.4.2, cov-4.0.0, anyio-3.6.2, httpx-0.21.2, asyncio-0.20.3, time-machine-2.9.0
asyncio: mode=strict
setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
collected 19 items                                                                                                                    

tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_connect_and_disconnect PASSED                                            [  5%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_connect_and_disconnect_via_nonssl PASSED                                 [ 10%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_get_email_address_single_email PASSED                                    [ 15%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_get_email_address_comma_sep_string PASSED                                [ 21%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_get_email_address_colon_sep_string PASSED                                [ 26%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_get_email_address_list PASSED                                            [ 31%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_get_email_address_tuple PASSED                                           [ 36%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_get_email_address_invalid_type PASSED                                    [ 42%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_get_email_address_invalid_type_in_iterable PASSED                        [ 47%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_build_mime_message PASSED                                                [ 52%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_send_smtp PASSED                                                         [ 57%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_hook_conn PASSED                                                         [ 63%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_send_mime_ssl PASSED                                                     [ 68%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_send_mime_nossl PASSED                                                   [ 73%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_send_mime_noauth PASSED                                                  [ 78%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_send_mime_dryrun PASSED                                                  [ 84%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_send_mime_ssl_complete_failure PASSED                                    [ 89%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_send_mime_partial_failure PASSED                                         [ 94%]
tests/providers/smtp/hooks/smtp.py::TestSmtpHook::test_send_mime_custom_timeout_retrylimit PASSED                               [100%]

========================================================= 19 passed in 0.42s ==========================================================

Also the tests are nicely showing as Runnable tests in the IDE (regardless of pyproject.toml setting):

Now, I renamed TestSmtpHook to SmtpHook and I got this:

pytest tests/providers/smtp/hooks/smtp.py
/home/jarek/.pyenv/versions/3.7/envs/airflow-3.7/lib/python3.7/site-packages/pytest_asyncio/plugin.py:177: DeprecationWarning: You're using an outdated version of pytest. Newer releases of pytest-asyncio will not be compatible with this pytest version. Please update pytest to version 7 or later.
  DeprecationWarning,
========================================================= test session starts =========================================================
platform linux -- Python 3.7.16, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /home/jarek/.pyenv/versions/3.7/envs/airflow-3.7/bin/python3
cachedir: .pytest_cache
rootdir: /home/jarek/code/airflow, configfile: pyproject.toml
plugins: rerunfailures-9.1.1, requests-mock-1.10.0, timeouts-1.2.1, xdist-3.1.0, instafail-0.4.2, cov-4.0.0, anyio-3.6.2, httpx-0.21.2, asyncio-0.20.3, time-machine-2.9.0
asyncio: mode=strict
setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
collected 0 items                                                                                                                     

======================================================== no tests ran in 0.02s ========================================================

I even used the colon syntax when I renamed the class:

ytest tests/providers/smtp/hooks/smtp.py::SmtpHook
/home/jarek/.pyenv/versions/3.7/envs/airflow-3.7/lib/python3.7/site-packages/pytest_asyncio/plugin.py:177: DeprecationWarning: You're using an outdated version of pytest. Newer releases of pytest-asyncio will not be compatible with this pytest version. Please update pytest to version 7 or later.
  DeprecationWarning,
==================================================================================================================================================================================== test session starts =====================================================================================================================================================================================
platform linux -- Python 3.7.16, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /home/jarek/.pyenv/versions/3.7/envs/airflow-3.7/bin/python3
cachedir: .pytest_cache
rootdir: /home/jarek/code/airflow, configfile: pyproject.toml
plugins: rerunfailures-9.1.1, requests-mock-1.10.0, timeouts-1.2.1, xdist-3.1.0, instafail-0.4.2, cov-4.0.0, anyio-3.6.2, httpx-0.21.2, asyncio-0.20.3, time-machine-2.9.0
asyncio: mode=strict
setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
collected 0 items                                                                                                                                                                                                                                                                                                                                                                            

=================================================================================================================================================================================== no tests ran in 0.01s ====================================================================================================================================================================================

And neither test class nor test methods are runnable via IDE any more:

This is PRECISELY what happened when smtp tests were added (and maybe @hussein-awala can confirm it).

• smtp.py module is nicely runnable via IDE or command line and this is what made the author thought it is perfectly fine
• yet it is not discovered during CI run
• and there is absolutley no warning aboout it

So this is what I want to avoid - I do not want to enforce standards (that might be follow up) - I want to make sure tha human mistakes are not causing regressions.

May be just better follow naming conventions (this sample only cover regular airflow parts but not breeze, k8s and charts test) and check it again

Sure. I absolutely don't argue with that. Even more - i will absolutely support it. Feel absolutely free to follow that. It will require quite a number of catch-up with the modules that do not follow it. a lot of manual fixes and renames. But it's an approach we might take in the future. I will be certainly supportive and happilly take part in it :). It will likely take weeks or months to complete though and likely quite some communication and multiple people to contribute.

And I think this is a nice follow-up to he the change here and this change does not invalidate this one. It's done, green, functional. Standardized using pyproject.toml which is the new standard and fixes some of the problems along the waye. It solves the problem at-hand that it will avoid some of the problems that happened even recently (the smtp thing happened literally 2 weeks ago.

I believe merging (or not) this change does not change anything with the idea of adding conventions? We can still add it even if we merge this one I guess? Or do you think this change is problematic if we would like to follow with implementing the convention you mentioned?

[potiuk]

BTW. It's all green - all the problems are nicely solved and it's ready to be reviewed and merged.

[Taragolis]

I still think this is hacky and use some workaround things. This wouldn't be required if we had approved PR but unfortunetly this how the things happen, some one PRs more important that others.

I'm not happy with this changes because instead of fix the reason why it initially happen we tried to add new workarounds and hacks (and we for month that test that check doesn't work), and this hacks and workaround could potentially break something in the future.

But this is only my opinion, I just want to said it, rather than change some one mind. If someone approve, well why not, CI is green.

[potiuk]

I still think this is hacky and use some workaround things. This wouldn't be required if we had approved PR but unfortunetly this how the things happen, some one PRs more important that others.

Which PR? Why was it not approved? I think it's quite a jump you made from not approved PR to things being more important than others, implying a bit bad intentions in general and hinting into some accusations. I do not like the tone of this. If there is a problem, please raise it (likely privately if there is a personal problem you are dealing with and do not want to share it in public).
I am not even sure if you are trying to point it at me or someone else. I'd advise to reach out possibly to [email protected] if you think you are being treated unfairly - this is one of the reasons why we have private@ list.

I'm not happy with this changes because instead of fix the reason why it initially happen we tried to add new workarounds and hacks (and we for month that test that check doesn't work), and this hacks and workaround could potentially break something in the future.

I do not think they will - we often do workaround and hacks temporarily and this one is followed up by two issues to pytest:

• Pytest assert rewrite causes serialzation problem with dill pytest-dev/pytest#10845
• Pytest assert rewrite fails when rewriting some parts of cassandra cython code pytest-dev/pytest#10844

They might lead to removing the need for the workarounds. This is the usual practice we do here very ofen. I do not see why this would be different this time. And if someone makes a change to follow the conventions - we can easily remove the workarounds - they are environmental and do not change essentialy our tests we run. I see no reason why we should do it differently than what we do usually.

[potiuk]

I'd l;ove to merge this one in - as explained - it will prevent accidentally missed tests.

[o-nikolas]

I don't love the workarounds either, but this feels like a situation where we can't let perfection be the enemy of good enough, I think missing tests is much more critical of a problem so it's worth the change.
Though, having some mechanism (maybe just TODO??) to remember to revisit these workarounds in the future would be nice to have so they don't get forgotten.

[hussein-awala]

Although my preference is to detect these not collected tests in pre-commit (or in the CI), but the idea of collecting everything except provided files in collect_ignore list is very interesting and can ensure 100% test detection .
Great work! LGTM