Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#18575 closed Bug (fixed)

Testing framework doesn't work without a SQL Database

Reported by: delormemarco@… Owned by: Aymeric Augustin
Component: Testing framework Version: 1.4
Severity: Release blocker Keywords: regression
Cc: jan.bednarik@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If you choose not to use a database in your settings.py (DATABASE = {}), you can't run tests with

python manage.py test <my app>

You receive an ImproperlyConfigured exception with the following message: you must define a 'default' database

That's because django.test.signals import django.db.connection since 1.4 and obviously django.db.connectinon need a database to be configured

Attachments (3)

18575-1.diff (1.9 KB ) - added by Claude Paroz 12 years ago.
Late import of django.db.connections
18575-2.diff (1.1 KB ) - added by Claude Paroz 12 years ago.
Removed kwargs changes from the previous patch
18575-3.diff (1.6 KB ) - added by Claude Paroz 12 years ago.
Allow setting DATABASES to empty dict (will default to dummy engine)

Download all attachments as: .zip

Change History (22)

comment:1 by Claude Paroz, 12 years ago

Keywords: regression added
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

by Claude Paroz, 12 years ago

Attachment: 18575-1.diff added

Late import of django.db.connections

comment:2 by Claude Paroz, 12 years ago

Has patch: set

comment:3 by Aymeric Augustin, 12 years ago

The rewrite of update_connections_time_zone isn't correct; signal receivers must accept **kwargs for forwards-compatibilty.

I recommend restricting the patch to the changes required to fix this bug.

by Claude Paroz, 12 years ago

Attachment: 18575-2.diff added

Removed kwargs changes from the previous patch

comment:4 by Stephen Burrows, 12 years ago

Triage Stage: AcceptedReady for checkin

Looks straightforward enough & addresses previous concerns. Tests still all passing... didn't think they wouldn't, but just ftr.

comment:5 by Stephen Burrows, 12 years ago

Resolution: needsinfo
Status: newclosed
Triage Stage: Ready for checkinAccepted

Turns out the patch doesn't actually solve the issue, since django.db is imported in django.test.client, and ImproperlyConfigured is still raised. Ah well.

That brings me back to my original reaction: Is there a benefit of supporting settings files that don't define any databases? Django clearly depends on the database for a lot of its functionality, and it's pretty simple to just use SQLite.

If there is a use case that you feel needs to be addressed, please add some more information on it to the ticket and reopen it.

in reply to:  5 comment:6 by Preston Holmes, 12 years ago

Resolution: needsinfo
Status: closedreopened

Replying to melinath:

Django clearly depends on the database for a lot of its functionality, and it's pretty simple to just use SQLite.

The key bit there is that 'a lot of' != 'all of' - but more importantly this used to work, and so is a regression - even if it is an uncommon one.

Unfortunately issues with the test framework are hard to test inside of Django - a little too recursive - and so will require some manual testing with sample projects.

The django.test.client shouldn't be an issue, as you should be able to use straight python unittest

comment:7 by Jan Bednařík, 12 years ago

Cc: jan.bednarik@… added
Resolution: wontfix
Severity: Release blockerNormal
Status: reopenedclosed

Did you notice that you can't run python manage.py shell without database?

Patch doesn't work and it's impossible to fix this.

When do you execute from django.test import TestCase in app/tests.py then Python executes django.test.__init__. There is imported one or more modules/classes which imports django.db.connection (or their dependency do). django.test.client is just first of them. If you remove it from django.test.__init__, then another module/class will import django.db.connection. And so on. Import from any module inside django.db will execute django.db.__init__ which will raise exception ImproperlyConfigured.

Defining default database (e.g. SQLite :memory:) will not break your code. So this is not not bug at all. Django just depends on database.

comment:8 by Claude Paroz, 12 years ago

Resolution: wontfix
Severity: NormalRelease blocker
Status: closedreopened

We *want* to be able to use Django without a database. That's why we have a dummy backend after all (as Alex Gaynor reminded us on IRC).

comment:9 by Jan Bednařík, 12 years ago

Yesterday I tried DATABASES = {} as author of this ticket did. This configuration is not loading dummy backend and everything raises ImproperlyConfigured. Even manage.py shell is not working. I checked comment in dummy backend base.py which says "Right now the dummy backend is there for raising ImproperlyConfigured if database configuration is missing." so I thinked that all those raises are correct and Django can't run without database settings.

But with setting database engine as django.db.backends.dummy or removing whole databases configuration from settings is a different story. Shell works fine. Tests fail on creating database:

$ ./manage.py test myapp -v 2
Creating test database for alias 'default' ('test_')...
ImproperlyConfigured: settings.DATABASES is improperly configured. Please
supply the ENGINE value. Check settings documentation for more details.

I see here two problems:

  1. In docs is default DATABASES setting empty dict. But when you set DATABASES = {} in project settings (which doesn't use dummy backend) it doesn't work like real default without any DATABASES in project settings (which uses dummy backend).
  1. Test runner does not know how to handle dummy database.

in reply to:  9 comment:10 by Claude Paroz, 12 years ago

Replying to Architekt:

I see here two problems:

  1. In docs is default DATABASES setting empty dict. But when you set DATABASES = {} in project settings (which doesn't use dummy backend) it doesn't work like real default without any DATABASES in project settings (which uses dummy backend).

Maybe this ticket could be solved by fixing the settings documentation (suggesting to not set DATABASES to {})? We could also add a note in the testing docs.

  1. Test runner does not know how to handle dummy database.

I've created #19192 to track this issue (not a blocker).

comment:11 by Claude Paroz, 12 years ago

#18053 was a duplicate. Read also the interesting comment from Preston (ticket:18053#comment:2)

by Claude Paroz, 12 years ago

Attachment: 18575-3.diff added

Allow setting DATABASES to empty dict (will default to dummy engine)

comment:12 by Jan Bednařík, 12 years ago

Triage Stage: AcceptedReady for checkin

Patch 3 reviewed.

comment:13 by Aymeric Augustin, 12 years ago

Owner: changed from nobody to Aymeric Augustin
Status: reopenednew

comment:14 by Aymeric Augustin, 12 years ago

This ticket essentially re-opens #16873.

comment:15 by Aymeric Augustin, 12 years ago

Claude's latest patch looks good. I'd like to hear from the people involved on #16873 before making a decision.

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

comment:16 by Preston Holmes, 12 years ago

In handsight - I would have closed this as a dupe of #18053 instead of the other way - that ticket preceded this and represents the general case, while this is a specific case.

A question we should ask is how can we leave the best trail for others to understand what is happening - at a minimum, I think we should add a comment to global_settings explaining that initialization will set the connection to use the dummy backend if not set in settings.py

At that point I'm not sure whether having global settings be {} or leave it set to dummy backend as is currently the case matters - the comment will hopefully clarify for anyone who sets no DB or sets it to {} and wonders how the dummy backend got involved.

Perhaps adding a TODO comment in the db.init code noting my idea in #18053 of tweaking code to be explicit about when a DB is actually needed.

comment:17 by Aymeric Augustin, 12 years ago

To sum up, the patch for this ticket should:

  • activate the dummy backend when settings.DATABASES is empty,
  • change the default value in global_settings.py to match the documentation, and add a comment explaining that the dummy backend will be used if the setting is left empty,

18575-3.diff is nearly RFC — it's only missing the comment.

I checked with Carl on IRC that this is an improvement over #16873. At the time, no one had thought that people would write DATABASES = {} in their settings files, so this use case wasn't supported.

Making the test runner aware of the dummy backend — so it doesn't attempt to create the database before the tests and drop it after — was forked to #19192.

comment:18 by Claude Paroz <claude@…>, 12 years ago

Resolution: fixed
Status: newclosed

In f1cc2be0c53858b673afc3b26347d3bb25e424f6:

Fixed #18575 -- Empty DATABASES should default to dummy backend

Thanks delormemarco@… for the report.

comment:19 by Claude Paroz <claude@…>, 12 years ago

In b4627bcabe6704c5bd1cbfb2d3a1421e05c29cd7:

[1.5.x] Fixed #18575 -- Empty DATABASES should default to dummy backend

Thanks delormemarco@… for the report.

Backport of f1cc2be0c from master.

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