Skip to content

Commit a6474e1

Browse files
authored
Remove side-effects in models/tests_dags affecting plugin manager tests (#52258)
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
1 parent ce8aa4d commit a6474e1

File tree

1 file changed

+15
-17
lines changed

1 file changed

+15
-17
lines changed

airflow-core/tests/unit/plugins/test_plugins_manager.py

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ def on_load(self, *args, **kwargs):
4949

5050

5151
@pytest.fixture(autouse=True, scope="module")
52-
def clean_plugins():
52+
def _clean_listeners():
5353
get_listener_manager().clear()
5454
yield
5555
get_listener_manager().clear()
@@ -268,22 +268,20 @@ class MacroPlugin(AirflowPlugin):
268268
def test_registering_plugin_listeners(self):
269269
from airflow import plugins_manager
270270

271-
try:
272-
with mock.patch("airflow.plugins_manager.plugins", []):
273-
plugins_manager.load_plugins_from_plugin_directory()
274-
plugins_manager.integrate_listener_plugins(get_listener_manager())
275-
276-
assert get_listener_manager().has_listeners
277-
listeners = get_listener_manager().pm.get_plugins()
278-
listener_names = [el.__name__ if inspect.ismodule(el) else qualname(el) for el in listeners]
279-
# sort names as order of listeners is not guaranteed
280-
assert sorted(listener_names) == [
281-
"airflow.example_dags.plugins.event_listener",
282-
"unit.listeners.class_listener.ClassBasedListener",
283-
"unit.listeners.empty_listener",
284-
]
285-
finally:
286-
get_listener_manager().clear()
271+
assert not get_listener_manager().has_listeners
272+
with mock.patch("airflow.plugins_manager.plugins", []):
273+
plugins_manager.load_plugins_from_plugin_directory()
274+
plugins_manager.integrate_listener_plugins(get_listener_manager())
275+
276+
assert get_listener_manager().has_listeners
277+
listeners = get_listener_manager().pm.get_plugins()
278+
listener_names = [el.__name__ if inspect.ismodule(el) else qualname(el) for el in listeners]
279+
# sort names as order of listeners is not guaranteed
280+
assert sorted(listener_names) == [
281+
"airflow.example_dags.plugins.event_listener",
282+
"unit.listeners.class_listener.ClassBasedListener",
283+
"unit.listeners.empty_listener",
284+
]
287285

288286
@skip_if_force_lowest_dependencies_marker
289287
def test_should_import_plugin_from_providers(self):

0 commit comments

Comments
 (0)