Opened 13 years ago
Closed 13 years ago
#17928 closed Uncategorized (wontfix)
Tests with import time dependency on DB fail
Reported by: | Malcolm Box | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If a test has an import time dependency on a database table, the test fails as at the point the test suite is created, the test databases have not been
This is caused by the following around line 357 of test/simple.py:
self.setup_test_environment() suite = self.build_suite(test_labels, extra_tests) old_config = self.setup_databases() result = self.run_suite(suite) self.teardown_databases(old_config)
build_suite() is called before the databases are set up. There doesn't seem to be any dependency on the suite in setup_databases(), so this can be fixed by moving the setup_databases() call before the build_suite()
Change History (5)
follow-up: 2 comment:1 by , 13 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:2 by , 13 years ago
Resolution: | needsinfo |
---|---|
Status: | closed → reopened |
Replying to carljm:
Do you have a legitimate use case for an import-time database query? In non-test code, those are almost always bugs, as they result in data that never gets updated until a process restart; so it's a good thing if they fail under test.
I believe I do have a valid use case. The code under test uses it to read the domain name info from the Sites table (DOMAIN_NAME = Site.objects.get_current().domain). I know that the domain is fixed, and since my site is scaling to 10K+ requests/s I can't afford a DB query on that path.
Thus the underlying use case is "read something from the database that you know doesn't change often, where you can't afford the DB lookup in line". The alternatives would be to cache this (hassle, more code), or to initialise the variable later (all sorts of interesting race conditions).
While it might be true that doing this is sometimes a cause of bugs, those should be caught by the user's test code explicitly checking it can change the values and have things work, not by a blanket ban.
comment:3 by , 13 years ago
IMO the best way to handle this use case is to load the value lazily and to cache it.
SQL queries at import time always come back and bite you (at least in my experience).
comment:4 by , 13 years ago
Easy pickings: | unset |
---|
Also note that switching the two lines as suggested in the summary would be backwards incompatible for people who have customized the test runner.
comment:5 by , 13 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
Marking wontfix, as this is a backwards-incompatible change without sufficient justification; it doesn't permit anything that isn't better handled with lazy-loading/caching, and it opens the door for significant bugs to go undetected. Import-time database queries are not supported. (Note also that it's not just the test-runner; putting an import-time database query in the wrong place in your codebase can easily cause circular import troubles).
Do you have a legitimate use case for an import-time database query? In non-test code, those are almost always bugs, as they result in data that never gets updated until a process restart; so it's a good thing if they fail under test.
Marking needsinfo, as without more detail I don't think this is a valid use case that should be supported. If you feel you have a strong use case that can't easily be handled any other way, reopen. My inclination is to close this wontfix.