Opened 11 years ago

Closed 11 years ago

Last modified 8 years ago

#8393 closed (worksforme)

Test client handler should call set_script_prefix

Reported by: Joost Cassee Owned by: nobody
Component: Testing framework Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Since [8015] every HTTP handler calls urlresolvers.set_script_prefix to set the prefix for reverse URL resolving. The only except in the handler used by the test client.

The problem I am solving may seem a bit esoteric. I'm trying to remove the necessity of monkey-patching the urlresolvers.reverse function in the (third-party) localeurl app. My solution is to append to the script prefix for the request, but because the test client handler does not reset it at every request my tests fail.

The attached patch makes the test client handler call set_script_prefix at every request.

Attachments (2)

8393-r8431.diff (990 bytes) - added by Joost Cassee 11 years ago.
8393-r8442.diff (845 bytes) - added by Joost Cassee 11 years ago.

Download all attachments as: .zip

Change History (11)

Changed 11 years ago by Joost Cassee

Attachment: 8393-r8431.diff added

comment:1 Changed 11 years ago by Joost Cassee

On second though, the test client should not depend on the environment. Just set the prefix to '/'. Updated patch.

Changed 11 years ago by Joost Cassee

Attachment: 8393-r8442.diff added

comment:2 Changed 11 years ago by Malcolm Tredinnick

milestone: 1.0 maybe

I'm a little worried that this is fixing some kind of problem at the wrong level. If you do absolutely nothing, it should be the same as if the script prefix is '/'. This is because lots of code that relies on the script prefix (anything calling reverse()) can be called outside of Django's request-response loop. For example, in a cronjob.

So I'm not sure why anything at all needs to be done here. If it does need to be done, why is that so? What fails if you don't make this change?

comment:3 Changed 11 years ago by Joost Cassee

I'm not sure what I can tell you that is not already in the ticket summary. The patch is trying to fix a problem with this (pseudo-)code in middleware:

def process_request(self, request):
    prefix = urlresolvers.get_script_prefix()
    prefix += <something>

Because the real handlers reset the prefix on every request this works. The test client, however, does not. This causes tests for this middleware to fail.

I notice now, however, that these handlers set the prefix after the middleware has loaded. I now wonder how come this code snippet (which I tested and use 'in real life') work.

comment:4 Changed 11 years ago by Joost Cassee

Bah, loading != processing.

comment:5 Changed 11 years ago by Jacob

milestone: 1.0 maybe1.0
Triage Stage: UnreviewedAccepted

comment:6 Changed 11 years ago by Jacob

Resolution: worksforme
Status: newclosed

Looking at urlresolvers.get_script_prefix, I can't see how it could fail -- if the prefix isn't set, it'll just return / anyway. Without any more details -- and psudocode won't cut it -- I can't verify this is actually a bug. I suspect it's a problem in your code.

I'll take a shot in the dark, I suspect that it's a matter of you not cleaning up after yourself in your middleware: if you modify the script prefix in process_request, you need to clean it up at some point during the response. Otherwise the script prefix will keep growing, and that's broken.

comment:7 Changed 11 years ago by Jacob

I should have also noted that if you can verify that it's a bug in Django please feel free to reopen the ticket.

comment:8 Changed 11 years ago by Joost Cassee

Jacob, it's definitely a resource-cleanup problem. Thanks for pointing that out. The prefix is reset (set_script_prefix is called) at every request (at least when using FastCGI or the development server), so I got away with it. The test client does not reset it, so my code failed there.

comment:9 Changed 8 years ago by Jacob

milestone: 1.0

Milestone 1.0 deleted

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