Opened 12 years ago

Closed 12 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)

comment:1 by Carl Meyer, 12 years ago

Resolution: needsinfo
Status: newclosed

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.

in reply to:  1 comment:2 by Malcolm Box, 12 years ago

Resolution: needsinfo
Status: closedreopened

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 Aymeric Augustin, 12 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 Aymeric Augustin, 12 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.

Last edited 12 years ago by Aymeric Augustin (previous) (diff)

comment:5 by Carl Meyer, 12 years ago

Resolution: wontfix
Status: reopenedclosed

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).

Note: See TracTickets for help on using tickets.
Back to Top