Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move new provider tests to "provider_tests" submodule #45955

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jan 23, 2025

While testing "standard" provider move to the new structure it turned out, that "providers" package used now to keep tests in the new providers is problematic - while it solves the ambiguity of "from celery import", it introduces another ambiguity when trying to import internal utility classes in tests. For example when trying to import BasePythonTest in standard operatori currently we refer to the import from root directory of Airflow::

from providers.tests.standard.operators.test_python

However, if we change the package structure to have providers as subpackage of tests and trying to import it from tests as root folder to tests, we have:

from providers.standard.operators.test_python

And it can ambiguously attempt to import

from airflow.providers.standard.operators.test_python

This confuses IDEs and attempts to run multiple tests together, because pytest modifies PYTHONPATH while running tests - and PYTHONPATH modification can be different, depending on which tests are selected to run.

Changing the sub-package to provider_tests makes it unambiguous and we can now import:

from provider_tests.standard.operators.test_python

This is unambiguous and allows to import the tests inside the provider package, without importing from root of Airlfow package - making provider package tests "standalone" inside provider.


^ 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.

While testing "standard" provider move to the new structure
it turned out, that "providers" package used now to keep tests
in the new providers is problematic - while it solves the
ambiguity of "from celery import", it introduces another
ambiguity when trying to import internal utility classes in tests.
For example when trying to import BasePythonTest in standard
operatori currently we refer to the import from root directory
of Airflow::

```
from providers.tests.standard.operators.test_python
```

However, if we change the package structure to have `providers`
as subpackage of `tests` and trying to import it from `tests`
as root folder to tests, we have:

```
from providers.standard.operators.test_python
```

And it can ambiguously attempt to import

```
from airflow.providers.standard.operators.test_python
```

This confuses IDEs and attempts to run multiple tests
together, because pytest modifies PYTHONPATH while running
tests - and PYTHONPATH modification can be different, depending
on which tests are selected to run.

Changing the sub-package to provider_tests makes it unambiguous
and we can now import:

```
from provider_tests.standard.operators.test_python
```

This is unambiguous and allows to import the tests inside
the provider package, without importing from root of Airlfow
package - making provider package tests "standalone" inside
provider.
@potiuk potiuk merged commit 4dda6ba into apache:main Jan 23, 2025
64 checks passed
@potiuk potiuk deleted the move-tests-in-providers-to-provider-tests branch January 23, 2025 07:13
dauinh pushed a commit to dauinh/airflow that referenced this pull request Jan 24, 2025
While testing "standard" provider move to the new structure
it turned out, that "providers" package used now to keep tests
in the new providers is problematic - while it solves the
ambiguity of "from celery import", it introduces another
ambiguity when trying to import internal utility classes in tests.
For example when trying to import BasePythonTest in standard
operatori currently we refer to the import from root directory
of Airflow::

```
from providers.tests.standard.operators.test_python
```

However, if we change the package structure to have `providers`
as subpackage of `tests` and trying to import it from `tests`
as root folder to tests, we have:

```
from providers.standard.operators.test_python
```

And it can ambiguously attempt to import

```
from airflow.providers.standard.operators.test_python
```

This confuses IDEs and attempts to run multiple tests
together, because pytest modifies PYTHONPATH while running
tests - and PYTHONPATH modification can be different, depending
on which tests are selected to run.

Changing the sub-package to provider_tests makes it unambiguous
and we can now import:

```
from provider_tests.standard.operators.test_python
```

This is unambiguous and allows to import the tests inside
the provider package, without importing from root of Airlfow
package - making provider package tests "standalone" inside
provider.
@utkarsharma2 utkarsharma2 added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jan 27, 2025
gpathak128 pushed a commit to gpathak128/airflow that referenced this pull request Jan 29, 2025
While testing "standard" provider move to the new structure
it turned out, that "providers" package used now to keep tests
in the new providers is problematic - while it solves the
ambiguity of "from celery import", it introduces another
ambiguity when trying to import internal utility classes in tests.
For example when trying to import BasePythonTest in standard
operatori currently we refer to the import from root directory
of Airflow::

```
from providers.tests.standard.operators.test_python
```

However, if we change the package structure to have `providers`
as subpackage of `tests` and trying to import it from `tests`
as root folder to tests, we have:

```
from providers.standard.operators.test_python
```

And it can ambiguously attempt to import

```
from airflow.providers.standard.operators.test_python
```

This confuses IDEs and attempts to run multiple tests
together, because pytest modifies PYTHONPATH while running
tests - and PYTHONPATH modification can be different, depending
on which tests are selected to run.

Changing the sub-package to provider_tests makes it unambiguous
and we can now import:

```
from provider_tests.standard.operators.test_python
```

This is unambiguous and allows to import the tests inside
the provider package, without importing from root of Airlfow
package - making provider package tests "standalone" inside
provider.
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
While testing "standard" provider move to the new structure
it turned out, that "providers" package used now to keep tests
in the new providers is problematic - while it solves the
ambiguity of "from celery import", it introduces another
ambiguity when trying to import internal utility classes in tests.
For example when trying to import BasePythonTest in standard
operatori currently we refer to the import from root directory
of Airflow::

```
from providers.tests.standard.operators.test_python
```

However, if we change the package structure to have `providers`
as subpackage of `tests` and trying to import it from `tests`
as root folder to tests, we have:

```
from providers.standard.operators.test_python
```

And it can ambiguously attempt to import

```
from airflow.providers.standard.operators.test_python
```

This confuses IDEs and attempts to run multiple tests
together, because pytest modifies PYTHONPATH while running
tests - and PYTHONPATH modification can be different, depending
on which tests are selected to run.

Changing the sub-package to provider_tests makes it unambiguous
and we can now import:

```
from provider_tests.standard.operators.test_python
```

This is unambiguous and allows to import the tests inside
the provider package, without importing from root of Airlfow
package - making provider package tests "standalone" inside
provider.
niklasr22 pushed a commit to niklasr22/airflow that referenced this pull request Feb 8, 2025
While testing "standard" provider move to the new structure
it turned out, that "providers" package used now to keep tests
in the new providers is problematic - while it solves the
ambiguity of "from celery import", it introduces another
ambiguity when trying to import internal utility classes in tests.
For example when trying to import BasePythonTest in standard
operatori currently we refer to the import from root directory
of Airflow::

```
from providers.tests.standard.operators.test_python
```

However, if we change the package structure to have `providers`
as subpackage of `tests` and trying to import it from `tests`
as root folder to tests, we have:

```
from providers.standard.operators.test_python
```

And it can ambiguously attempt to import

```
from airflow.providers.standard.operators.test_python
```

This confuses IDEs and attempts to run multiple tests
together, because pytest modifies PYTHONPATH while running
tests - and PYTHONPATH modification can be different, depending
on which tests are selected to run.

Changing the sub-package to provider_tests makes it unambiguous
and we can now import:

```
from provider_tests.standard.operators.test_python
```

This is unambiguous and allows to import the tests inside
the provider package, without importing from root of Airlfow
package - making provider package tests "standalone" inside
provider.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) provider:airbyte provider:apache-iceberg provider:celery provider:edge Edge Executor / Worker (AIP-69)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants