Tests fail with trivial usage of AUTH_PROFILE_MODULE

[Steve Lacy]

I started a totally blank project using 1.4 release (Version in Ticket in 1.4-rc2 because there's no 1.4 release in the ticket system yet).

I added a new app called "account" with a models.py that looks like this:

from django.db import models
from django.contrib.auth.models import User
    
class Account(models.Model):
    user = models.ForeignKey(User)

I put "account" in INSTALLED_APPS
I put "AUTH_PROFILE_MODULE = 'account.Account'" in settings.py

I ran python ./manage.py test and got:

ERROR: test_site_profile_not_available (django.contrib.auth.tests.models.ProfileTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/slacy/src/tmp/env/local/lib/python2.7/site-packages/django/contrib/auth/tests/models.py", line 29, in test_site_profile_not_available
    del settings.AUTH_PROFILE_MODULE
  File "/home/slacy/src/tmp/env/local/lib/python2.7/site-packages/django/utils/functional.py", line 215, in __delattr__
    delattr(self._wrapped, name)
AttributeError: AUTH_PROFILE_MODULE

----------------------------------------------------------------------
Ran 417 tests in 10.302s

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

[Steve Lacy]

I've used git bisect to narrow this down to commit(s):

e669c483ffcd8313b51a43f602ba2c8c74fd78bd
c88db46afdcacef8504c5831254b6d5592ac0cf0

Both of which are by aaugustin

[Aymeric Augustin]

I'm probably going to take a look at this, but the common practice here is *not* to assign tickets to other people. Assigning a ticket to oneself means "I'm currently working on a patch", and in this case, it isn't true.

[Claude Paroz]

Possible fix

[Aymeric Augustin]

I can see how this happened -- mixing override_settings and code to achieve the same effect manually in setUp and tearDown probably isn't a good idea.

We should refactor the test suite to use override_settings consistently. This is a much broader effort than fixing this specific bug.

It's also a n-th instance of the usual problem: "tests of contrib apps aren't sufficiently isolated"...

[rob@…]

I'm experiencing this issue also, on a completely blank project with AUTH_PROFILE_MODULE set.

[Sam Bull]

Is there a workaround for this issue? It's preventing us from migrating to 1.4 because our deployment system won't let us push out a release with failing tests.

[rob@…]

Replying to SamBull:

Is there a workaround for this issue? It's preventing us from migrating to 1.4 because our deployment system won't let us push out a release with failing tests.

I'm in the same situation, and would really like to migrate. I've been trying to figure out a workaround for this, and can't get the test to pass. I certainly don't want to use a custom version of Django in production.

[Aymeric Augustin]

Replying to SamBull:

Is there a workaround for this issue? It's preventing us from migrating to 1.4 because our deployment system won't let us push out a release with failing tests.

You should only test your own apps, not Django's contrib apps: ./manage.py test myapp1 myapp2 ...

[Sam Bull]

Replying to aaugustin:

Replying to SamBull:

Is there a workaround for this issue? It's preventing us from migrating to 1.4 because our deployment system won't let us push out a release with failing tests.

You should only test your own apps, not Django's contrib apps: ./manage.py test myapp1 myapp2 ...

Huh. That's not true is it? Some of django's tests are integration tests that help us verify that we're using everything correctly (providing essential templates, settings, etc.).

Is there any way to set the AUTH_PROFILE_MODULE setting without causing this test to error out?

Also, I take it this failure isn't getting caught by django's test suite, so shouldn't that be updated too?

[Carl Meyer]

Replying to SamBull:

Replying to aaugustin:

Replying to SamBull:

Is there a workaround for this issue? It's preventing us from migrating to 1.4 because our deployment system won't let us push out a release with failing tests.

You should only test your own apps, not Django's contrib apps: ./manage.py test myapp1 myapp2 ...

Huh. That's not true is it? Some of django's tests are integration tests that help us verify that we're using everything correctly (providing essential templates, settings, etc.).

That was the theory at one time, but it doesn't work very well in practice. There are so many ways to integrate contrib or other third-party apps, oftentimes with customizations, wrapped views, etc., and it's almost impossible for a reusable-app author to write tests that will reliably pass with all functional customized integrations. Over time, as "failing test" reports (just like this one) come in against contrib apps and are fixed, the tests become more and more isolated from the settings of the project they are running in (provide their own templates, temporarily override settings to known values, etc), and eventually lose any value they may have once had as integration tests.

Fundamentally, integration tests are the responsibility of the integrator, who is the only party who knows how their specific integration is supposed to work.

For Django 1.5 it is likely that the default test runner will not run tests from external apps by default. So yes, I would say that changing your continuous integration tests now to match that practice would be sensible, and I don't believe that in practice you'll lose significant value in terms of coverage of your code and settings.

[rob@…]

Replying to carljm:

Replying to SamBull:

Replying to aaugustin:

Replying to SamBull:

Is there a workaround for this issue? It's preventing us from migrating to 1.4 because our deployment system won't let us push out a release with failing tests.

You should only test your own apps, not Django's contrib apps: ./manage.py test myapp1 myapp2 ...

Huh. That's not true is it? Some of django's tests are integration tests that help us verify that we're using everything correctly (providing essential templates, settings, etc.).

That was the theory at one time, but it doesn't work very well in practice. There are so many ways to integrate contrib or other third-party apps, oftentimes with customizations, wrapped views, etc., and it's almost impossible for a reusable-app author to write tests that will reliably pass with all functional customized integrations. Over time, as "failing test" reports (just like this one) come in against contrib apps and are fixed, the tests become more and more isolated from the settings of the project they are running in (provide their own templates, temporarily override settings to known values, etc), and eventually lose any value they may have once had as integration tests.

Fundamentally, integration tests are the responsibility of the integrator, who is the only party who knows how their specific integration is supposed to work.

For Django 1.5 it is likely that the default test runner will not run tests from external apps by default. So yes, I would say that changing your continuous integration tests now to match that practice would be sensible, and I don't believe that in practice you'll lose significant value in terms of coverage of your code and settings.

I find that often the strange edge-cases are caught by running the tests in contrib, which my own tests may not necessarily cover (though this should obviously be rectified, if such a situation is discovered).

For example, the behaviour of custom middleware on login/logout views. I'd feel somewhat uncomfortable excluding Django's tests from the build (is there even a way to do this - or must you specify each of your own apps explicitly on the command line?)

[Carl Meyer]

I replied on #17365, as this discussion is really more on-topic there.

[robgolding63]

I've opened a pull request with a fix for this one: ​https://github.com/django/django/pull/36.

I know the use of GitHub is new to the Django workflow and some things still need to be worked out, but I figured having it there waiting is the easiest way to show the changes. It's just a single commit, which I think fits with Django's current procedures.

[Claude Paroz <claude@…>]

In [531878302735e6a2b36d82b584947bbf8eae8111]:

Fixed #17966 -- Isolated ProfileTestCase from custom AUTH_PROFILE_MODULE

Thanks Rob Golding for helping on the patch.

[Mathijs de Bruin]

Replying to Claude Paroz <claude@…>:

In [531878302735e6a2b36d82b584947bbf8eae8111]:

Fixed #17966 -- Isolated ProfileTestCase from custom AUTH_PROFILE_MODULE

Thanks Rob Golding for helping on the patch.

The patch seems to fix the initial problem but the tests now fail due to SiteProfileNotAvailable now not being raised in my specific setup. It seems as though override_settings is not doing quite what it ought to do:

• With AUTH_PROFILE_MODULE commented out in my settings, the test runs fine.
• When I comment out the full @override_settings line the test runs fine.
• When I remove the AUTH_PROFILE_MODULE override, the test fails with AttributeError: AUTH_PROFILE_MODULE as in the original bug report.
• When I remove the USE_TZ override, I get the SiteProfileNotAvailable exception not being raised.

No doubt this is a weird and difficult problem. Let me know if more feedback is required - I personally have no real clue what's going on here and do not have the time to delve much deeper but would love to help this bug get fixed.

Lastly, it seems that Rob Golding's original patch from the pull request passes the unittest just fine.

[Claude Paroz]

Please provide the necessary settings to be able to reproduce your problem. Please try also to debug the test to see why SiteProfileNotAvailable is not raised.

[anonymous]

Replying to dokterbob:

Replying to Claude Paroz <claude@…>:

In [531878302735e6a2b36d82b584947bbf8eae8111]:

Fixed #17966 -- Isolated ProfileTestCase from custom AUTH_PROFILE_MODULE

Thanks Rob Golding for helping on the patch.

The patch seems to fix the initial problem but the tests now fail due to SiteProfileNotAvailable now not being raised in my specific setup. It seems as though override_settings is not doing quite what it ought to do:

• With AUTH_PROFILE_MODULE commented out in my settings, the test runs fine.
• When I comment out the full @override_settings line the test runs fine.
• When I remove the AUTH_PROFILE_MODULE override, the test fails with AttributeError: AUTH_PROFILE_MODULE as in the original bug report.
• When I remove the USE_TZ override, I get the SiteProfileNotAvailable exception not being raised.

No doubt this is a weird and difficult problem. Let me know if more feedback is required - I personally have no real clue what's going on here and do not have the time to delve much deeper but would love to help this bug get fixed.

Lastly, it seems that Rob Golding's original patch from the pull request passes the unittest just fine.

Just wanted to note that I'm having the same issue on Python 2.7, Django 1.5.0. What seems to be occurring is that prior to line 15: del settings.AUTH_PROFILE_MODULE, settings.AUTH_PROFILE_MODULE reports itself as an empty string. However, after the del call, settings.AUTH_PROFILE_MODULE reports itself as the model defined in my settings.py. As a result, user.get_profile() is able to run fine and thus causes a test failure by not raising an error.

I was able to replicate the issue by doing the following:

• from a fresh project and app, creating a UserProfile model following these instructions: ​https://docs.djangoproject.com/en/dev/topics/auth/#storing-additional-information-about-users
• this includes registering a handler as shown in the instructions
• adding an AUTH_PROFILE_MODULE setting to settings.py pointing to the UserProfile model

At this point, running python manage.py test exhibits the problematic behavior described above where settings.AUTH_PROFILE_MODULE defaults to what I defined in settings.py after the attribute is deleted.

[Claude Paroz]

This problem is a symptom of #18824.

[robgolding63]

As far as I can see, moving the separate tests to their own functions would remove the need to use del settings.AUTH_PROFILE_MODULE, and so would work around this issue. As it stands there are 3 tests sitting in the same function (test_site_profile_not_available), so it probably makes more sense to break them out?

[Claude Paroz]

I don't think so. AUTH_PROFILE_MODULE can be set in the current project settings and therefore I don't see how we can avoid the del. However, you have a point in that the del would probably succeed if the test was not in an override_settings context. But then we should also restore it at the end of the test, exactly the sort of manipulation we want to avoid with override_settings. That's why I still favour the solution proposed in #18824.

[robgolding63]

My thinking was that override_settings is a class decorator, so by having AUTH_PROFILE_MODULE set to the empty string (or None), it's effectively unset at the start of each method call. We could then set it as required, or just leave it unset in this particular case.

Perhaps the only way that would work, though, is if the tests were in separate classes altogether - which isn't exactly ideal.

In any case, implementing #18824 will no doubt be useful in other places too - so that's probably the best course of action.

[alan.johnson@…]

I'm a rather new Django developer, and I'm trying to build a site in a test-driven way...is the takeaway of this that tests will always fail when following the UserProfile instructions from the docs? Is there a workaround?

[Claude Paroz]

This should be resolved after commit [22f85b9057825b5b4139abf5fd7e8c4ba0d16981].

@alan.johnson@…:
See comment:12