ckan / ckanext-scheming

Easy, shareable custom CKAN schemas
Other
86 stars 163 forks source link

Can not modify presets across tests #424

Open amercader opened 1 month ago

amercader commented 1 month ago

Looks like the first time presets are loaded they are set for the rest of the session. This causes problems in tests that load different sets of presets, e.g.

# test_something.py

@pytest.mark.usefixtures("with_plugins", "clean_db")
@pytest.mark.ckan_config("ckan.plugins", "dcat scheming_datasets")
@pytest.mark.ckan_config(
    "scheming.dataset_schemas", "ckanext.dcat.schemas:dcat_ap.yaml"
)
@pytest.mark.ckan_config(
    "scheming.presets",
    "ckanext.scheming:presets.json"
)
class TestScheming():
    def test_something(self):
         pass

# test_other.py
@pytest.mark.usefixtures("with_plugins", "clean_db")
@pytest.mark.ckan_config("ckan.plugins", "dcat scheming_datasets fluent")
@pytest.mark.ckan_config(
    "scheming.dataset_schemas", "ckanext.dcat.schemas:dcat_ap_multilingual.yaml"
)
@pytest.mark.ckan_config(
    "scheming.presets",
    "ckanext.scheming:presets.json ckanext.fluent:presets.json"
)

class TestSchemingFluent():
    def test_other(self):
         pass

When run individually, test_other.py will run fine, but if test_something.py has been run before it will fail with

E               ckanext.scheming.errors.SchemingException: preset 'fluent_core_translated' not defined                                     

../ckanext-scheming/ckanext/scheming/plugins.py:651: SchemingException       

Looks like these lines prevent further re-loading of the presets on each update_config() call:

https://github.com/ckan/ckanext-scheming/blob/27035f4d5b3722c2bc64d39b6c2b1d76c9883636/ckanext/scheming/plugins.py#L106-L107

Is this just for performance reasons @wardi ? or to avoid conflicts?

wardi commented 1 month ago

This is to prevent repeated parsing for performance reasons but you're right it doesn't get cleared out when the configuration is reloaded.

What's the best fix, should the presets be cached in the configuration object instead?

amercader commented 1 month ago

should the presets be cached in the configuration object instead?

But then they will have to be refreshed on each update_config() call anyway right? Wouldn't that be the same that removing the caching?

wardi commented 1 month ago

Oh, I didn't notice that _load_presets is only called from update_config. Let's simply remove those two lines.