Code

Opened 4 years ago

Closed 3 years ago

#14105 closed (duplicate)

django.contrib.auth tests failing with cache middleware

Reported by: baumer1122 Owned by: PaulM
Component: Core (Cache system) Version: 1.2
Severity: Keywords:
Cc: christandiono@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Steps to reproduce:

  1. Start a new project. Add django.contrib.admin and setup a sqlite database. Test result: OK.
  1. Add the required cache settings and the update/fetch middleware. Tests result: FAILED (failures=1, errors=27)

I'm testing against a locmem cache with Django 1.2.1 and Python 2.6. I've attached my settings file for reference

Attachments (1)

diffsettings.py (1.1 KB) - added by baumer1122 4 years ago.

Download all attachments as: .zip

Change History (10)

Changed 4 years ago by baumer1122

comment:1 Changed 4 years ago by PaulM

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to PaulM
  • Patch needs improvement unset
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

I can duplicate this problem on trunk.

The test failures all seem to point to response.context being returned as None.

The failures are generally of this form:

======================================================================
ERROR: test_last_login (django.contrib.auth.tests.remote_user.RemoteUserCustomTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/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

comment:2 Changed 4 years ago by PaulM

This seems related to the fact that some of the auth tests add django.contrib.auth.midddleware.RemoteUserMiddleware to the end of settings.MIDDLEWARE_CLASSES, breaking the requirement that FetchFromCacheMiddleware be last. Working on a patch now.

comment:3 Changed 4 years ago by PaulM

Further investigation suggests that it's not a problem with the order of the middleware classes.

The final failure on the test run seems to be caused by django.contrib.auth.views.password_reset_confirm() not being properly decorated with @never_cache.

I'm still investigating the response.context issue, but I believe that the cache is bypassing part of the test runner.

comment:4 follow-up: Changed 4 years ago by toofishes

This bug comes down to the following line in client.py:224:

signals.template_rendered.connect(on_template_render, dispatch_uid="template-render")

The problem occurs when a view is rendered any subsequent time in the test run- we will never render any templates, causing our signal that attempts to trap template and context items to never fire, which in turn leads to context and template attributes on HttpResponse being left empty/undefined even though the response content itself and the status_code end up being correct.

I'm not sure of the solution here; they all seem ugly:

  • Always set CACHE_BACKEND to 'dummy://' when running tests- sucks if you actually want to test caching concerns like making sure different views go to different users even with caching enabled.
  • Set CACHE_BACKEND to 'dummy://' in supplied tests when we know we want to examine context or templates in the response.
  • Remove cache middleware explicitly from settings.MIDDLEWARE_CLASSES in these tests.
  • Call cache.clear() in test setup/teardown- seems like overkill and is potentially non-safe it people aren't careful about their setup.
  • Attempt to set template and context before the response ends up being cached, so when we later retrieve it we get the same results as the first non-cached run.

comment:5 Changed 4 years ago by PaulM

I opened #14446 with a patch to fix the single failure, since it seems to be a separate issue from the major reason this ticket is open.

comment:6 in reply to: ↑ 4 Changed 4 years ago by christandiono

Replying to toofishes:

  • Always set CACHE_BACKEND to 'dummy://' when running tests- sucks if you actually want to test caching concerns like making sure different views go to different users even with caching enabled.
  • Set CACHE_BACKEND to 'dummy://' in supplied tests when we know we want to examine context or templates in the response.

By the way, these don't seem to work, bizarrely enough. Our cache backend has long been set to dummy because we've been too lazy to set it up, although the two middlewares were still there (and causing tests to fail).

comment:7 Changed 3 years ago by christandiono

  • Cc christandiono@… added

comment:8 Changed 3 years ago by julien

  • Component changed from Contrib apps to Cache system

This is primarily a cache-related issue.

comment:9 Changed 3 years ago by russellm

  • Resolution set to duplicate
  • Status changed from assigned to closed

Closing as a duplicate of #15142; This ticket came first, but the other ticket has a more complete analysis and a proposed patch.

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.