Raise SystemExit if breeze test specifies a directory that is also ignored

dev/breeze/src/airflow_breeze/commands/testing_commands.py

@@ -152,24 +152,8 @@ def docker_compose_tests(
 PERCENT_TEST_PROGRESS_REGEXP = r"^tests/.*\[[ \d%]*\].*|^\..*\[[ \d%]*\].*"
 
 
-def _run_test(
-    shell_params: ShellParams,
-    extra_pytest_args: tuple,
-    python_version: str,
-    output: Output | None,
-    test_timeout: int,
-    output_outside_the_group: bool = False,
-    skip_docker_compose_down: bool = False,
-) -> tuple[int, str]:
-    if "[" in shell_params.test_type and not shell_params.test_type.startswith("Providers"):
-        get_console(output=output).print(
-            "[error]Only 'Providers' test type can specify actual tests with \\[\\][/]"
-        )
-        sys.exit(1)
-    project_name = file_name_from_test_type(shell_params.test_type)
-    compose_project_name = f"airflow-test-{project_name}"
-    env = shell_params.env_variables_for_docker_commands
-    down_cmd = [
+def docker_down_command(compose_project_name):
+    return [
         "docker",
         "compose",
         "--project-name",
@@ -178,8 +162,10 @@ def _run_test(
         "--remove-orphans",
         "--volumes",
     ]
-    run_command(down_cmd, output=output, check=False, env=env)
-    run_cmd = [
+
+
+def docker_run_command(compose_project_name):
+    return [
         "docker",
         "compose",
         "--project-name",
@@ -190,33 +176,77 @@ def _run_test(
         "--rm",
         "airflow",
     ]
-    run_cmd.extend(
-        generate_args_for_pytest(
-            test_type=shell_params.test_type,
-            test_timeout=test_timeout,
-            skip_provider_tests=shell_params.skip_provider_tests,
-            skip_db_tests=shell_params.skip_db_tests,
-            run_db_tests_only=shell_params.run_db_tests_only,
-            backend=shell_params.backend,
-            use_xdist=shell_params.use_xdist,
-            enable_coverage=shell_params.enable_coverage,
-            collect_only=shell_params.collect_only,
-            parallelism=shell_params.parallelism,
-            python_version=python_version,
-            parallel_test_types_list=shell_params.parallel_test_types_list,
-            helm_test_package=None,
-            keep_env_variables=shell_params.keep_env_variables,
-            no_db_cleanup=shell_params.no_db_cleanup,
-        )
+
+
+def pytest_command(shell_params, python_version, test_timeout):
+    return generate_args_for_pytest(
+        test_type=shell_params.test_type,
+        test_timeout=test_timeout,
+        skip_provider_tests=shell_params.skip_provider_tests,
+        skip_db_tests=shell_params.skip_db_tests,
+        run_db_tests_only=shell_params.run_db_tests_only,
+        backend=shell_params.backend,
+        use_xdist=shell_params.use_xdist,
+        enable_coverage=shell_params.enable_coverage,
+        collect_only=shell_params.collect_only,
+        parallelism=shell_params.parallelism,
+        python_version=python_version,
+        parallel_test_types_list=shell_params.parallel_test_types_list,
+        helm_test_package=None,
+        keep_env_variables=shell_params.keep_env_variables,
+        no_db_cleanup=shell_params.no_db_cleanup,
     )
-    run_cmd.extend(list(extra_pytest_args))
-    # Skip "FOLDER" in case "--ignore=FOLDER" is passed as an argument
+
+
+def find_specified_and_ignored_pytest_directories(run_cmd) -> list[str]:
+    # Find any "FOLDER" argument where "--ignore=FOLDER" is also passed as an argument
     # Which might be the case if we are ignoring some providers during compatibility checks
-    run_cmd = [arg for arg in run_cmd if f"--ignore={arg}" not in run_cmd]
+    return [arg for arg in run_cmd if f"--ignore={arg}" in run_cmd]
+
+
+def _run_test(
+    shell_params: ShellParams,
+    extra_pytest_args: tuple,
+    python_version: str,
+    output: Output | None,
+    test_timeout: int,
+    output_outside_the_group: bool = False,
+    skip_docker_compose_down: bool = False,
+) -> tuple[int, str]:
+    if "[" in shell_params.test_type and not shell_params.test_type.startswith("Providers"):
+        get_console(output=output).print(
+            "[error]Only 'Providers' test type can specify actual tests with \\[\\][/]"
+        )
+        sys.exit(1)
+
+    project_name = file_name_from_test_type(shell_params.test_type)
+    compose_project_name = f"airflow-test-{project_name}"
+    env = shell_params.env_variables_for_docker_commands
+
+    # build the docker down command args
+    down_args = docker_down_command(compose_project_name)
+
+    # build the docker run command args
+    run_docker_args = docker_run_command(compose_project_name)
+
+    pytest_args = []
+    pytest_args.extend(pytest_command(shell_params, python_version, test_timeout))
+    pytest_args.extend(list(extra_pytest_args))
+
+    specified_and_ignored_pytest_args = find_specified_and_ignored_pytest_directories(pytest_args)
+    if specified_and_ignored_pytest_args:
+        get_console(output=output).print(
+            f"[error]Aborting test as pytest args contained directories that were also specified to be ignored: {specified_and_ignored_pytest_args}\n"
+        )
+        return 0, f"Test: {shell_params.test_type} blocked from running due to ambiguous arguments"
+
+    run_args = [*run_docker_args, *pytest_args]
+
+    run_command(down_args, output=output, check=False, env=env)
     try:
         remove_docker_networks(networks=[f"{compose_project_name}_default"])
         result = run_command(
-            run_cmd,
+            run_args,
             output=output,
             check=False,
             output_outside_the_group=output_outside_the_group,
@@ -788,7 +818,7 @@ def _run_test_command(
     perform_environment_checks()
     if skip_providers:
         ignored_path_list = [
-            f"--ignore=tests/providers/{provider_id.replace('.','/')}"
+            f"--ignore=tests/providers/{provider_id.replace('.', '/')}"
             for provider_id in skip_providers.split(" ")
         ]
         extra_pytest_args = (*extra_pytest_args, *ignored_path_list)

dev/breeze/src/airflow_breeze/utils/selective_checks.py

@@ -870,10 +870,14 @@ def providers_test_types_list_as_string(self) -> str | None:
     def separate_test_types_list_as_string(self) -> str | None:
         if not self.run_tests:
             return None
-        current_test_types = set(self._get_test_types_to_run(split_to_individual_providers=True))
+        # current_test_types = set(self._get_test_types_to_run(split_to_individual_providers=True))
+        current_test_types = {"Providers"}
         if "Providers" in current_test_types:
             current_test_types.remove("Providers")
-            current_test_types.update({f"Providers[{provider}]" for provider in get_available_packages()})
+            # current_test_types.update({f"Providers[{provider}]" for provider in get_available_packages()})
+            current_test_types.update(
+                {f"Providers[{provider}]" for provider in ["cloudant", "http", "mongo", "sftp"]}
+            )
         if self.skip_provider_tests:
             current_test_types = {
                 test_type for test_type in current_test_types if not test_type.startswith("Providers")

dev/breeze/tests/test_run_test.py

@@ -0,0 +1,181 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from unittest.mock import patch
+
+import pytest
+
+from airflow_breeze.commands.testing_commands import _run_test
+from airflow_breeze.params.shell_params import ShellParams
+
+
+@pytest.fixture(autouse=True)
+def mock_run_command():
+    """We mock run_command to capture its call args; it returns nothing so mock training is unnecessary."""
+    with patch("airflow_breeze.commands.testing_commands.run_command") as mck:
+        yield mck
+
+
+@pytest.fixture(autouse=True)
+def mock_generate_args_for_pytest():
+    with patch("airflow_breeze.commands.testing_commands.generate_args_for_pytest") as mck:
+        yield mck
+
+
+@pytest.fixture(autouse=True)
+def mock_get_suspended_provider_folders():
+    with patch("airflow_breeze.utils.run_tests.get_suspended_provider_folders") as mck:
+        mck.return_value = []
+        yield mck
+
+
+@pytest.fixture(autouse=True)
+def mock_get_excluded_provider_folders():
+    with patch("airflow_breeze.utils.run_tests.get_excluded_provider_folders") as mck:
+        mck.return_value = []
+        yield mck
+
+
+@pytest.fixture(autouse=True)
+def mock_sleep():
+    """_run_test does a 10-second sleep in CI, so we mock the sleep function to save CI test time."""
+    with patch("airflow_breeze.commands.testing_commands.sleep"):
+        yield
+
+
+@pytest.fixture(autouse=True)
+def mock_remove_docker_networks():
+    """We mock remove_docker_networks to avoid making actual docker calls during these tests;
+    it returns nothing so mock training is unnecessary."""
+    with patch("airflow_breeze.commands.testing_commands.remove_docker_networks") as mck:
+        yield mck
+
+
+def test_exits_on_bad_test_type():
+    """Verify the quick bail-out if the test type doesn't make sense."""
+    with pytest.raises(SystemExit) as se:
+        _run_test(
+            shell_params=ShellParams(test_type="[bogus]TestType"),
+            extra_pytest_args=(),
+            python_version="3.8",
+            output=None,
+            test_timeout=60,
+            skip_docker_compose_down=True,
+        )
+    print(se.value.code)
+
+
+def test_calls_docker_down(mock_run_command):
+    """Verify docker down is called with expected arguments."""
+    _run_test(
+        shell_params=ShellParams(test_type="Core"),
+        extra_pytest_args=(),
+        python_version="3.8",
+        output=None,
+        test_timeout=60,
+        skip_docker_compose_down=True,
+    )
+
+    docker_down_call_args = mock_run_command.call_args_list[0].args[0]
+
+    assert docker_down_call_args == [
+        "docker",
+        "compose",
+        "--project-name",
+        "airflow-test-core",
+        "down",
+        "--remove-orphans",
+        "--volumes",
+    ]
+
+
+def test_calls_docker_run_with_expected_args(mock_run_command, mock_generate_args_for_pytest):
+    """This test verifies that 'docker run' is called with arguments concatenated from:
+    (a) the docker compose command
+    (b) generate_args_for_pytest(), and
+    (c) the extra pytest args
+
+    We mock generate_args_for_pytest() since it calls out to another module.
+    """
+    test_mocked_pytest_args = ["pytest_arg_0", "pytest_arg_1"]
+    test_extra_pytest_args = ("extra_1", "extra_2")
+    mock_generate_args_for_pytest.return_value = test_mocked_pytest_args
+
+    _run_test(
+        shell_params=ShellParams(test_type="Core"),
+        extra_pytest_args=test_extra_pytest_args,
+        python_version="3.8",
+        output=None,
+        test_timeout=60,
+        skip_docker_compose_down=True,
+    )
+
+    docker_run_call_args = mock_run_command.call_args_list[1].args[0]
+
+    assert docker_run_call_args == [
+        # docker command args:
+        "docker",
+        "compose",
+        "--project-name",
+        "airflow-test-core",
+        "run",
+        "-T",
+        "--service-ports",
+        "--rm",
+        "airflow",
+        # test_mocked_pytest_args:
+        *test_mocked_pytest_args,
+        # extra_pytest_args:
+        *test_extra_pytest_args,
+    ]
+
+
+def test_returns_when_one_test_directory_is_also_ignored(mock_run_command, mock_generate_args_for_pytest):
+    test_mocked_pytest_args = ["tests/providers/alpha", "tests/providers/beta"]
+    test_extra_pytest_args = ("--ignore=tests/providers/alpha", "--verbose")
+    mock_generate_args_for_pytest.return_value = test_mocked_pytest_args
+
+    result = _run_test(
+        shell_params=ShellParams(test_type="Providers[alpha,beta]"),
+        extra_pytest_args=test_extra_pytest_args,
+        python_version="3.8",
+        output=None,
+        test_timeout=60,
+        skip_docker_compose_down=True,
+    )
+
+    assert result == (0, "Test: Providers[alpha,beta] blocked from running due to ambiguous arguments")
+    mock_run_command.assert_not_called()
+
+
+def test_returns_when_all_test_directories_are_also_ignored(mock_run_command, mock_generate_args_for_pytest):
+    test_mocked_pytest_args = ["tests/providers/alpha"]
+    test_extra_pytest_args = ("--ignore=tests/providers/alpha",)
+    mock_generate_args_for_pytest.return_value = test_mocked_pytest_args
+
+    result = _run_test(
+        shell_params=ShellParams(test_type="Providers[alpha]"),
+        extra_pytest_args=test_extra_pytest_args,
+        python_version="3.8",
+        output=None,
+        test_timeout=60,
+        skip_docker_compose_down=True,
+    )
+
+    assert result == (0, "Test: Providers[alpha] blocked from running due to ambiguous arguments")
+    mock_run_command.assert_not_called()