Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Can not modify presets across tests #424

Open
amercader opened this issue Oct 24, 2024 · 3 comments · May be fixed by #425
Open

Can not modify presets across tests #424

amercader opened this issue Oct 24, 2024 · 3 comments · May be fixed by #425

Comments

@amercader
Copy link
Member

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:

ckanext-scheming/ckanext/scheming/plugins.py

Lines 106 to 107 in 27035f4

if _SchemingMixin._presets is not None:
return

Is this just for performance reasons @wardi ? or to avoid conflicts?
amercader added a commit to ckan/ckanext-dcat that referenced this issue Oct 24, 2024
@amercader
Use single scheming.presets config
5f091a2 
Because of ckan/ckanext-scheming#424
@wardi
Copy link
Contributor

wardi commented Oct 24, 2024

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
Copy link
Member Author

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
Copy link
Contributor

wardi commented Oct 25, 2024

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

amercader added a commit that referenced this issue Oct 28, 2024
@amercader
Remove presets parsing cache
2986314 
As it creates issues when loading plugins (e.g. in tests)
See #424

Fixes #424
@amercader amercader linked a pull request Oct 28, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Remove presets parsing cache ckan/ckanext-scheming
2 participants
@wardi @amercader