potiuk commented on PR #45259:
URL: https://github.com/apache/airflow/pull/45259#issuecomment-2572427916
Indeed - worth to describe the proposal how tests should work - this is one
of the changes that is not yet "final" setup but more of a step in the right
direction.
It's not obvious and worth mentioning what's the setup I propose now (and
how it can slightly evolve).
I propose that we move all the provider tests in the "tests" folder or
"tests/system" folder as `providers.<PROVIDER>`. Main reason is so that we can
mark "tests" as the root for all tests similarly as "src" is root for all
sources.
This will allow to avoid problems like "from email import something" failing
which was often the problem that we had when we wanted to treat "tests" as
PYTHONPATh entry - because it was clashing with built-in email package (for
example - there were more packages like that).
So Inside provider folders we (I use google as an example with sytem tests)
will have this:
```
tests
|- conftest.py -> here we import common plugins for both "tests" and
system tests"
tests/providers/google/
|- -> all google tests are here
tests/system/providers/google
|- conftest.py -> here we have common stuff for system tests of
google provider
```
This should help with making nice separation between regular and "system"
tests - but also allow to have unambiguous imports:
```python
from email import ....
from providers.email import test_whatever
```
But at the same time, even if someone will have "tests" added to PYTHONPATH
(some IDEs will do it automatically) we will avoid this to be interpreted as
"from tests.email import" (which happened to a number of people in the past -
and it's pretty confusing).
```python
from email
```
We will still have to figure out what to do with "tests_common" - currently
we use plugins in the common conftest.py:
```python
pytest_plugins = "tests_common.pytest_plugin"
```
This allows to run provider tests from the root of breeze, and where we also
install airflow: `pip install -e.[]` (or by default when you run `uv sync`
and have `.venv` environment created. This **just works**:
```bash
pytest providers/airbyte/tests/providers/airbyte/test_something.py
```
But after we complete the provider split we have the possibility of moving
"airflow" to subfolder of the main repo - we have not decided yet, but that
would make sense - and in this case we will be able to refactor slightly the
way how tests_common is imported. One problem with the proposal now is that you
cannot run easily tests when you create venv directly in the "provider" folder.
This does **NOT** work yet (you really need to run provider tests in context
of Airflow venv):
```
cd providers/airbyte
uv sync
uv run pytest tests/providers/airfbyte/test_something.py
```
But when we move airflow out and make "test_common" a "referrable"
uv-workspae project, we should be able to make it possible to make it "common"
project that will be imported by all projects: airflow, task_sdk, each of the
provider etc. I did not want to make those changes now - that would require a
number of import changes in tests , so I want to do it as a separte refactor
after all providers have been moved.
This will also make each "provider" project "truly" standalone - i.e. you
will be able to easily run tests there without even creating a venv in "."
top-level project - and essentially work on provider project separately.
At the same time (thanks to workspaces) we will allow to have all - task_sdk
+ providers + test_common installed together and run and test them togetther as
well. This is the promise of workspaces - that you can work separately on each
project but you can also install everything together and implemnt - at the same
time - changes in all the projects (which was so far the only way we could
develop airflow).
--
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]