Opened 11 years ago

Closed 11 years ago

#21250 closed Cleanup/optimization (fixed)

Make Remote User tests more flexible

Reported by: elbarto Owned by: elbarto
Component: Testing framework Version: 1.5
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (4)

comment:1 by Tim Graham, 11 years ago

Triage Stage: UnreviewedAccepted

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.

comment:2 by elbarto, 11 years ago

Owner: changed from nobody to elbarto
Status: newassigned

comment:4 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: assignedclosed

In 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.

Note: See TracTickets for help on using tickets.
Back to Top