Skip to content

Remove side-effects in plugin manager tests #52258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

ashb
Copy link
Member

@ashb ashb commented Jun 25, 2025

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

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
@ashb ashb changed the title Remove side-effects in models/tests_dags affecting plugin manager tests Remove side-effects in plugin manager tests Jun 25, 2025
@ashb ashb merged commit a6474e1 into main Jun 25, 2025
54 checks passed
@ashb ashb deleted the fix-pluginmgr-and-asset-test-leaking-state branch June 25, 2025 17:18
@potiuk
Copy link
Member

potiuk commented Jun 26, 2025

Nice. I think a lot depends on sequence of executions and cleanups it between

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants