o-nikolas commented on code in PR #46608:
URL: https://github.com/apache/airflow/pull/46608#discussion_r1956849973
##########
dev/breeze/tests/test_pytest_args_for_test_types.py:
##########
@@ -25,15 +25,18 @@
# TODO(potiuk): rename to all_providers when we move all providers to the new
structure
def _all_new_providers() -> list[str]:
Review Comment:
We can do this rename now, ya?
##########
providers/amazon/tests/system/amazon/aws/example_appflow.py:
##########
@@ -30,6 +28,7 @@
AppflowRunFullOperator,
)
from airflow.providers.standard.operators.bash import BashOperator
+from system.amazon.aws.utils import SystemTestContextBuilder
Review Comment:
Much more succinct, very nice :+1:
##########
docs/conf.py:
##########
@@ -226,7 +226,11 @@
"sphinx_jinja",
]
)
- exclude_patterns = ["operators/_partials"]
+ empty_subpackages = ["apache", "atlassian", "common", "cncf", "dbt",
"microsoft"]
Review Comment:
Does Amazon need this treatment given the `aws` subdirectory it has?
##########
scripts/ci/pre_commit/check_providers_subpackages_all_have_init.py:
##########
@@ -30,40 +32,119 @@
"static",
]
+PATH_EXTENSION_STRING = '__path__ =
__import__("pkgutil").extend_path(__path__, __name__) # type: ignore'
+
+# Here we should add the second level paths that we want to have sub-packages
in
+KNOWN_SECOND_LEVEL_PATHS = ["apache", "atlassian", "common", "cncf", "dbt",
"microsoft"]
Review Comment:
This list is in a lot of places. If we ever have to add to it, it will be
tricky to find them all :grimacing:
##########
pyproject.toml:
##########
@@ -411,6 +414,10 @@ testing = ["dev", "providers.tests", "task_sdk.tests",
"tests_common", "tests"]
"tests_common/test_utils/compat.py" = ["TID251", "F401"]
"tests_common/pytest_plugin.py" = ["F811"]
+# While pandas imoport is banned, sql.pyi should be exclueded from it as it
does not have TYPE_CHECKING
+# mechanism and whole .pyi is really "type-checking" only
Review Comment:
Nit
```suggestion
# While pandas import is banned, sql.pyi should be excluded from it as it
does not have a TYPE_CHECKING
# mechanism and whole .pyi is really "type-checking" only
```
##########
dev/breeze/src/airflow_breeze/utils/run_tests.py:
##########
@@ -293,41 +291,26 @@ def convert_test_type_to_pytest_args(
f"[info]Removing {provider_test_to_exclude} from
{providers_with_exclusions}[/]"
)
providers_with_exclusions.remove(provider_test_to_exclude)
- else:
- # TODO(potiuk): remove me when all providers are migrated
- get_console().print(f"[info]Adding
{provider_test_to_exclude} to pytest ignores[/]")
- providers_with_exclusions.append(
- "--ignore=providers/tests/" +
excluded_provider.replace(".", "/")
- )
return providers_with_exclusions
if test_type.startswith(PROVIDERS_LIST_PREFIX):
provider_list = test_type[len(PROVIDERS_LIST_PREFIX) :
-1].split(",")
providers_to_test = []
for provider in provider_list:
- # TODO(potiuk) - remove when all providers are new-style
- provider_path =
OLD_TESTS_PROVIDERS_ROOT.joinpath(provider.replace(".", "/")).relative_to(
- AIRFLOW_SOURCES_ROOT
+ provider_path = (
+ AIRFLOW_PROVIDERS_DIR.joinpath(provider.replace(".",
"/")).relative_to(
+ AIRFLOW_SOURCES_ROOT
+ )
+ / "tests"
)
if provider_path.is_dir():
providers_to_test.append(provider_path.as_posix())
else:
- # TODO(potiuk) - remove when all providers are new-style
- old_provider_path = provider_path
- provider_path = (
- AIRFLOW_PROVIDERS_DIR.joinpath(provider.replace(".",
"/")).relative_to(
- AIRFLOW_SOURCES_ROOT
- )
- / "tests"
+ get_console().print(
+ f"[error]Neither {provider_path} exist for {provider} "
+ "- which means that provider has no tests. This is bad
idea. "
+ "Please add it (all providers should have a package in
tests)"
Review Comment:
We are only looking under one path now, so we can update the language of
this message:
```suggestion
f"[error] {provider_path} does not exist for
{provider} "
"- which means that this provider has no tests. This
is bad idea. "
"Please add it (all providers should have at least a
package in tests)."
```
--
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]
