Skip to content

Don't persist fixtures in test database between tests #2595

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mikerkelly
Copy link
Contributor

@mikerkelly mikerkelly commented May 28, 2025

Addresses #2523.

Replaces the process-level universe fixture that puts instances in the database that persist through the session and builds fixtures to retrieve copies of them with standard pytest-style fixture functions. This is work-in-progress and there's still quite a lot to do but I'd like some feedback on the approach.

The commits could made clearer by separating out moving things around within fixtures.py from other changes and so on. But I haven't spent the time to do that yet. There's still likely to be a Big Commit that changes the approach and fixes the tests. I don't currently think it's worth the effort of adding the new fixtures one at a time in separate commits, and those intermediate steps with a hybrid approach would look a bit confusing.

This means the universe of instances isn't always the same. Fixtures declare which other fixtures they depend upon. Several tests have been modified to explicitly declare things they depend on. I've provided fixtures for "instantiate lots of codelists and versions" where there are tests that want that such as the API tests. There were comments describing things like "this codelist has 4 versions" -- I haven't updated these yet to something more accurate. I also noted those weren't always up-to-date before these changes.

Coding system releases instances are still loaded into the core database once per session (setup_coding_systems). I think this can be improved by having a function-level fixture that loads the coding system release for a specific coding system and depends on the session-level fixtures that load data to the secondary coding system databases.

There are still a handful of test failures I need to resolve.

  • codelists/tests/test_api.py has a test that gets a representation of many different codelists and versions including version hashes which are tested for and have changed slightly -- need to work out why they change and whether this can be fixed, the hashes updated, or change the test approach slightly.
  • coding_systems/base/tests/test_import_data_utils.py -- possibly dependencies again, was a bit harder to reason about as multiple databases changing.
  • opencodelists/tests/integration/test_sanitise_backup.py -- need to tweak how these get the fixtures and iterate through all the instances.
  • opencodelists/tests/management/commands/test_setup_local_dev_databases.py -- see next point.

Making this change has knock-on implications I haven't resolved/investigated yet:

  • setup_local_dev_databases management command used the fixture building codes to populate some development data -- I think this can just import and call the individual functions in an appropriate order, respecting the dependencies.
  • I need to look at the implications on the Playwright fixtures.

I've changed the password hasher in tests to a faster less secure one as we now create users frequently (conftest.py).

The tests currently run a few second faster than before but probably due to the test errors and failures above. But I'm hopeful this approach is approximately as fast as before. We could look at multi-worker pytest-xdist.

We could possibly benefit by having FactoryBoy-style factories for different objects. create_codelists is a factory fixture, for example.

I've removed some asserts that didn't make sense in isolation and that we don't want to run every time the fixture is called (previously they would only have been called once). We could add these back in as tests of the fixtures.

There were two tests where the use of codes fixtures seemed to be the wrong way around between them, I haven't fully understood these yet:

  • codelists/tests/test_actions.py::test_create_or_update_codelist_update
  • codelists/tests/test_actions.py::test_create_or_update_codelist_update_no_change_to_codes - assert

@mikerkelly mikerkelly force-pushed the mikerkelly/spike-fixtures-db branch from 00fffa4 to 84522de Compare May 28, 2025 08:28
@mikerkelly mikerkelly changed the title Mikerkelly/spike fixtures db Don't persist fixtures in test database between tests May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant