Code

Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#21164 closed Bug (fixed)

get_user_model() does not handle custom user models for testing purposes

Reported by: raymond.penners@… Owned by: nobody
Component: contrib.auth Version: 1.6-alpha-1
Severity: Release blocker 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

I would have expected the following test code to work:

from django.contrib.auth import get_user_model

@override_settings(AUTH_USER_MODEL='auth.CustomUser')
class CustomUserAccountTests(TestCase):

    def test_user(self):
        User = get_user_model()

Yet, get_user_model does not seem to cope with the idea of a model being defined in the tests package:

ImproperlyConfigured: AUTH_USER_MODEL refers to model 'auth.CustomUser' that has not been installed

Attachments (1)

django_issue_21164.tar.gz (3.5 KB) - added by raymond.penners@… 10 months ago.
Example project reproducing this issue

Download all attachments as: .zip

Change History (18)

comment:1 Changed 10 months ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to needsinfo
  • Status changed from new to closed

I'm not denying you have a problem in your own test suite, but there's obviously an important piece of the puzzle missing from your report - Django's own test suite does exactly what you refer to (c.f., django/contrib/auth/tests/test_basic.py), and it works fine.

The first possible problem -- Is your sample code *literally* what is in your project, or have you edited it? The code you've provided won't work as literally provided -- the auth app doesn't contain a CustomUser, so the error message is completely accurate.

Changed 10 months ago by raymond.penners@…

Example project reproducing this issue

comment:2 Changed 10 months ago by raymond.penners@…

You can run the example project like this to reproduce the problem:

$ python manage.py test example
Creating test database for alias 'default'...
E
======================================================================
ERROR: test_user (django_issue_21164.example.tests.CustomUserAccountTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/pennersr/src/dt/django_issue_21164/django_issue_21164/example/tests.py", line 10, in test_user
    User = get_user_model()
  File "/Users/pennersr/src/dt/venv/lib/python2.7/site-packages/django/contrib/auth/__init__.py", line 127, in get_user_model
    raise ImproperlyConfigured("AUTH_USER_MODEL refers to model '%s' that has not been installed" % settings.AUTH_USER_MODEL)
ImproperlyConfigured: AUTH_USER_MODEL refers to model 'auth.CustomUser' that has not been installed

----------------------------------------------------------------------
Ran 1 test in 0.001s

FAILED (errors=1)
Destroying test database for alias 'default'...

comment:3 Changed 10 months ago by anonymous

  • Resolution needsinfo deleted
  • Status changed from closed to new

comment:4 Changed 10 months ago by russellm

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

Thanks for the example project -- however, you've just reinforced what I said in my initial comments. Your code says

@override_settings(AUTH_USER_MODEL='auth.CustomUser')

However, auth.CustomUser *does not exist*. There is no such model. Hence the error "AUTH_USER_MODEL refers to model 'auth.CustomUser' that has not been installed" is completely accurate.

comment:5 Changed 10 months ago by raymond.penners@…

Then the documentation over at https://docs.djangoproject.com/en/dev/topics/auth/customizing/#custom-users-and-testing-fixtures is very misleading.

The examples suggest that this is the way to test with a customer user model:

class ApplicationTestCase(TestCase):
    ...

    @override_settings(AUTH_USER_MODEL='auth.CustomUser')
    def test_custom_user(self):
        "Run tests for a custom user model with email-based authentication"
        self.assertSomething()

However, attempting to do so fails (as shown by my example).

comment:6 Changed 10 months ago by raymond.penners@…

Sorry for reopening once more, but I would like to file this as a documentation bug then.

Please see the following lines taken from https://docs.djangoproject.com/en/dev/topics/auth/customizing/#custom-users-and-testing-fixtures

[...]
To ensure that your test suite will pass in any project configuration
[...]
tests may also be needed to be added to ensure that the application works with any user model
[...]
To assist with this, Django provides two substitute user models that can be used in test suites:
[...]
django.contrib.auth.tests.custom_user.CustomUser
django.contrib.auth.tests.custom_user.ExtensionUser
[...]
You can then use the @override_settings decorator to make that test run with the custom User model.
[...]
@override_settings(AUTH_USER_MODEL='auth.CustomUser')

So, unless I am mistaken, the example project follows the documentation to the letter.

comment:7 Changed 10 months ago by raymond.penners@…

  • Resolution invalid deleted
  • Status changed from closed to new

comment:8 Changed 10 months ago by russellm

  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.5 to 1.6-alpha-1

Ok - now I see what's going on - this is a regression that has been introduced in 1.6 as a result of the introduction of the new discovery test runner. Your sample project runs fine under Django 1.5.

In Django 1.5, the auth tests are loaded, which causes the test users to be loaded as well. In Django 1.6, the discovery test runner means that the auth tests aren't executed - and as a result, the test Users aren't installed.

I'm not sure what the fix will be here, but it's clearly a change in behavior that needs to be dealt with before the final release.

Thanks for being politely persistent :-)

comment:9 Changed 10 months ago by carljm

I didn't realize that the custom-user docs recommended this approach for testing. I was under the impression (from discussion in #7835) that models in test files being installed for test runs (only) was an undocumented accident of implementation, not a supported feature. (In any case, the new runner still supports it -- still only as an accident of implementation -- but only in test files that are actually being run, which can differ with the new runner, as observed here.)

I think the best solution for 1.6 is to no longer recommend reusing models from the auth tests, and instead recommend that people create their own custom user model if they need to test with one. A note about this could also be added to the release-notes backwards-incompatibility section about the new test runner.

I think the only kinds of apps that need to be writing tests against various AUTH_USER_MODEL settings are reusable ones, and personally I think for testing reusable apps it's more robust to put your test models in a models.py of a test-only (sub)app, which you include in the INSTALLED_APPS setting in the standalone script that runs your reusable app's tests. (Instead of relying on the "models in test files" behavior.) This approach doesn't let you run the reusable app's tests from a "real project", but the discovery runner already discourages doing that anyway.

comment:10 Changed 10 months ago by carljm

  • Has patch set

Here's a proposed patch implementing that approach: https://github.com/carljm/django/compare/ticket21164

comment:11 Changed 10 months ago by russellm

@carljm Re #7835: I agree that the documented behavior (CustomAuth being installed simply by virtue of being in the test suite) is a side effect, not strictly intended behavior. The purpose of that ticket came from my original work with schema evolution -- I was looking for ways to introduce a model for the purpose of a single test, and then reset that model at the end of the test (or reset the model *at* the end of the test to validate that the database matched what I expected). The key factor here is that a test case needed to be able to say "I need this model X to be in the app cache", and have that be the case fo a single test run. I wouldn't have expected models to exist simply by virtue of being in the test directory.

To that end, an alternate approach to this ticket would be to require users to put:

from django.contrib.auth.tests.test_custom_user import CustomUser

at the top of the test that requires auth.CustomUser. This would cause an explicit loading of the required User model, and because of Meta:app_label, it will appear in the auth app as expected. The alternative it so get every app with a dependency on User to ship with a "CustomUser" that is a replica of the stub provided by contrib.auth, which seems like a waste of effort to me.

comment:12 Changed 9 months ago by carljm

@russellm That approach doesn't depend on the auth tests being run, but it does still depend on the previously-undocumented "models in test files are installed during test runs" behavior. I'm not sure that's behavior that we really want to solidify with a backwards-compatibility guarantee, but pragmatically given that the testing-custom-users docs already rely on it, I'm ok with your suggestion.

comment:13 Changed 9 months ago by russellm

@carljm -- I don't think the "models in test models" thing is documented, but it's definitely in wide use -- Django's own test suite has a couple of places where models are defined in the tests package -- (e.g., the app_cache tests). In most cases it's a non-issue, because if you're running the tests for a module, then you're probably importing everything in that module anyway. This is an odd case because the auth tests won't be run, but the models we want are stored in the auth tests.

Based on your feedback, I'll commit a patch to docs that clarifies the need for an explicit import.

comment:14 Changed 9 months ago by Russell Keith-Magee <russell@…>

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

In ddb53856b62506128a6d19184dfa80da73c77db8:

Fixed #21164 -- Added documentation for issue with test users.

The package renaming restores the older package names (which were also the
documented package names). This doesn't affect test discovery because the
module in question doesn't contain any tests.

Thanks to Carl for the design discussion.

comment:15 Changed 9 months ago by Russell Keith-Magee <russell@…>

In 1ab84b6c653c3169994064f7c1772a8e869c7282:

[1.6.x] Fixed #21164 -- Added documentation for issue with test users.

The package renaming restores the older package names (which were also the
documented package names). This doesn't affect test discovery because the
module in question doesn't contain any tests.

Thanks to Carl for the design discussion.

Backport of ddb5385 from master.

comment:16 Changed 9 months ago by Claude Paroz <claude@…>

In ef22d512b54cbf0b5f76151bc760391d839fbf72:

Imported custom user classes in tests depending on it

Without those imports, affected test files cannot be run
independently. Refs #21164.

comment:17 Changed 9 months ago by Claude Paroz <claude@…>

In 4a9bae0b39aebdedf38fcb760405c04b8216c508:

[1.6.x] Imported custom user classes in tests depending on it

Without those imports, affected test files cannot be run
independently. Refs #21164.

Backport of ef22d512b5 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.