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: unaizalakain Owned by: unaizalakain
Component: Core (Cache system) Version: master
Severity: Normal Keywords: tests, cache, request
Cc: bmispelon Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


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 unaizalakain

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to unaizalakain
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:2 Changed 3 years ago by unaizalakain

  • Has patch set

comment:3 Changed 3 years ago by bmispelon

  • Cc bmispelon added
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Bug to Cleanup/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 unaizalakain

  • 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 set to fixed
  • Status changed from assigned to closed

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 unaizalakain

Thanks for the review!

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