Merge pull request #7638 from ckan/strict-with-plugins

Stricter with_plugins

changes/7638.bugfix

@@ -0,0 +1,2 @@
+Plugins randomly change their order during test session and somethimes they
+work even without ``with_plugins`` fixture.

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)
@@ -327,6 +328,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).

ckan/tests/pytest_ckan/ckan_setup.py

@@ -53,9 +53,14 @@ 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 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.load_all()
+    plugins.unload_non_system_plugins()
 
 
 def pytest_runtest_setup(item):

ckan/tests/pytest_ckan/fixtures.py

@@ -45,7 +45,7 @@
 import ckan.plugins
 import ckan.cli
 import ckan.model as model
-from ckan.common import config, aslist
+from ckan.common import config
 from ckan.lib import redis, search
 
 
@@ -337,23 +337,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_non_system_plugins()
 
 
 @pytest.fixture

ckan/tests/pytest_ckan/test_fixtures.py

@@ -35,10 +35,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")