Skip to content

Commit c552e97

Browse files
committed
Remove requirement for test_ prefix for pytest test modules
Currently we had a requirement to use `test_` prefix for modules for all our tests, however this means that we could have missed some of the tests from pytest collection when they were placed in a module without the `test_` prefix. This happened already: see apache#30311 and apache#30306 The previous attempts to remove it failed, because cassandra tests seemed to be incompatible with pytest collection when we allowed all Python files, also there were a few names that caused the pytest collection to fail with selecting all Python files for collection. The move is accompanied by converting pytest configuration to pyproject.toml. After long investigation, it turned out that the cause of cassandra failures was pytest assert rewrite with collided with Cython-related type_codes.py definition of types. See datastax/python-driver#1142 for PR that is aimed to fix it on cassandra side, in the meantime we are manually patching the type_codes.py file from Cassandra by adding PYTEST_DONT_REWRITE to its docstring when building the CI image for testing. Another error was using run_module method which also suffers from assert rewriting incompatibilities but this could be fixed by using run_path instead. pytest-dev/pytest#9007 Yet another assert-rewrite problem was that ``test_airflow_context`` also failed with ``--assert=rewrite`` turned on when python_files were not filtered. No solution was found on how we can disable the rewrite (apparently it happens for yaml.py file but adding PYTEST_DONT_REWRITE to yaml.py and related files did not help, so we had to extract the test to separate test type) Also test in docker_tests/kubernetes_tests should be run with working directory being in their folders in order to apply pyproject.toml from their directores (without specifying asyncio mode). The following changes are applied: * conversion of pytest configuration from pytest.ini to pyproject.toml (for main project, docker_tests and breeze) * addedd pyproject.toml to breeze, docker_tests, kubernetes_tests to reflect they need different configuration for pytest (lack of pytest-asyncio for example) * adding automated patching of Cassandra type_codes.py with PYTEST_DONT_REWRITE docstring marker * add pyproject.toml to docker context in order to include it in sources of airflow in the image * changed working directory for k8s suite of breeze commands to be kubernetes_tests * remove pytest.ini from docker compose mounting and automated removal in entrypoin in case old image is used * rename few breeze function to not start with "test_" so that they are not collected as test methods * remove few test dags from collecton by pytest by name * replace run_module with run_path usage in EKS test and test_www. * CI workflow is updated to use docker_tests as working directory for those tests * perf dags were moved out of the tests/test_utils dir to dev directory. * PlainAsserts test type was added and the ``test_airflow_context`` test is run in this separate test type when running parallel tests by default.
1 parent 6e75181 commit c552e97

37 files changed

+306
-98
lines changed

.dockerignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
!.coveragerc
4747
!.rat-excludes
4848
!.dockerignore
49-
!pytest.ini
5049
!RELEASE_NOTES.rst
5150
!LICENSE
5251
!MANIFEST.in
@@ -71,6 +70,7 @@
7170
# Setup/version configuration
7271
!setup.cfg
7372
!setup.py
73+
!pyproject.toml
7474
!manifests
7575
!generated
7676
# Now - ignore unnecessary files inside allowed directories

.github/workflows/ci.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,8 +499,9 @@ jobs:
499499
cache-dependency-path: ./dev/requirements.txt
500500
- name: "Test examples of PROD image building"
501501
run: >
502-
python -m pip install -r ./docker_tests/requirements.txt &&
503-
python -m pytest docker_tests/test_examples_of_prod_image_building.py -n auto --color=yes
502+
cd ./docker_tests &&
503+
python -m pip install -r requirements.txt &&
504+
python -m pytest test_examples_of_prod_image_building.py -n auto --color=yes
504505
505506
test-git-clone-on-windows:
506507
timeout-minutes: 5

BREEZE.rst

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,16 @@ For example this will run API and WWW tests in parallel:
786786
787787
breeze testing tests --test-types "API WWW" --run-in-parallel
788788
789+
There are few special types of tests that you can run:
790+
791+
* ``All`` - all tests are run in single pytest run.
792+
* ``PlainAsserts`` - some tests of ours fail when ``--assert=rewrite`` feature of pytest is used. This
793+
is in order to get better output of ``assert`` statements This is a special test type that runs those
794+
select tests tests with ``--assert=plain`` flag.
795+
* ``Postgres`` - runs all tests that require Postgres database
796+
* ``MySQL`` - runs all tests that require MySQL database
797+
* ``Quarantine`` - runs all tests that are in quarantine (marked with ``@pytest.mark.quarantined``
798+
decorator)
789799

790800
Here is the detailed set of options for the ``breeze testing tests`` command.
791801

@@ -1017,7 +1027,7 @@ Run selected tests:
10171027

10181028
.. code-block::bash
10191029
1020-
breeze k8s tests kubernetes_tests/test_kubernetes_executor.py
1030+
breeze k8s tests test_kubernetes_executor.py
10211031
10221032
All parameters of the command are here:
10231033

@@ -1042,7 +1052,7 @@ output during test execution.
10421052

10431053
.. code-block::bash
10441054
1045-
breeze k8s tests -- kubernetes_tests/test_kubernetes_executor.py -s
1055+
breeze k8s tests -- test_kubernetes_executor.py -s
10461056
10471057
Running k8s complete tests
10481058
..........................
@@ -1061,7 +1071,7 @@ Run selected tests:
10611071

10621072
.. code-block::bash
10631073
1064-
breeze k8s run-complete-tests kubernetes_tests/test_kubernetes_executor.py
1074+
breeze k8s run-complete-tests test_kubernetes_executor.py
10651075
10661076
All parameters of the command are here:
10671077

@@ -1086,7 +1096,7 @@ output during test execution.
10861096

10871097
.. code-block::bash
10881098
1089-
breeze k8s run-complete-tests -- kubernetes_tests/test_kubernetes_executor.py -s
1099+
breeze k8s run-complete-tests -- test_kubernetes_executor.py -s
10901100
10911101
10921102
Entering k8s shell
@@ -1110,16 +1120,16 @@ be created and airflow deployed to it before running the tests):
11101120

11111121
.. code-block::bash
11121122
1113-
(kind-airflow-python-3.9-v1.24.0:KubernetesExecutor)> pytest kubernetes_tests/test_kubernetes_executor.py
1123+
(kind-airflow-python-3.9-v1.24.0:KubernetesExecutor)> pytest test_kubernetes_executor.py
11141124
================================================= test session starts =================================================
11151125
platform linux -- Python 3.10.6, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /home/jarek/code/airflow/.build/.k8s-env/bin/python
11161126
cachedir: .pytest_cache
11171127
rootdir: /home/jarek/code/airflow, configfile: pytest.ini
11181128
plugins: anyio-3.6.1
11191129
collected 2 items
11201130
1121-
kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag PASSED [ 50%]
1122-
kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure PASSED [100%]
1131+
test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag PASSED [ 50%]
1132+
test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure PASSED [100%]
11231133
11241134
================================================== warnings summary ===================================================
11251135
.build/.k8s-env/lib/python3.10/site-packages/_pytest/config/__init__.py:1233

Dockerfile.ci

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,8 @@ if [[ ${SKIP_ENVIRONMENT_INITIALIZATION=} != "true" ]]; then
852852
fi
853853
fi
854854

855+
rm -f "${AIRFLOW_SOURCES}/pytest.ini"
856+
855857
set +u
856858
if [[ "${RUN_TESTS}" != "true" ]]; then
857859
exec /bin/bash "${@}"
@@ -1009,6 +1011,16 @@ else
10091011
echo "${COLOR_YELLOW}Skip ${providers_dir} as the directory does not exist.${COLOR_RESET}"
10101012
fi
10111013
done
1014+
elif [[ ${TEST_TYPE} =~ PlainAsserts ]]; then
1015+
# Those tests fail when --asert=rewrite is set, therefore we run them separately
1016+
# with --assert=plain to make sure they pass.
1017+
SELECTED_TESTS=(
1018+
# this on is mysteriously failing dill serialization. It could be removed once
1019+
# https://github.com/pytest-dev/pytest/issues/10845 is fixed
1020+
"tests/operators/test_python.py::TestPythonVirtualenvOperator::test_airflow_context"
1021+
)
1022+
EXTRA_PYTEST_ARGS+=("--assert=plain")
1023+
export PYTEST_PLAIN_ASSERTS="true"
10121024
else
10131025
echo
10141026
echo "${COLOR_RED}ERROR: Wrong test type ${TEST_TYPE} ${COLOR_RESET}"
@@ -1073,6 +1085,32 @@ COPY <<"EOF" /entrypoint_exec.sh
10731085
exec /bin/bash "${@}"
10741086
EOF
10751087

1088+
# The content below is automatically copied from scripts/docker/patch_cassandra_type_code.py
1089+
COPY <<"EOF" /patch_cassandra_type_code.py
1090+
#!/usr/bin/env python
1091+
1092+
from __future__ import annotations
1093+
1094+
import cassandra.type_codes as cassandra_type_codes
1095+
1096+
if __name__ == "__main__":
1097+
print()
1098+
path_to_patch = cassandra_type_codes.__file__
1099+
with open(path_to_patch, "r+") as f:
1100+
content = f.read()
1101+
if "PYTEST_DONT_REWRITE" in content:
1102+
print(f"The {path_to_patch} is already patched with PYTEST_DONT_REWRITE")
1103+
print()
1104+
exit(0)
1105+
f.seek(0)
1106+
content = content.replace('"""', '"""\nPYTEST_DONT_REWRITE', 1)
1107+
f.write(content)
1108+
f.truncate()
1109+
print(f"Patched {path_to_patch} with PYTEST_DONT_REWRITE")
1110+
print()
1111+
exit(0)
1112+
EOF
1113+
10761114
FROM ${PYTHON_BASE_IMAGE} as main
10771115

10781116
# Nolog bash flag is currently ignored - but you can replace it with other flags (for example
@@ -1301,6 +1339,15 @@ RUN bash /scripts/docker/install_pip_version.sh; \
13011339
bash /scripts/docker/install_additional_dependencies.sh; \
13021340
fi
13031341

1342+
COPY --from=scripts patch_cassandra_type_code.py /patch_cassandra_type_code.py
1343+
1344+
# Patch cassandra type_code to avoide accidental type_code assert rewriting breaking pytest
1345+
# test discovery and execution.
1346+
# This one can be fixed once https://github.com/pytest-dev/pytest/issues/10844
1347+
# is fixed and released or once the workaround is merged in cassandra-driver
1348+
# https://github.com/datastax/python-driver/pull/1142
1349+
RUN python /patch_cassandra_type_code.py
1350+
13041351
# Install autocomplete for airflow
13051352
RUN if command -v airflow; then \
13061353
register-python-argcomplete airflow >> ~/.bashrc ; \
@@ -1309,6 +1356,7 @@ RUN if command -v airflow; then \
13091356
# Install autocomplete for Kubectl
13101357
RUN echo "source /etc/bash_completion" >> ~/.bashrc
13111358

1359+
13121360
# We can copy everything here. The Context is filtered by dockerignore. This makes sure we are not
13131361
# copying over stuff that is accidentally generated or that we do not need (such as egg-info)
13141362
# if you want to add something that is missing and you expect to see it in the image you can

TESTING.rst

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,15 +1154,15 @@ The virtualenv required will be created automatically when the scripts are run.
11541154
========================================================================================= test session starts ==========================================================================================
11551155
platform darwin -- Python 3.9.9, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /Users/jarek/IdeaProjects/airflow/.build/.k8s-env/bin/python
11561156
cachedir: .pytest_cache
1157-
rootdir: /Users/jarek/IdeaProjects/airflow, configfile: pytest.ini
1157+
rootdir: /Users/jarek/IdeaProjects/airflow/kubernetes_tests
11581158
plugins: anyio-3.6.1, instafail-0.4.2, xdist-2.5.0, forked-1.4.0, timeouts-1.2.1, cov-3.0.0
11591159
setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
11601160
collected 55 items
11611161
1162-
kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag PASSED [ 1%]
1163-
kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure PASSED [ 3%]
1164-
kubernetes_tests/test_kubernetes_pod_operator.py::TestKubernetesPodOperatorSystem::test_already_checked_on_failure PASSED [ 5%]
1165-
kubernetes_tests/test_kubernetes_pod_operator.py::TestKubernetesPodOperatorSystem::test_already_checked_on_success ...
1162+
test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag PASSED [ 1%]
1163+
test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure PASSED [ 3%]
1164+
test_kubernetes_pod_operator.py::TestKubernetesPodOperatorSystem::test_already_checked_on_failure PASSED [ 5%]
1165+
test_kubernetes_pod_operator.py::TestKubernetesPodOperatorSystem::test_already_checked_on_success ...
11661166
11671167
8b) You can enter an interactive shell to run tests one-by-one
11681168

@@ -1239,11 +1239,12 @@ and this is where KUBECONFIG env should point to.
12391239

12401240
You can iterate with tests while you are in the virtualenv. All the tests requiring Kubernetes cluster
12411241
are in "kubernetes_tests" folder. You can add extra ``pytest`` parameters then (for example ``-s`` will
1242-
print output generated test logs and print statements to the terminal immediately.
1242+
print output generated test logs and print statements to the terminal immediately. You should have
1243+
kubernetes_tests as your working directory.
12431244

12441245
.. code-block:: bash
12451246
1246-
pytest kubernetes_tests/test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure -s
1247+
pytest test_kubernetes_executor.py::TestKubernetesExecutor::test_integration_run_dag_with_scheduler_failure -s
12471248
12481249
You can modify the tests or KubernetesPodOperator and re-run them without re-deploying
12491250
Airflow to KinD cluster.

dev/airflow-github

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ def is_core_commit(files: list[str]) -> bool:
146146
"images/",
147147
"TESTING.rst",
148148
"codecov.yml",
149-
"pytest.ini",
150149
"kubernetes_tests/",
151150
".github/",
152151
".pre-commit-config.yaml",

dev/breeze/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,6 @@ PLEASE DO NOT MODIFY THE HASH BELOW! IT IS AUTOMATICALLY UPDATED BY PRE-COMMIT.
5252

5353
---------------------------------------------------------------------------------------------------------
5454

55-
Package config hash: a98c6dc9a12523e5aaacaade98c721e869377ab59eead92344006f7fd71fe2978398b87703dfc5efa61fac372a4e798cef9de26d6f4283a47f919cd7e0dc4dfa
55+
Package config hash: 279584a15bd0cec7a270b8fb85c3a6f853abca893fd4ecb419636fc2319a75d3644bd76e773008d6e3f77440fb2651a307f974a13df32191bea2c2f471c368f2
5656

5757
---------------------------------------------------------------------------------------------------------

dev/breeze/pyproject.toml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,19 @@
1717
[tool.black]
1818
line-length = 110
1919
target-version = ['py37', 'py38', 'py39', 'py310']
20+
21+
[tool.pytest.ini_options]
22+
addopts = "-rasl --verbosity=2 -p no:flaky -p no:nose"
23+
norecursedirs = [
24+
".eggs",
25+
]
26+
log_level = "INFO"
27+
filterwarnings = [
28+
"error::pytest.PytestCollectionWarning",
29+
]
30+
python_files = [
31+
"*.py",
32+
]
33+
testpaths = [
34+
"tests",
35+
]

dev/breeze/src/airflow_breeze/breeze.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@
3535
from airflow_breeze.commands.production_image_commands import prod_image # noqa
3636
from airflow_breeze.commands.release_management_commands import release_management # noqa
3737
from airflow_breeze.commands.setup_commands import setup # noqa
38-
from airflow_breeze.commands.testing_commands import testing # noqa
38+
from airflow_breeze.commands.testing_commands import group_for_testing # noqa
3939

40-
main.add_command(testing)
40+
main.add_command(group_for_testing)
4141
main.add_command(kubernetes_group)
4242
main.add_command(ci_group)
4343
main.add_command(ci_image)

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,7 +1266,9 @@ def shell(
12661266
extra_args.append("--no-rcs")
12671267
elif shell_binary.endswith("bash"):
12681268
extra_args.extend(["--norc", "--noprofile"])
1269-
result = run_command([shell_binary, *extra_args, *shell_args], env=env, check=False)
1269+
result = run_command(
1270+
[shell_binary, *extra_args, *shell_args], env=env, check=False, cwd="kubernetes_tests"
1271+
)
12701272
if result.returncode != 0:
12711273
sys.exit(result.returncode)
12721274

@@ -1300,17 +1302,15 @@ def _run_tests(
13001302
extra_shell_args.append("--no-rcs")
13011303
elif shell_binary.endswith("bash"):
13021304
extra_shell_args.extend(["--norc", "--noprofile"])
1303-
the_tests = []
1304-
if not any(arg.startswith("kubernetes_tests") for arg in test_args):
1305-
# if no tests specified - use args
1306-
the_tests.append("kubernetes_tests")
1305+
the_tests: list[str] = []
13071306
command_to_run = " ".join([quote(arg) for arg in ["pytest", *the_tests, *test_args]])
13081307
get_console(output).print(f"[info] Command to run:[/] {command_to_run}")
13091308
result = run_command(
13101309
[shell_binary, *extra_shell_args, "-c", command_to_run],
13111310
output=output,
13121311
env=env,
13131312
check=False,
1313+
cwd="kubernetes_tests",
13141314
)
13151315
return result.returncode, f"Tests {kubectl_cluster_name}"
13161316

@@ -1336,7 +1336,7 @@ def _run_tests(
13361336
@option_verbose
13371337
@option_dry_run
13381338
@click.argument("test_args", nargs=-1, type=click.Path())
1339-
def tests(
1339+
def kubernetes_tests_command(
13401340
python: str,
13411341
kubernetes_version: str,
13421342
executor: str,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
from airflow_breeze.commands.ci_image_commands import ci_image
2828
from airflow_breeze.commands.production_image_commands import prod_image
29-
from airflow_breeze.commands.testing_commands import testing
29+
from airflow_breeze.commands.testing_commands import group_for_testing
3030
from airflow_breeze.configure_rich_click import click
3131
from airflow_breeze.utils.click_utils import BreezeGroup
3232
from airflow_breeze.utils.common_options import (
@@ -74,7 +74,7 @@ def get_command(self, ctx: Context, cmd_name: str):
7474
return prod_image.get_command(ctx, "build")
7575
if cmd_name == "tests":
7676
print_deprecated("tests", "testing tests")
77-
return testing.get_command(ctx, "tests")
77+
return group_for_testing.get_command(ctx, "tests")
7878
if cmd_name == "config":
7979
print_deprecated("config", "setup config")
8080
return setup.get_command(ctx, "config")

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,11 @@
7272

7373

7474
@click.group(cls=BreezeGroup, name="testing", help="Tools that developers can use to run tests")
75-
def testing():
75+
def group_for_testing():
7676
pass
7777

7878

79-
@testing.command(
79+
@group_for_testing.command(
8080
name="docker-compose-tests",
8181
context_settings=dict(
8282
ignore_unknown_options=True,
@@ -308,7 +308,7 @@ def run_tests_in_parallel(
308308
)
309309

310310

311-
@testing.command(
311+
@group_for_testing.command(
312312
name="tests",
313313
help="Run the specified unit test targets.",
314314
context_settings=dict(
@@ -347,7 +347,7 @@ def run_tests_in_parallel(
347347
@click.option(
348348
"--test-types",
349349
help="Space separated list of test types used for testing in parallel.",
350-
default=" ".join(all_selective_test_types()),
350+
default=" ".join(all_selective_test_types()) + " PlainAsserts",
351351
show_default=True,
352352
envvar="TEST_TYPES",
353353
)
@@ -366,7 +366,7 @@ def run_tests_in_parallel(
366366
@option_verbose
367367
@option_dry_run
368368
@click.argument("extra_pytest_args", nargs=-1, type=click.UNPROCESSED)
369-
def tests(
369+
def command_for_tests(
370370
python: str,
371371
backend: str,
372372
postgres_version: str,
@@ -432,7 +432,7 @@ def tests(
432432
sys.exit(returncode)
433433

434434

435-
@testing.command(
435+
@group_for_testing.command(
436436
name="integration-tests",
437437
help="Run the specified integratio tests.",
438438
context_settings=dict(
@@ -506,7 +506,7 @@ def integration_tests(
506506
sys.exit(returncode)
507507

508508

509-
@testing.command(
509+
@group_for_testing.command(
510510
name="helm-tests",
511511
help="Run Helm chart tests.",
512512
context_settings=dict(

dev/breeze/src/airflow_breeze/global_constants.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ class SelectiveUnitTestTypes(Enum):
109109
ALLOWED_TEST_TYPE_CHOICES = [
110110
"All",
111111
*all_selective_test_types(),
112-
"Helm",
112+
"PlainAsserts",
113113
"Postgres",
114114
"MySQL",
115115
"Quarantine",

0 commit comments

Comments
 (0)