Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#21476 closed Cleanup/optimization (fixed)

Cache tests make an incorrect use of `HttpRequest`

Reported by: Unai Zalakain Owned by: Unai Zalakain
Component: Core (Cache system) Version: master
Severity: Normal Keywords: tests, cache, request
Cc: Baptiste Mispelon Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Cache tests directly append the query string to the request path:

request = HttpRequest()
request.META = {
    'SERVER_NAME': 'testserver',
    'SERVER_PORT': 80,
}
request.method = method 
request.path = request.path_info = "/cache/%s" % path
return request

HttpRequest.get_full_path() uses the HttpRequest.META['QUERY_STRING'] attribute to determine, escape and encode the query string.

Change History (6)

comment:1 Changed 3 years ago by Unai Zalakain

Needs documentation: unset
Needs tests: unset
Owner: changed from nobody to Unai Zalakain
Patch needs improvement: unset
Status: newassigned

comment:2 Changed 3 years ago by Unai Zalakain

Has patch: set

comment:3 Changed 3 years ago by Baptiste Mispelon

Cc: Baptiste Mispelon added
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

Yes, it seems that the tests have accumulated some cruft over the years and they could use some cleaning up.
I'm accepting the ticket on this basis.

In particular (though the list is probably not exhaustive):

  • Use django.test.RequestFactory instead of manually constructing HttpRequest objects
  • Get rid of the various inconsistent _get_request method on the test cases (they have different signatures, some prepend self.path and others don't, ...)

comment:4 Changed 3 years ago by Unai Zalakain

Patch needs improvement: unset

PR updated, all the _get_request mess has been substituted by django.test.client.RequestFactory ;)

comment:5 Changed 3 years ago by Baptiste Mispelon <bmispelon@…>

Resolution: fixed
Status: assignedclosed

In 2b4bed6dbd864ce78e9300f28b05045c386cd248:

Fixed #21476 -- Cache tests make an incorrect use of HttpRequest

Using django.test.client.RequestFactory solves the problem and cleans up all
the get_request mess.

comment:6 Changed 3 years ago by Unai Zalakain

Thanks for the review!

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