Opened 7 years ago
Closed 6 years ago
#29513 closed Cleanup/optimization (fixed)
Make TransactionTestCase.multi_db documentation more discoverable.
| Reported by: | Adam Johnson | Owned by: | nobody |
|---|---|---|---|
| Component: | Documentation | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
I had a pull request that took me two years to resolve (okay, I deferred it quite a lot): https://github.com/adamchainz/django-mysql/pull/278 . This was adding pytest-randomly to my project, which randomly sorts the tests and exposes test interdependence. It turned out the failure was due to interdependence between tests that touched a secondary database, with the TestCase classes not resetting the secondary database, because multi_db=True was missing from such TestCases to get the secondary DB wiped between tests.
Part of the trouble debugging this was that multi_db is not easy to find:
- The page on multiple databases doesn't mention or link to testing considerations at all: https://docs.djangoproject.com/en/2.0/topics/db/multi-db/
- The 'test database' section of 'writing and running tests' *does* mention and link to 'advanced multi-db testing topics' : https://docs.djangoproject.com/en/2.0/topics/testing/overview/#the-test-database -> https://docs.djangoproject.com/en/2.0/topics/testing/advanced/#topics-testing-advanced-multidb . However the linked section, "Tests and multiple databases", doesn't mention
multi_db- it's only in the "Advanced features of TransactionTestCase" section - and I'd argue it's not an advanced feature but a basic one, even a risk that you'll end up taking 2 years to debug an issue :) - When
multi_dbis mentioned, it's only in the context ofTransactionTestCase.TestCaseisn't mentioned, which yes does inherit fromTransactionTestCase, but it also has more behaviour, in the scope of class-levelatomic()s that also obey themulti_dbflag
I think we should fix the docs for these problems 🎉.
Secondarily, the feature is confusing and I'd flip the default of the flag to True and allow setting it to False as an optimization - but this is a much bigger change.
Change History (6)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
| Component: | Testing framework → Documentation |
|---|---|
| Summary: | Improve testing multi_db documentation → Make TransactionTestCase.multi_db documentation for discoverable |
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 7 years ago
| Summary: | Make TransactionTestCase.multi_db documentation for discoverable → Make TransactionTestCase.multi_db documentation more discoverable |
|---|
comment:6 by , 6 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
| Summary: | Make TransactionTestCase.multi_db documentation more discoverable → Make TransactionTestCase.multi_db documentation more discoverable. |
We can treat this ticket as fixed, since multi_db will be removed in Django 3.1 (see b61ea56789a5825bd2961a335cb82f65e09f1614).
#28478 is related.
It suggests merging
multi_dbandallow_database_queriesflags into a singledatabasesaliases set that would be unionized to determine which databases needs to be created based on the tests retrieved during discovery.When a query to an unspecified database would be performed an error similar to the one raised when performing queries in a
SimpleTestCaseright now would be raised.SimpleTestCase.databaseswould be an empty set whileTestCase.databases = {DEFAULT_DB_ALIAS}.multi_db = Truecould be alias fordatabases = {alias for alias in connections}during the deprecation period. We could also support a specialdatabases = '__all__'value to replicate the actualmulti_dbbehavior.Here's the PoC.