Code

Opened 2 years ago

Closed 21 months ago

Last modified 21 months ago

#18575 closed Bug (fixed)

Testing framework doesn't work without a SQL Database

Reported by: delormemarco@… Owned by: aaugustin
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 claudep 23 months ago.
Late import of django.db.connections
18575-2.diff (1.1 KB) - added by claudep 22 months ago.
Removed kwargs changes from the previous patch
18575-3.diff (1.6 KB) - added by claudep 21 months ago.
Allow setting DATABASES to empty dict (will default to dummy engine)

Download all attachments as: .zip

Change History (22)

comment:1 Changed 2 years ago by claudep

  • Keywords regression added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

Changed 23 months ago by claudep

Late import of django.db.connections

comment:2 Changed 23 months ago by claudep

  • Has patch set

comment:3 Changed 22 months ago by aaugustin

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.

Changed 22 months ago by claudep

Removed kwargs changes from the previous patch

comment:4 Changed 22 months ago by melinath

  • Triage Stage changed from Accepted to Ready for checkin

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

comment:5 follow-up: Changed 22 months ago by melinath

  • Resolution set to needsinfo
  • Status changed from new to closed
  • Triage Stage changed from Ready for checkin to Accepted

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.

comment:6 in reply to: ↑ 5 Changed 22 months ago by ptone

  • Resolution needsinfo deleted
  • Status changed from closed to reopened

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 Changed 21 months ago by Architekt

  • Cc jan.bednarik@… added
  • Resolution set to wontfix
  • Severity changed from Release blocker to Normal
  • Status changed from reopened to closed

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 Changed 21 months ago by claudep

  • Resolution wontfix deleted
  • Severity changed from Normal to Release blocker
  • Status changed from closed to reopened

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 follow-up: Changed 21 months ago by Architekt

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.

comment:10 in reply to: ↑ 9 Changed 21 months ago by claudep

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 Changed 21 months ago by claudep

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

Changed 21 months ago by claudep

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

comment:12 Changed 21 months ago by Architekt

  • Triage Stage changed from Accepted to Ready for checkin

Patch 3 reviewed.

comment:13 Changed 21 months ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Status changed from reopened to new

comment:14 Changed 21 months ago by aaugustin

This ticket essentially re-opens #16873.

comment:15 Changed 21 months ago by aaugustin

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

The alternative is to document the actual default value and instruct people not to set DATABASES = {}. But that isn't as nice as automatically setting up the dummy backend.

Last edited 21 months ago by aaugustin (previous) (diff)

comment:16 Changed 21 months ago by ptone

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 Changed 21 months ago by aaugustin

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 Changed 21 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from new to closed

In f1cc2be0c53858b673afc3b26347d3bb25e424f6:

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

Thanks delormemarco@… for the report.

comment:19 Changed 21 months ago by Claude Paroz <claude@…>

In b4627bcabe6704c5bd1cbfb2d3a1421e05c29cd7:

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

Thanks delormemarco@… for the report.

Backport of f1cc2be0c from master.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.