From 756ffb9179941c9b60a2b6a95c06ba943f043f8e Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Thu, 31 Oct 2024 00:29:22 +0100 Subject: [PATCH] Fix testing tests command to allow passing test as extra arg When providers have been moved in #42505 broke passing parameters when provider tests were passed as extra args of "testing tests" command of breeze. Previously all tests were under "tests" folder and there was an exclusion to disable "All" tests when any test was passed as parameter. But after moving tests to "providers" this stopped working. Additional exclusion needs to be added for "providers/tests" and "providers/tests_sdk/". This PR also adds autocompletion for tests passed this way by setting the click type to Path for the extra args (but without the need for the Path to exist). Also during this check it turned out that "All" tests are not working in the intended way - but this should not impact our CI only local runs. Appropriate comment has been added and it's captured in https://github.com/apache/airflow/issues/42632 --- dev/breeze/doc/images/output_testing_tests.txt | 2 +- .../src/airflow_breeze/commands/testing_commands.py | 12 ++++++++++-- dev/breeze/src/airflow_breeze/utils/run_tests.py | 6 ++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/dev/breeze/doc/images/output_testing_tests.txt b/dev/breeze/doc/images/output_testing_tests.txt index bc12c1fb42a0f..67808ed7f40ad 100644 --- a/dev/breeze/doc/images/output_testing_tests.txt +++ b/dev/breeze/doc/images/output_testing_tests.txt @@ -1 +1 @@ -4e128855ff2df624e0fc59e229b0973d +69c5e660ec3f263dad58cc7a7710d4ed diff --git a/dev/breeze/src/airflow_breeze/commands/testing_commands.py b/dev/breeze/src/airflow_breeze/commands/testing_commands.py index 3c3015cd7fe12..2d76b66f37421 100644 --- a/dev/breeze/src/airflow_breeze/commands/testing_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/testing_commands.py @@ -553,7 +553,7 @@ def _verify_parallelism_parameters( @option_use_packages_from_dist @option_use_xdist @option_verbose -@click.argument("extra_pytest_args", nargs=-1, type=click.UNPROCESSED) +@click.argument("extra_pytest_args", nargs=-1, type=click.Path(path_type=str)) def command_for_tests(**kwargs): _run_test_command(**kwargs) @@ -877,8 +877,16 @@ def _run_test_command( ) else: if shell_params.test_type == "Default": - if any([arg.startswith("tests") for arg in extra_pytest_args]): + if any( + [ + arg.startswith("tests/") + or arg.startswith("providers/tests/") + or arg.startswith("task_sdk/tests/") + for arg in extra_pytest_args + ] + ): # in case some tests are specified as parameters, do not pass "tests" as default + # test_type = "All" as default and it will run all "tests" by default then shell_params.test_type = "None" shell_params.parallel_test_types_list = [] else: diff --git a/dev/breeze/src/airflow_breeze/utils/run_tests.py b/dev/breeze/src/airflow_breeze/utils/run_tests.py index 99763c1196bf1..81cd0671dd8fc 100644 --- a/dev/breeze/src/airflow_breeze/utils/run_tests.py +++ b/dev/breeze/src/airflow_breeze/utils/run_tests.py @@ -212,6 +212,12 @@ def find_all_other_tests() -> list[str]: PROVIDERS_LIST_EXCLUDE_PREFIX = "Providers[-" ALL_TEST_SUITES: dict[str, tuple[str, ...]] = { + # TODO: This is not really correct now - we should allow to run both providers and airflow + # as "ALL" tests - currently it is not possible due to conftest.py present at top-level of + # all different test suites ("tests", "providers/tests", "task_sdk/tests") + # The only reason it is working now in CI is because we run tests in parallel (both DB and non-DB) + # each test subfolder is separately specified in the pytest command line + # This should be solved as part of https://github.com/apache/airflow/issues/42632 "All": ("tests",), "All-Long": ("tests", "-m", "long_running", "--include-long-running"), "All-Quarantined": ("tests", "-m", "quarantined", "--include-quarantined"),