Update to Django 5.1

[StevenMaude]

Fixes #2115.

This requires a workaround of the coding systems tests that use databases dynamically, due to changes in Django 5.1. We agreed in #2115 that working around the new restrictions was the best thing to do, to allow the upgrade without delay. In future, we may have to revisit this code and associated tests, either for future Django changes or to make maintenance and improvements easier.

TODO:

• local testing of the site
• repin Django to 5.1.x not doing this
• upgrade this branch to Django 5.1.6
• recheck with django-upgrade before merge (maybe there are newer migrations?)
• open an issue to consider refactoring the BNF tests that use a pytest fixture instead of data stored as a file (Consider refactoring BNF coding systems import data to be like other tests #2335)

[StevenMaude]

The issue here is with coding systems test failures, due to the way the tests use databases.

[StevenMaude]

Legacy comment:

Current status of this:

* the working tests do pass, but are a bit of a mess still, so this PR does look somewhat horrendous right now, until we can get everything working, and then refine it
* the use of pytest fixtures, that those fixtures aren't easily available on classes when you're using unittest.TestCase and, for the base coding system tests, the time at which those fixtures are created, all makes this more complicated
* it's probably possible to move the general modified TestCase class out of each test module into a higher level, in most cases, and remove that duplication
* the import_data_path on the TestCase isn't always needed
* there's a bit of reviewing which pytest fixtures are being used (maybe the settings fixture is needed in places)
* from what I've looked at of the failing base tests, there's some other magic going on with fixtures (bnf_release and maybe others) that still means that the specified db_alias doesn't get picked up by the test, and we get the same DatabaseOperationForbidden error

[mikerkelly]

Good progress so far, Steve. I think this is pretty close now! Quick comments, happy to discuss anything that's helpful...

fixtures aren't easily available on classes when you're using unittest.TestCase

@pytest.mark.usefixtures may help here? I think that's the intended approach for using fixtures in unittest/Django-style class-based test cases. Instead of the autouse=True fixtures in the draft base / test_import_data_utils.py commit.

• it's probably possible to move the general modified TestCase class out of each test module into a higher level, in most cases, and remove that duplication

Would be very good to do this before we move on, as it's easiest to do now and will be useful if this is modified or extended at all in future. Could be a separate PR or even Issue if the base tests can be fixed up.

• the import_data_path on the TestCase isn't always needed
• there's a bit of reviewing which pytest fixtures are being used (maybe the settings fixture is needed in places)

Similar.

• from what I've looked at of the failing base tests, there's some other magic going on with fixtures (bnf_release and maybe others) that still means that the specified db_alias doesn't get picked up by the test, and we get the same DatabaseOperationForbidden error

Could this issue be different-but-related to the issue in the others modules as this test case doesn't seem to be about dynamically creating a database? The exception stacktrace is related to the fixture creation, which does seem to do something dynamically with test case config, in setup_db. I think fixture code is called before the test setUp -- try putting a breakpoint in the setUp, it doesn't get reached. So the test cls.databases hasn't been updated by setUp by the time setup_db runs. So it's the same issue as before and again we don't have access to the test case class from inside setup_db. I guess we need to find a different way to access it, or, more likely, redesign the fixtures approach for this test module a bit. Maybe the work done in setup_db can be a function called in the setUp rather than a fixture, if that solves the sequencing issue?

[StevenMaude]

NB: the tests ostensibly pass as of 37d1546, but that's because several are commented out, while I got one single coding system base test working.

[StevenMaude]

There's one test left to fix, then I'll start thinking about how the debris can be tidied up.

Everything passes; time to clean up the mess.

[StevenMaude]

I'm now tidying this up on another branch and when done will push back to this PR branch. Now pushing back to this branch.

The stages there:

• factor out the main TestCase class to override the database configuration we've created from the bnf tests to shared code
• possibly subclass that new TestCase class for the base tests
• use plain pytest-style assert instead of self.assert…
• somehow figure out a way of generalising for CTV3 tests, or for the BNF such that the import data is introduced in the same way; probably better to just package the BNF data?
• allow for multiple db_alias values
• think about some better way of wrangling the database settings, to allow for multiple updates
• review use of fixtures; can we use mark.usefixtures and reduce autouse? do we even need fixtures inside the class?
• check handling of optional attributes like import_data_path and import_data_fixture
• check which tests actually need the database_tmp_dir, and can we make it optional?
• refactor the base tests if possible
• should we bother generalising db_alias to accommodate the one case where multiple additional databases are used? (maybe not, it feels like a special case right now)
• do we set the use of coding_systems_database_tmp_dir as optional, if possible
• can we move out the expected_db_path from setUp?

[StevenMaude]

Moving the messy branch with all the stages of progress to steve/django-5.1-fixup and the cleaned branch back here.

[StevenMaude]

Local testing flagged up that logout didn't work in Django 5.1 (until I fixed it). That's another good case for "we should have automated end-to-end tests", as in #2135.

[StevenMaude]

Thanks to Mike and Jon for the helpful review. Based on their comments, I've done the following:

• added another subclass of the DynamicDatabaseTestCase to cover the cases using the temporary path
• moved some code comments into docstrings, and expanded on docstrings/comments
• renamed some of the methods/attributes to be clearer
• moved out the two staticmethod methods back to module-level
• labelled some module/class-only functions/methods with a single leading underscore to indicate they are internal use
• renamed the module containing the class from helpers.py to dynamic_db_classes.py

I've also:

• upgraded again to Django 5.1.6 which was released 2025-02-06, and contains a couple of bug fixes
• ran django-upgrade again, with no changes

The new commits are those following 4b400217002487253eff2366fd5b75ba8f9c522b. Earlier commit IDs have changed because I've rebased on newer main a couple of times.

I left these post-review fix commits deliberately separate, and small for easier review.

When the review is approved, I could squash these newer commits back to where the relevant code is changed in each case, to keep the history cleaner.

The previous commits reviewed typically made all the changes to a single file in a single commit (which makes logical sense as each Python module covers one aspect of the tests). That might also make sense once we've finalised these changes.

[mikerkelly]

Thanks Steve. I think everything looks good now (one minor docs comment). Phew, good work! I think this is deployable, so approving.

It's up to you whether to rebase further or not.

[StevenMaude]

I've squashed the subsequent post-review commits back in to where they would have gone, if I'd got it looking good the first time.