Contrib tests throwing errors on bare project when cache middleware enabled and cache specified

[Jim Dalton]

I ran into an issue recently where the contrib tests were throwing multiple errors during a test run when cache middleware was enabled and a cache was specified.

This is exactly the behavior described in this Stack Overflow question: ​http://stackoverflow.com/questions/3219668/using-django-cache-middleware-causes-contrib-auth-unit-tests-to-fail

Here is a description of the setup and the steps to reproduce the problem (taken from that question):

My Middleware:

MIDDLEWARE_CLASSES = (
    'django.middleware.cache.UpdateCacheMiddleware',
    'django.middleware.common.CommonMiddleware',
    'django.contrib.sessions.middleware.SessionMiddleware',
    'django.middleware.csrf.CsrfViewMiddleware',
    'django.contrib.auth.middleware.AuthenticationMiddleware',
    'django.contrib.messages.middleware.MessageMiddleware',
    'django.middleware.cache.FetchFromCacheMiddleware',
)

All my test failures look like the one below:

======================================================================
Error: test_last_login (django.contrib.auth.tests.remote_user.RemoteUserTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python26\lib\site-packages\django\contrib\auth\tests\remote_user.py",
 line 87, in test_last_login
    self.assertNotEqual(default_login, response.context['user'].last_login)
TypeError: 'NoneType' object is unsubscriptable

Steps to Reproduce:

1. Start a new django project (django-admin.py startproject myproject) and configure settings.py
2. Add CACHE_BACKEND to settings.py and add the two Cache Middlewares from Django
3. Run python manage.py test

I have an idea as to the cause and a proposed patch to resolve this, but I will add it with a separate comment to keep the description of the issue clean here.

[Jim Dalton]

I have traced the cause of these errors to some of the views that are created by contrib.auth and contrib.messages, in the urls.py file of each. These views are created for the purposes of running tests against them.

The problem is that some of the these views are hit multiple times in a test run. When a view is hit a second time, the cache is returned to the test client (it would appear). In any case, the context attribute, which is normally set by the test client in the 'response is not set (i.e. it is None`) when a cache page is retrieved.. As a result, the tests cause an error like the one in the description of the ticket.

My proposed solution to the problem is to affix the @never_cache decorator to the views created by the contrib apps for testing. These views are only used in testing, and no aspect of the caching system is tested during those test runs. In fact, the desired behavior of the views, per the test code, *is* to get a fresh response, with context.

A patch with this solution is attached. Applying this patch and then re-running the tests with the setup described in the ticket description will result in a clean test run (no errors or failures).

[Jim Dalton]

Oops, please ignore the first patch cleanup_session_records_after_invalid_key_test.diff and use dont_cache_test_views.diff. I accidentally uploaded the wrong patch, and I'm not sure how to delete it from trac.

[Russell Keith-Magee]

To clarify -- this only appears to be a problem for the memcache backend; locmem doesn't seem to cause any issues.

I'm not completely convinced that @never_cache is the right solution here. Unit tests shouldn't require any special preparation to enable them to be test isolated -- the testing environment should provide those guarantees. However, any test involving caching will cause data to be put into a cache that can be easily lost.

Django's test framework clears the database at the end of every test for exactly this reason; there is an argument that the cache should be flushed in the same way. However, I can see two problemw with this:

1) Accidentially flushing a cache that contains other useful data (e.g., the production cache)
2) The cost of cache flushing when there is no cache used during a test.

[Jim Dalton]

Thanks Russ.

To clarify, for at least a few of the test errors related to this issue, the page is viewed multiple times in the same test *method*, e.g. RemoteUserTest.test_no_remote_user() in contrib.auth.tests. (I said test "run" above, which was inaccurate.) So it doesn't really matter whether we get the teardown right or not...the test needs to ensure the view is not cached in order to be run correctly. The same is true in a few of the other test methods (though I have not gone through and systematically analyzed whether the methods that are throwing errors are all calling the same view multiple times in a test method). So I think in this case ensuring the view is not cached is really the only satisfactory way. The only alternative would be to determine the cache key used to save the response and go in and delete it, which seems burdensome.

I do agree with your overall point. We had a similar discussion on a similar issue #15026. The fact is, the Django test suite does an excellent job of restoring most of the major systems (e.g. DB, mail outbox) to their pre-test state state for each test method run, but cache (memcached at least) does not. I agree that calling flush() is problematic, because there are many examples where that would be undesirable behavior. I don't have any great ideas as to how one could clean up the cache after each test but only flush those keys that were set during that test, but maybe there's an idea lurking somewhere there...

[Jim Dalton]

I was just reminded of this issue again this a.m. when I ran my tests against a fresh copy of the Django trunk and this problem popped up for me again.

Given that this issue is causing failure in the contrib tests with a fairly typical configuration, I'd like to at a minimum suggest that the ticket gets marked as a blocker, to ensure a fix makes it into 1.3?

If the patch I submitted needs improvement I'm open to additional suggestions, etc.

[Raphael Kubo da Costa]

Hi there.

Isn't this a duplicate of ticket #14105?

[Russell Keith-Magee]

@rakuco -- Looks like it is.

@jsdalton -- Thanks for the analysis; the sounds reasonable to me, and your patch looks good from first inspection. Marking RFC to remind me to take a look at getting this to trunk.

[Russell Keith-Magee]

In [15865]:

Fixed #15142 -- Force test views to be non-cached so that projects with caching middleware enabled don't cause test failures. Thanks to jsdalton for the report and patch

[Russell Keith-Magee]

In [15866]:

[1.2.X] Fixed #15142 -- Force test views to be non-cached so that projects with caching middleware enabled don't cause test failures. Thanks to jsdalton for the report and patch

Backport of r15865 from trunk.

[Jacob]

Milestone 1.3 deleted