[Django] #21250: Make Remote User tests more flexible

2 views
Skip to first unread message

Django

unread,
Oct 9, 2013, 11:28:53 AM10/9/13
to django-...@googlegroups.com
#21250: Make Remote User tests more flexible
--------------------------------------+--------------------
Reporter: elbarto | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: 1.5
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
I recently had to implement my own middleware with a custom HTTP header,
not REMOTE_USER, like:

{{{
from django.contrib.auth.middleware import RemoteUserMiddleware

class MyRemoteUserMiddleware(RemoteUserMiddleware):
header = 'MY_HTTP_AUTHUSER'
}}}

This is perfect, but then I needed to test my code, so my first idea was
to inherit from Django tests overriding my middleware config (I don't know
if it's a bad idea) like:

{{{
from django.contrib.auth.tests import RemoteUserTest

class MyRemoteUserTest(RemoteUserTest):
middleware = 'path.to.MyRemoteUserMiddleware'
}}}

since it would save me to write a lot of code.

The problem is that all tests in such module (
https://github.com/django/django/blob/master/django/contrib/auth/tests/test_remote_user.py
) are dependent for REMOTE_USER, because of many REMOTE_USER keywords are
passed to self.client.get() and I was forced to duplicate all the code
simply changing REMOTE_USER keyword for my header.

I think it would be great if it could be more flexible.

I was thinking about something like:

{{{
from django.contrib.auth.tests import RemoteUserTest

class MyRemoteUserTest(RemoteUserTest):
middleware = 'path.to.MyRemoteUserMiddleware'
header = 'MY_HTTP_AUTHUSER'
}}}

and then changing:

{{{
response = self.client.get('/remote_user/', REMOTE_USER=None)
}}}

for:
{{{
response = self.client.get('/remote_user/', **{self.header: None})
}}}

in all calls.

Or maybe instantiate the middleware (I don't know if it's possible) and
getting its self.header.

It would make a lot of easier to implement tests (without duplicating
code) when is needed to change the header for the Remote User middleware.

If it were accepted I could code the patch myself.

--
Ticket URL: <https://code.djangoproject.com/ticket/21250>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Oct 9, 2013, 11:50:01 AM10/9/13
to django-...@googlegroups.com
#21250: Make Remote User tests more flexible
--------------------------------------+------------------------------------

Reporter: elbarto | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by timo):

* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted


Comment:

I would say that our tests probably shouldn't be considered a stable API
that you can re-use, but if cleaning things up by making them more DRY has
the benefit of allowing you to use them and you are willing to submit a
patch, it seems ok.

--
Ticket URL: <https://code.djangoproject.com/ticket/21250#comment:1>

Django

unread,
Dec 6, 2013, 6:50:34 PM12/6/13
to django-...@googlegroups.com
#21250: Make Remote User tests more flexible
--------------------------------------+------------------------------------
Reporter: elbarto | Owner: elbarto
Type: Cleanup/optimization | Status: assigned

Component: Testing framework | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by elbarto):

* status: new => assigned
* owner: nobody => elbarto


--
Ticket URL: <https://code.djangoproject.com/ticket/21250#comment:2>

Django

unread,
Dec 8, 2013, 7:01:32 AM12/8/13
to django-...@googlegroups.com
#21250: Make Remote User tests more flexible
--------------------------------------+------------------------------------
Reporter: elbarto | Owner: elbarto
Type: Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 1.5
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by elbarto):

Fixed in https://github.com/django/django/pull/2051

The test name was inspired by https://docs.djangoproject.com/en/dev/howto
/auth-remote-user/#configuration

--
Ticket URL: <https://code.djangoproject.com/ticket/21250#comment:3>

Django

unread,
Dec 14, 2013, 1:06:33 PM12/14/13
to django-...@googlegroups.com
#21250: Make Remote User tests more flexible
--------------------------------------+------------------------------------
Reporter: elbarto | Owner: elbarto
Type: Cleanup/optimization | Status: closed

Component: Testing framework | Version: 1.5
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Tim Graham <timograham@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"8f994f1bccfe4309d63433d78978109a32f0826b"]:
{{{
#!CommitTicketReference repository=""
revision="8f994f1bccfe4309d63433d78978109a32f0826b"
Fixed #21250 -- Made HTTP auth user header configurable in tests

Currently, if the authentication mechanism uses a custom HTTP header
and not REMOTE_USER, it is not easy to test. This commit modifies
remote user tests in order to make them more generic.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/21250#comment:4>

Reply all
Reply to author
Forward
0 new messages