Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#19167 closed Uncategorized (fixed)

When running tests, Django accesses the database referred to in settings

Reported by: Daniele Procida Owned by: nobody
Component: Documentation Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

https://docs.djangoproject.com/en/dev/topics/testing/#the-test-database says:

Tests that require a database (namely, model tests) will not use your "real" (production) database. Separate, blank databases are created for the tests.

There seem to be circumstances in which Django will access the real database that is referred to in settings.

Say I have a model called Entity.

I find I can't directly access the real database's Entity objects in tests, and in most of the application code, as expected, Django doesn't find anything in Entity.objects.all(). This is what the documentation tells me to expect.

However, in other parts of the code, Entity.objects.all() does return the Entity objects from the real database, which surprised me. The real database is accessed before Django gets to "Creating test database for alias 'default'..."

If the real database is removed, then things behave as I would expect.

If it is to be expected that Django code when running tests will sometimes access the real database, then the documents need to explain when or how this will happen - but I don't think it should happen.

Change History (15)

comment:1 by Aymeric Augustin, 12 years ago

If the real database is accessed before creating the test database, it means that your code runs SQL queries (most likely via ORM callls) at compilation time. Don't do that.

in reply to:  1 comment:2 by Daniele Procida, 12 years ago

So for example trying to access objects from the database in a module (and not inside a class or function)? Because that's what in fact I do...

comment:3 by Anssi Kääriäinen, 12 years ago

Depends on when the module is imported. Inside class has the same problem. Most likely this would result in accessing the production DB. As aaugustin above said the fix is: don't do this.

There would be certain point in ensuring no queries are even possible through django.db.connections until the test database is set up. The problem here is that to set up the test databases we have to run some queries (CREATE DATABASE for example). We could likely do this by making a local copy of the connections dict first thing in the test runner, then clearing the global connections dictionary. When the test databases are set up, we repopulate the global connections dictionary again.

The question is if we actually want this, and if so, what will break.

comment:4 by Daniele Procida, 12 years ago

Maybe I should find a better way to do what I am trying to do, then, that avoids this (if it is even possible, I don't know).

However, I wasn't aware that accessing the DB when modules are imported is a bad idea in general, or that it could interfere with tests.

Is it worth making a note of this on the test documentation, or anywhere else, if it's not already made clear that it's something one should not do?

comment:5 by Aymeric Augustin, 12 years ago

The easiest way to work around this problem is cached lazy loading:

_things = None
def get_things():
    if _things is None:
        _things = Things.objects.all()
    return _things

I presume that syncdb fails on your current code, because it will attempt to load Entity objects before the corresponding table is created in the database. This is the most obvious reason why I said "don't do that".


Right now I don't know how Django could forbid this. It's a fairly common pitfall.

Version 0, edited 12 years ago by Aymeric Augustin (next)

comment:6 by Anssi Kääriäinen, 12 years ago

I am not sure if this is documented or not. If not a docs note would be welcome (likely recommending something like what was done in comment:5).

I am pretty sure we could prevent this in tests using the idea in comment:3. Assuming users actually write tests, then import time queries would be spotted...

Another option is to have prevent_normal_queries() and enable_normal_queries() available. These could be called in various places, for example app-loading, test setup, syncdb etc. Then we would still need some way to bypass this prevention. If prevent_normal_queries just appends __hidden__ to each alias in connections, then we could still run queries by .using('__hidden__' + alias) when needed.

I am not at all sure this is worth all the trouble... And, I am pretty sure some user code would break.

comment:7 by Daniele Procida, 12 years ago

I will write a couple of notes for the documentation then, warning that real database items may leak into tests if the code accesses the database at compilation time.

I have not seen advice not to do that in the documentation previously, but I am not even sure where it might say such a thing or what terms it would use.

I'll also add aaugustin's workaround in comment:5.

Thanks for your feedback.

comment:8 by Preston Holmes, 12 years ago

Component: UncategorizedDocumentation
Triage Stage: UnreviewedAccepted

A suggested location for the doc improvements would be:

https://docs.djangoproject.com/en/dev/topics/testing/#the-test-database

comment:9 by Anssi Kääriäinen, 12 years ago

Here is a proof-of-concept for disabling queries during import time: https://github.com/akaariai/django/commit/1b99fcee4c0b3782d3136744cfaaccfea1d535e0 - I quickly tested this on different databases using a test project which does import time "SomeModel.objects.get(pk=1)" query. Seemed to work. And, Django's test suite didn't complain on sqlite either.

IMO we should do the disabling of queries. This will only hurt users who do import time queries when running tests. They will benefit from fixing their code. If this change isn't possible for them, they can just do _enable_connections().

The implementation only ensures that the main thread can't run queries (django.db.connections is thread-local). I wonder if there is any point in making sure all threads are prevented from running queries...

I don't see any easy way to test this.

comment:10 by Daniele Procida, 12 years ago

https://github.com/evildmp/django/commit/f0b5d1b91698452a468fd6a6af63c5b7a7d95405 for the documentation; once I work out how to make a pull request containing this and not two other files that I came from I don't-know-where, I will do that.

comment:11 by Daniele Procida, 12 years ago

I've made a pull request for the documentation: https://github.com/django/django/pull/463

comment:12 by Daniele Procida, 12 years ago

Please ignore the previous pull request, here's another attempt: https://github.com/django/django/pull/486

comment:13 by Tim Graham, 12 years ago

Has patch: set

comment:14 by Tim Graham <timograham@…>, 12 years ago

Resolution: fixed
Status: newclosed

In 07361d1fd6b4531e422e2593c91b47bc6bf88993:

Fixed #19167 - Added a warning regarding module-level database queries

Thanks Daniele Procida for the patch.

comment:15 by Tim Graham <timograham@…>, 12 years ago

In 90af863410cae9d043f5378a7cfdab92a696b562:

[1.5.X] Fixed #19167 - Added a warning regarding module-level database queries

Thanks Daniele Procida for the patch.

Backport of 07361d1fd6 from master

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