Skip to content

Commit

Permalink
Fix problem, skip tests if only not compatible providers are tested a… (
Browse files Browse the repository at this point in the history
apache#42204)

* Fix problem, skip tests if only not compatible providers are tested against old airflow versions

* Fix pytests

* Fix pytests
  • Loading branch information
jscheffl authored Sep 12, 2024
1 parent 3bc2d3a commit d3b411e
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 30 deletions.
43 changes: 23 additions & 20 deletions dev/breeze/src/airflow_breeze/commands/testing_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,29 +190,32 @@ 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,
)
pytest_args = 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))
pytest_args.extend(extra_pytest_args)
# Skip "FOLDER" in case "--ignore=FOLDER" is 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]
pytest_args_before_skip = pytest_args
pytest_args = [arg for arg in pytest_args if f"--ignore={arg}" not in pytest_args]
# Double check: If no test is leftover we can skip running the test
if pytest_args_before_skip != pytest_args and pytest_args[0].startswith("--"):
return 0, f"Skipped test, no tests needed: {shell_params.test_type}"
run_cmd.extend(pytest_args)
try:
remove_docker_networks(networks=[f"{compose_project_name}_default"])
result = run_command(
Expand Down
33 changes: 23 additions & 10 deletions dev/breeze/tests/test_run_test_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,24 +100,18 @@ def test_irregular_provider_with_extra_ignore_should_be_valid_cmd(mock_run_comma


def test_primary_test_arg_is_excluded_by_extra_pytest_arg(mock_run_command):
"""This code scenario currently has a bug - if a test type resolves to a single test directory,
but the same directory is also set to be ignored (either by extra_pytest_args or because a provider is
suspended or excluded), the _run_test function removes the test directory from the argument list,
which has the effect of running all of the tests pytest can find. Not good!
NB: this test accurately describes the buggy behavior; IOW when fixing the bug the test must be changed.
TODO: fix this bug that runs unintended tests; probably the correct behavior is to skip the run."""
test_provider = "http" # "Providers[<id>]" scans the source tree so we need to use a real provider id
test_provider_not_skipped = "ftp"
_run_test(
shell_params=ShellParams(test_type=f"Providers[{test_provider}]"),
shell_params=ShellParams(test_type=f"Providers[{test_provider},{test_provider_not_skipped}]"),
extra_pytest_args=(f"--ignore=tests/providers/{test_provider}",),
python_version="3.8",
output=None,
test_timeout=60,
skip_docker_compose_down=True,
)

assert mock_run_command.call_count > 1
run_cmd_call = mock_run_command.call_args_list[1]
arg_str = " ".join(run_cmd_call.args[0])

Expand All @@ -127,6 +121,25 @@ def test_primary_test_arg_is_excluded_by_extra_pytest_arg(mock_run_command):
# bc without a directory or module arg, pytest tests everything (which we don't want!)
# We check "--verbosity=0" to ensure nothing is between the airflow container id and the verbosity arg,
# IOW that the primary test arg is removed
match_pattern = re.compile(f"airflow --verbosity=0 .+ --ignore=tests/providers/{test_provider}")
match_pattern = re.compile(
f"airflow tests/providers/{test_provider_not_skipped} --verbosity=0 .+ --ignore=tests/providers/{test_provider}"
)

assert match_pattern.search(arg_str)


def test_test_is_skipped_if_all_are_ignored(mock_run_command):
test_providers = [
"http",
"ftp",
] # "Providers[<id>]" scans the source tree so we need to use a real provider id
_run_test(
shell_params=ShellParams(test_type=f"Providers[{','.join(test_providers)}]"),
extra_pytest_args=[f"--ignore=tests/providers/{provider}" for provider in test_providers],
python_version="3.8",
output=None,
test_timeout=60,
skip_docker_compose_down=True,
)

mock_run_command.assert_called_once() # called only to compose down

0 comments on commit d3b411e

Please sign in to comment.