Skip to content

Commit

Permalink
Raise an error on Airflow Deprecation warnings in tests (#38504)
Browse files Browse the repository at this point in the history
* POC: Raise an error on Airflow Deprecation warnings in tests

* Add fixture for check deprecations

* Ignore providers tests per test

* Rever changes in test_pod.py

* Add categories for tests, and evaluate most of the tests

* Exclude TestSetDagRunNote::test_should_respond_200_with_anonymous_user

* Check other groups

* Ignore FAB / Connexion 2 specific errors

* Remove non-compliant DbApiHook deprecations

* Disable buitin warning summary and buitify output

* Add information about prohibited warnings

* Add to ignore additional tests from tests/models/test_taskinstance.py::TestTaskInstance

(cherry picked from commit 50a4c95)
  • Loading branch information
Taragolis authored and ephraimbuddy committed Apr 2, 2024
1 parent 4e392c6 commit 256a255
Show file tree
Hide file tree
Showing 4 changed files with 1,141 additions and 50 deletions.
43 changes: 38 additions & 5 deletions contributing-docs/testing/unit_tests.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,45 @@ Follow the guidelines when writing unit tests:

* For standard unit tests that do not require integrations with external systems, make sure to simulate all communications.
* All Airflow tests are run with ``pytest``. Make sure to set your IDE/runners (see below) to use ``pytest`` by default.
* For new tests, use standard "asserts" of Python and ``pytest`` decorators/context managers for testing
rather than ``unittest`` ones. See `pytest docs <http://doc.pytest.org/en/latest/assert.html>`_ for details.
* Use a parameterized framework for tests that have variations in parameters.
* For tests, use standard "asserts" of Python and ``pytest`` decorators/context managers for testing
rather than ``unittest`` ones. See `pytest docs <http://doc.pytest.org/en/latest/assert.html>`__ for details.
* Use a ``pytest.mark.parametrize`` marker for tests that have variations in parameters.
See `pytest docs <https://docs.pytest.org/en/latest/how-to/parametrize.html>`__ for details.
* Use with ``pytest.warn`` to capture warnings rather than ``recwarn`` fixture. We are aiming for 0-warning in our
tests, so we run Pytest with ``--disable-warnings`` but instead we have ``pytest-capture-warnings`` plugin that
overrides ``recwarn`` fixture behaviour.
tests, so we run Pytest with ``--disable-warnings`` but instead we have custom warning capture system.

Handling warnings
.................

By default, in the new tests selected warnings are prohibited:

* ``airflow.exceptions.AirflowProviderDeprecationWarning``
* ``airflow.exceptions.RemovedInAirflow3Warning``
* ``airflow.utils.context.AirflowContextDeprecationWarning``

That mean if one of this warning appear during test run and do not captured the test will failed.

.. code-block:: console
root@91e633d08aa8:/opt/airflow# pytest tests/models/test_dag.py::TestDag::test_clear_dag
...
FAILED tests/models/test_dag.py::TestDag::test_clear_dag[None-None] - airflow.exceptions.RemovedInAirflow3Warning: Calling `DAG.create_dagrun()` without an explicit data interval is deprecated
For avoid this make sure:

* You do not use deprecated method, classes and arguments in your test cases;
* Your change do not affect other component, e.g. deprecate one part of Airflow Core or one of Community Supported
Providers might be a reason for new deprecation warnings. In this case changes should be also made in all affected
components in backward compatible way.
* You use ``pytest.warn`` (see `pytest doc <https://docs.pytest.org/en/latest/how-to/capture-warnings.html#warns>`__
context manager for catch warning during the test deprecated components.
Yes we still need to test legacy/deprecated stuff until it complitly removed)

.. code-block:: python
def test_deprecated_argument():
with pytest.warn(AirflowProviderDeprecationWarning, match="expected warning pattern"):
SomeDeprecatedClass(foo="bar", spam="egg")
Airflow configuration for unit tests
Expand Down
13 changes: 12 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,8 @@ fixture-parentheses = false
# * Disable `flaky` plugin for pytest. This plugin conflicts with `rerunfailures` because provide same marker.
# * Disable `nose` builtin plugin for pytest. This feature deprecated in 7.2 and will be removed in pytest>=8
# * And we focus on use native pytest capabilities rather than adopt another frameworks.
addopts = "-rasl --verbosity=2 -p no:flaky -p no:nose --asyncio-mode=strict"
# * Disable warnings summary, because we use our warning summary.
addopts = "-rasl --verbosity=2 -p no:flaky -p no:nose --asyncio-mode=strict --disable-warnings"
norecursedirs = [
".eggs",
"airflow",
Expand All @@ -468,10 +469,20 @@ filterwarnings = [
"error::pytest.PytestCollectionWarning",
"ignore::DeprecationWarning:flask_appbuilder.filemanager",
"ignore::DeprecationWarning:flask_appbuilder.widgets",
# FAB do not support SQLAclhemy 2
"ignore::sqlalchemy.exc.MovedIn20Warning:flask_appbuilder",
# https://github.com/dpgaspar/Flask-AppBuilder/issues/2194
"ignore::DeprecationWarning:marshmallow_sqlalchemy.convert",
# https://github.com/dpgaspar/Flask-AppBuilder/pull/1940
"ignore::DeprecationWarning:flask_sqlalchemy",
# https://github.com/dpgaspar/Flask-AppBuilder/pull/1903
"ignore::DeprecationWarning:apispec.utils",
# Connexion 2 use different deprecated objects, this should be resolved into Connexion 3
# https://github.com/spec-first/connexion/pull/1536
'ignore::DeprecationWarning:connexion.spec',
'ignore:jsonschema\.RefResolver:DeprecationWarning:connexion.json_schema',
'ignore:jsonschema\.exceptions\.RefResolutionError:DeprecationWarning:connexion.json_schema',
'ignore:Accessing jsonschema\.draft4_format_checker:DeprecationWarning:connexion.decorators.validation',
]
python_files = [
"test_*.py",
Expand Down
84 changes: 40 additions & 44 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

import pytest
import time_machine
import yaml

# We should set these before loading _any_ of the rest of airflow so that the
# unit test mode config is set as early as possible.
Expand Down Expand Up @@ -370,7 +371,6 @@ def pytest_configure(config: pytest.Config) -> None:
)
pytest.exit(msg, returncode=6)

config.addinivalue_line("filterwarnings", "error::airflow.utils.context.AirflowContextDeprecationWarning")
config.addinivalue_line("markers", "integration(name): mark test to run with named integration")
config.addinivalue_line("markers", "backend(name): mark test to run with named backend")
config.addinivalue_line("markers", "system(name): mark test to run with named system")
Expand Down Expand Up @@ -1206,6 +1206,28 @@ def cleanup_providers_manager():
ProvidersManager()._cleanup()


@pytest.fixture(scope="session")
def deprecations_ignore() -> tuple[str, ...]:
with open(Path(__file__).absolute().parent.resolve() / "deprecations_ignore.yml") as fp:
return tuple(yaml.safe_load(fp))


@pytest.fixture(autouse=True)
def check_deprecations(request: pytest.FixtureRequest, deprecations_ignore):
from airflow.exceptions import AirflowProviderDeprecationWarning, RemovedInAirflow3Warning
from airflow.utils.context import AirflowContextDeprecationWarning

if request.node.nodeid.startswith(deprecations_ignore):
yield
return

with warnings.catch_warnings():
warnings.simplefilter("error", AirflowProviderDeprecationWarning)
warnings.simplefilter("error", RemovedInAirflow3Warning)
warnings.simplefilter("error", AirflowContextDeprecationWarning)
yield


# The code below is a modified version of capture-warning code from
# https://github.com/athinkingape/pytest-capture-warnings

Expand Down Expand Up @@ -1246,54 +1268,24 @@ def pytest_runtest_call(item):
"""
Needed to grab the item.location information
"""
global warnings_recorder

if os.environ.get("PYTHONWARNINGS") == "ignore":
yield
return

warnings_recorder.__enter__()
yield
warnings_recorder.__exit__(None, None, None)

for warning in warnings_recorder.list:
# this code is adapted from python official warnings module

# Search the filters
for filter in warnings.filters:
action, msg, cat, mod, ln = filter

module = warning.filename or "<unknown>"
if module[-3:].lower() == ".py":
module = module[:-3] # XXX What about leading pathname?

if (
(msg is None or msg.match(str(warning.message)))
and issubclass(warning.category, cat)
and (mod is None or mod.match(module))
and (ln == 0 or warning.lineno == ln)
):
break
else:
action = warnings.defaultaction

# Early exit actions
if action == "ignore":
continue

warning.item = item
with warnings.catch_warnings(record=True) as records:
yield
for record in records:
quadruplet: tuple[str, int, type[Warning], str] = (
warning.filename,
warning.lineno,
warning.category,
str(warning.message),
record.filename,
record.lineno,
record.category,
str(record.message),
)

if quadruplet in captured_warnings:
captured_warnings_count[quadruplet] += 1
continue
else:
captured_warnings[quadruplet] = warning
captured_warnings[quadruplet] = record
captured_warnings_count[quadruplet] = 1


Expand All @@ -1314,11 +1306,12 @@ def format_test_function_location(item):
yield

if captured_warnings:
print("\n ======================== Warning summary =============================\n")
print(f" The tests generated {sum(captured_warnings_count.values())} warnings.")
print(f" After removing duplicates, {len(captured_warnings.values())} of them remained.")
print(f" They are stored in {warning_output_path} file.")
print("\n ======================================================================\n")
terminalreporter.section(
f"Warning summary. Total: {sum(captured_warnings_count.values())}, "
f"Unique: {len(captured_warnings.values())}",
yellow=True,
bold=True,
)
warnings_as_json = []

for warning in captured_warnings.values():
Expand Down Expand Up @@ -1348,6 +1341,9 @@ def format_test_function_location(item):
for i in warnings_as_json:
f.write(f'{i["path"]}:{i["lineno"]}: [W0513(warning), ] {i["warning_message"]}')
f.write("\n")
terminalreporter.write("Warnings saved into ")
terminalreporter.write(os.fspath(warning_output_path), yellow=True)
terminalreporter.write(" file.\n")
else:
# nothing, clear file
with warning_output_path.open("w") as f:
Expand Down
Loading

0 comments on commit 256a255

Please sign in to comment.