From f2c65d4a69e36663fc4d41d8798747fca2efef1e Mon Sep 17 00:00:00 2001 From: Ash Berlin-Taylor Date: Wed, 25 Jun 2025 17:13:00 +0100 Subject: [PATCH] Remove side-effects in models/tests_dags affecting plugin manager tests I guess CI must not run this exact combination of tests together, but prior to this change if you ran `pytest airflow-core/tests/unit/models/test_dag.py::TestDag::test_bulk_write_to_db_assets airflow-core/tests/unit/plugins/test_plugins_manager.py::TestPluginsManager::test_registering_plugin_listeners` you would get a test failure. The issue was caused by having two fixtures of the same name, a module level `clean_plugins`, and a class level one. This is by design in Pytest and is how to override plugins at different scopes. This also explains why we had `listener_manager.clear()` in a finally block when it should have been handled by the fixture --- .../unit/plugins/test_plugins_manager.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/airflow-core/tests/unit/plugins/test_plugins_manager.py b/airflow-core/tests/unit/plugins/test_plugins_manager.py index 3b61297f0b1b9..addcc4745a2fc 100644 --- a/airflow-core/tests/unit/plugins/test_plugins_manager.py +++ b/airflow-core/tests/unit/plugins/test_plugins_manager.py @@ -49,7 +49,7 @@ def on_load(self, *args, **kwargs): @pytest.fixture(autouse=True, scope="module") -def clean_plugins(): +def _clean_listeners(): get_listener_manager().clear() yield get_listener_manager().clear() @@ -268,22 +268,20 @@ class MacroPlugin(AirflowPlugin): def test_registering_plugin_listeners(self): from airflow import plugins_manager - try: - with mock.patch("airflow.plugins_manager.plugins", []): - plugins_manager.load_plugins_from_plugin_directory() - plugins_manager.integrate_listener_plugins(get_listener_manager()) - - assert get_listener_manager().has_listeners - listeners = get_listener_manager().pm.get_plugins() - listener_names = [el.__name__ if inspect.ismodule(el) else qualname(el) for el in listeners] - # sort names as order of listeners is not guaranteed - assert sorted(listener_names) == [ - "airflow.example_dags.plugins.event_listener", - "unit.listeners.class_listener.ClassBasedListener", - "unit.listeners.empty_listener", - ] - finally: - get_listener_manager().clear() + assert not get_listener_manager().has_listeners + with mock.patch("airflow.plugins_manager.plugins", []): + plugins_manager.load_plugins_from_plugin_directory() + plugins_manager.integrate_listener_plugins(get_listener_manager()) + + assert get_listener_manager().has_listeners + listeners = get_listener_manager().pm.get_plugins() + listener_names = [el.__name__ if inspect.ismodule(el) else qualname(el) for el in listeners] + # sort names as order of listeners is not guaranteed + assert sorted(listener_names) == [ + "airflow.example_dags.plugins.event_listener", + "unit.listeners.class_listener.ClassBasedListener", + "unit.listeners.empty_listener", + ] @skip_if_force_lowest_dependencies_marker def test_should_import_plugin_from_providers(self):