From 29b49a8cc91adf8fc2ad5586d7a7e28dc80af671 Mon Sep 17 00:00:00 2001 From: Sergey Motornyuk Date: Sat, 10 Jun 2023 16:48:09 +0300 Subject: [PATCH 1/4] fix: Stricter with_plugins --- ckan/tests/pytest_ckan/ckan_setup.py | 6 ++- ckan/tests/pytest_ckan/fixtures.py | 54 ++++++++++++++++++++----- ckan/tests/pytest_ckan/test_fixtures.py | 19 +++++++-- 3 files changed, 63 insertions(+), 16 deletions(-) diff --git a/ckan/tests/pytest_ckan/ckan_setup.py b/ckan/tests/pytest_ckan/ckan_setup.py index 2f2af13bddb..073e1908fb9 100644 --- a/ckan/tests/pytest_ckan/ckan_setup.py +++ b/ckan/tests/pytest_ckan/ckan_setup.py @@ -54,9 +54,11 @@ def pytest_runtestloop(session): """When all the tests collected, extra plugin may be enabled because python interpreter visits their files. - Make sure only configured plugins are active when test loop starts. + Make sure all plugins are disabled. If test requires a plugin, it must rely + on `with_plugins` fixture. + """ - plugins.load_all() + plugins.unload_all() def pytest_runtest_setup(item): diff --git a/ckan/tests/pytest_ckan/fixtures.py b/ckan/tests/pytest_ckan/fixtures.py index f7edd1a92be..9fa4e500e03 100644 --- a/ckan/tests/pytest_ckan/fixtures.py +++ b/ckan/tests/pytest_ckan/fixtures.py @@ -268,23 +268,57 @@ def clean_index(reset_index): @pytest.fixture def with_plugins(ckan_config): - """Load all plugins specified by the ``ckan.plugins`` config option - at the beginning of the test. When the test ends (even it fails), it will - unload all the plugins in the reverse order. + """Load all plugins specified by the ``ckan.plugins`` config option at the + beginning of the test(and disable any plugin which is not listed inside + ``ckan.plugins``). When the test ends (including fail), it will unload all + the plugins. .. literalinclude:: /../ckan/tests/test_factories.py :start-after: # START-CONFIG-OVERRIDE :end-before: # END-CONFIG-OVERRIDE + Use this fixture if test relies on CKAN plugin infrastructure. For example, + if test calls an action or helper registered by plugin XXX:: + + @pytest.mark.ckan_config("ckan.plugins", "XXX") + @pytest.mark.usefixtures("with_plugin") + def test_action_and_helper(): + assert call_action("xxx_action") + assert tk.h.xxx_helper() + + It will not work without ``with_plugins``. If ``XXX`` plugin is not loaded, + ``xxx_action`` and ``xxx_helper`` do not exist in CKAN registries. + + But if the test above use direct imports instead, ``with_plugins`` is + optional:: + + def test_action_and_helper(): + from ckanext.xxx.logic.action import xxx_action + from ckanext.xxx.helpers import xxx_helper + + assert xxx_action() + assert xxx_helper() + + Keep in mind, that generally it's a bad idea to import helpers and actions + directly. If **every** test of extension requires standard set of plugins, + specify these plugins inside test config file(``test.ini``):: + + ckan.plugins = essential_plugin another_plugin_required_by_every_test + + And create an autouse-fixture that depends on ``with_plugins`` inside + the main ``conftest.py`` (``ckanext/ext/tests/conftest.py``):: + + @pytest.fixture(autouse=True) + def load_standard_plugins(with_plugins): + ... + + This will automatically enable ``with_plugins`` for every test, even if + it's not required explicitely. + """ - plugins = aslist(ckan_config["ckan.plugins"]) - for plugin in plugins: - if not ckan.plugins.plugin_loaded(plugin): - ckan.plugins.load(plugin) + ckan.plugins.load_all() yield - for plugin in reversed(plugins): - if ckan.plugins.plugin_loaded(plugin): - ckan.plugins.unload(plugin) + ckan.plugins.unload_all() @pytest.fixture diff --git a/ckan/tests/pytest_ckan/test_fixtures.py b/ckan/tests/pytest_ckan/test_fixtures.py index df6f4d495c9..0f806d3c508 100644 --- a/ckan/tests/pytest_ckan/test_fixtures.py +++ b/ckan/tests/pytest_ckan/test_fixtures.py @@ -33,10 +33,21 @@ def test_ckan_config_mark_without_explicit_config_fixture(): assert config[u"some.new.config"] == u"exists" -@pytest.mark.ckan_config(u"ckan.plugins", u"stats") -@pytest.mark.usefixtures(u"with_plugins") -def test_with_plugins_is_able_to_run_with_stats(): - assert plugins.plugin_loaded(u"stats") +class TestWithPlugins: + @pytest.fixture() + def load_example_helpers(self): + plugins.unload_all() + plugins.load("example_itemplatehelpers") + + @pytest.mark.ckan_config(u"ckan.plugins", u"stats") + @pytest.mark.usefixtures(u"with_plugins") + def test_with_plugins_is_able_to_run_with_stats(self): + assert plugins.plugin_loaded(u"stats") + + def test_with_plugins_unloads_enabled_plugins( + self, load_example_helpers, with_plugins + ): + assert "example_helper" not in plugins.toolkit.h @pytest.mark.ckan_config("ckan.site_url", "https://example.org") From b82536fc72037c126db7e6940ccaa2567cfd8e00 Mon Sep 17 00:00:00 2001 From: Sergey Motornyuk Date: Sat, 10 Jun 2023 17:24:51 +0300 Subject: [PATCH 2/4] chore: add changelog entry --- changes/7638.fix | 2 ++ ckan/tests/pytest_ckan/fixtures.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changes/7638.fix diff --git a/changes/7638.fix b/changes/7638.fix new file mode 100644 index 00000000000..3488ecd848c --- /dev/null +++ b/changes/7638.fix @@ -0,0 +1,2 @@ +Plugins randomly change their order during test session and somethimes they +work even without ``with_plugins`` fixture. diff --git a/ckan/tests/pytest_ckan/fixtures.py b/ckan/tests/pytest_ckan/fixtures.py index 9fa4e500e03..aa68d308c1a 100644 --- a/ckan/tests/pytest_ckan/fixtures.py +++ b/ckan/tests/pytest_ckan/fixtures.py @@ -46,7 +46,7 @@ import ckan.cli import ckan.lib.search as search import ckan.model as model -from ckan.common import config, aslist +from ckan.common import config @register From 053b10c9c6276010f6e61d09f788afdc19c4e483 Mon Sep 17 00:00:00 2001 From: Sergey Motornyuk Date: Sat, 10 Jun 2023 18:13:11 +0300 Subject: [PATCH 3/4] Keep system plugins enabled --- changes/{7638.fix => 7638.bugfix} | 0 ckan/tests/pytest_ckan/ckan_setup.py | 30 +++++++++++++++++++++++++--- ckan/tests/pytest_ckan/fixtures.py | 3 ++- 3 files changed, 29 insertions(+), 4 deletions(-) rename changes/{7638.fix => 7638.bugfix} (100%) diff --git a/changes/7638.fix b/changes/7638.bugfix similarity index 100% rename from changes/7638.fix rename to changes/7638.bugfix diff --git a/ckan/tests/pytest_ckan/ckan_setup.py b/ckan/tests/pytest_ckan/ckan_setup.py index 073e1908fb9..6b70c355746 100644 --- a/ckan/tests/pytest_ckan/ckan_setup.py +++ b/ckan/tests/pytest_ckan/ckan_setup.py @@ -54,11 +54,35 @@ def pytest_runtestloop(session): """When all the tests collected, extra plugin may be enabled because python interpreter visits their files. - Make sure all plugins are disabled. If test requires a plugin, it must rely - on `with_plugins` fixture. + Make sure all normal plugins are disabled. If test requires a plugin, it + must rely on `with_plugins` fixture. + + We keep system plugins enabled in order to keep auto-indexing of datasets + available without `with_plugins` fixture. """ - plugins.unload_all() + unload_non_system_plugins() + + +def unload_non_system_plugins(): + """Unload all plugins except for system plugins. + + System plugins must remain available because they provide essential CKAN + functionality. + + At the moment we have only one system plugin - synchronous_search - which + automatically sends all datasets to Solr after modifications. Without it + you have to indexed datasets manually after any `package_*` action. + + """ + from ckan.plugins.core import _PLUGINS, unload, find_system_plugins + + system_plugins = find_system_plugins() + plugins_to_unload = [ + p for p in reversed(_PLUGINS) + if p not in system_plugins + ] + unload(*plugins_to_unload) def pytest_runtest_setup(item): diff --git a/ckan/tests/pytest_ckan/fixtures.py b/ckan/tests/pytest_ckan/fixtures.py index aa68d308c1a..8f159c8ac75 100644 --- a/ckan/tests/pytest_ckan/fixtures.py +++ b/ckan/tests/pytest_ckan/fixtures.py @@ -47,6 +47,7 @@ import ckan.lib.search as search import ckan.model as model from ckan.common import config +from .ckan_setup import unload_non_system_plugins @register @@ -318,7 +319,7 @@ def load_standard_plugins(with_plugins): """ ckan.plugins.load_all() yield - ckan.plugins.unload_all() + unload_non_system_plugins() @pytest.fixture From b2cc827d386e0b3b6830b74770d1b67f41a7fb08 Mon Sep 17 00:00:00 2001 From: Sergey Motornyuk Date: Sat, 10 Jun 2023 18:15:53 +0300 Subject: [PATCH 4/4] move unload_non_system_plugins to plugins.core --- ckan/plugins/core.py | 20 ++++++++++++++++++++ ckan/tests/pytest_ckan/ckan_setup.py | 23 +---------------------- ckan/tests/pytest_ckan/fixtures.py | 3 +-- 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/ckan/plugins/core.py b/ckan/plugins/core.py index 66f170837ab..6540db8d09b 100644 --- a/ckan/plugins/core.py +++ b/ckan/plugins/core.py @@ -31,6 +31,7 @@ 'load', 'load_all', 'unload', 'unload_all', 'get_plugin', 'plugins_update', 'use_plugin', 'plugin_loaded', + 'unload_non_system_plugins', ] TInterface = TypeVar('TInterface', bound=interfaces.Interface) @@ -329,6 +330,25 @@ def find_system_plugins() -> list[str]: return eps +def unload_non_system_plugins(): + """Unload all plugins except for system plugins. + + System plugins must remain available because they provide essential CKAN + functionality. + + At the moment we have only one system plugin - synchronous_search - which + automatically sends all datasets to Solr after modifications. Without it + you have to indexed datasets manually after any `package_*` action. + + """ + system_plugins = find_system_plugins() + plugins_to_unload = [ + p for p in reversed(_PLUGINS) + if p not in system_plugins + ] + unload(*plugins_to_unload) + + def _get_service(plugin_name: Union[str, Any]) -> SingletonPlugin: ''' Return a service (ie an instance of a plugin class). diff --git a/ckan/tests/pytest_ckan/ckan_setup.py b/ckan/tests/pytest_ckan/ckan_setup.py index 6b70c355746..2617848860c 100644 --- a/ckan/tests/pytest_ckan/ckan_setup.py +++ b/ckan/tests/pytest_ckan/ckan_setup.py @@ -61,28 +61,7 @@ def pytest_runtestloop(session): available without `with_plugins` fixture. """ - unload_non_system_plugins() - - -def unload_non_system_plugins(): - """Unload all plugins except for system plugins. - - System plugins must remain available because they provide essential CKAN - functionality. - - At the moment we have only one system plugin - synchronous_search - which - automatically sends all datasets to Solr after modifications. Without it - you have to indexed datasets manually after any `package_*` action. - - """ - from ckan.plugins.core import _PLUGINS, unload, find_system_plugins - - system_plugins = find_system_plugins() - plugins_to_unload = [ - p for p in reversed(_PLUGINS) - if p not in system_plugins - ] - unload(*plugins_to_unload) + plugins.unload_non_system_plugins() def pytest_runtest_setup(item): diff --git a/ckan/tests/pytest_ckan/fixtures.py b/ckan/tests/pytest_ckan/fixtures.py index 8f159c8ac75..619fdd77839 100644 --- a/ckan/tests/pytest_ckan/fixtures.py +++ b/ckan/tests/pytest_ckan/fixtures.py @@ -47,7 +47,6 @@ import ckan.lib.search as search import ckan.model as model from ckan.common import config -from .ckan_setup import unload_non_system_plugins @register @@ -319,7 +318,7 @@ def load_standard_plugins(with_plugins): """ ckan.plugins.load_all() yield - unload_non_system_plugins() + ckan.plugins.unload_non_system_plugins() @pytest.fixture