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]

Reply via email to