Opened 14 years ago

Closed 12 months ago

#14611 closed New feature (fixed)

Added dedicated option to test.Client methods (other than get()) for passing query parameters.

Reported by: Jari Pennanen Owned by: Tom Carrick
Component: Testing framework Version: 1.2
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a view that takes GET and POST parameters, currently there exists django.test.Client.post() and Client.get() but they don't provide means to test this combination cleanly.

As an example solution I suggest to implement Client.request() also which would take POST and GET parameters, e.g.

c = Client()

c.request(reverse('my-view-name'), 
          get={'getvar' : 'value'}, 
          post={'postvar' : 'value'}, ...)

Currently workaround is to fake this using the post method like this:

c = Client()

c.post(reverse('my-test-view') + '?' + django.utils.http.urlencode({'getvar' : 'value'}), 
       {'postvar' : 'value'}, ...)

Which as said, is not very clean.

Change History (16)

comment:1 by Daniel F Moisset, 14 years ago

Owner: changed from nobody to Daniel F Moisset

comment:2 by Daniel F Moisset, 14 years ago

Needs documentation: set
Needs tests: set
Owner: changed from Daniel F Moisset to nobody
Triage Stage: UnreviewedAccepted

I dislike the TestClient.request method, because it hides which actual HTTP method is used. The only way to do an HTTP POST should be TestClient.post to avoid ambiguity.

However I think that this would work pretty well:

c = Client()
c.post(reverse('my-view-name'), 
       getdata={'getvar' : 'value'}, 
       data={'postvar' : 'value'}, ...)

i.e., adding a getdata kwarg to the post method. This is useful and doesn't break compatibility, and we still keep a single obvious way to do POST requests.

comment:3 by Jari Pennanen, 14 years ago

Yes you are totally right.

Though I suggest for backwards compatibility to put the getdata as at least third kwarg, if not last, since people probably use the Client.post without specifying the kwarg name, like:

c = Client()
c.post(reverse('my-view-name'], {'postvar': 'value'}, ...)

comment:4 by Julien Phalip, 13 years ago

Severity: Normal
Type: New feature

comment:5 by Aymeric Augustin, 13 years ago

Easy pickings: unset
Resolution: wontfix
Status: newclosed
UI/UX: unset

To be honest, the title of this ticket doesn't make much sense given how HTTP works. There are two orthogonal mechanisms involved here:

  • 1) HTTP methods
    • Client.get performs a GET request;
    • Client.post performs a POST request.
  • 2) HTML forms
    • when submitted with a GET, form data is encoded in the query string, and Django exposes it in request.GET;
    • when submitted with a POST, it's in the request body, and Django exposes it in request.POST.

The data argument of the test client is designed to hold simulated form data, and is automatically inserted in requests according to these rules.


That said, it's absolutely legal to use query string parameters in the URL of a POST request. But that isn't be form data. Adding an argument called get_data would be extremely confusing.

In addition, I'm not eager to add a query_string_parameters argument to the test client, because Django favors clean URL design. I'm also afraid of possible confusions with form data in GET requests.

So, for all these reasons, I'm going to reject this ticket. The recommended solution is to use self.client.post('/my/url/?param=value', form_data).

comment:6 by Dan Strokirk, 17 months ago

Would it be possible to reopen and reevaluate this ticket? It’s 11 years later, and I feel this would be a great, simple addition to Django’s testing toolset.

in reply to:  6 ; comment:7 by Mariusz Felisiak, 17 months ago

Replying to Dan Strokirk:

Would it be possible to reopen and reevaluate this ticket? It’s 11 years later, and I feel this would be a great, simple addition to Django’s testing toolset.

For now, I don't see a consensus in the discussion and we need it to reopen the ticket. I'd wait a little longer and get more opinions.

in reply to:  7 ; comment:8 by Lily Foote, 13 months ago

Replying to Mariusz Felisiak:

Replying to Dan Strokirk:

Would it be possible to reopen and reevaluate this ticket? It’s 11 years later, and I feel this would be a great, simple addition to Django’s testing toolset.

For now, I don't see a consensus in the discussion and we need it to reopen the ticket. I'd wait a little longer and get more opinions.

I think we have consensus in the linked thread and should reopen.

in reply to:  8 comment:9 by Mariusz Felisiak, 13 months ago

Summary: django.test.Client is missing post *and* get possibilityAdded dedicated option to test.Client methods (other than get()) for passing query parameters.

Replying to Lily Foote:

I think we have consensus in the linked thread and should reopen.

Thread discuss adding a dedicated keyword-argument, but I don't see any concrete proposition. How would you like to call it to avoid confusion? query_params maybe.

comment:10 by Lily Foote, 13 months ago

The forum discussion suggested that query_params is the most consistent with similar apis in Django. I'd go with that.

comment:11 by Mariusz Felisiak, 13 months ago

Needs documentation: unset
Needs tests: unset
Resolution: wontfix
Status: closednew

Accepted based on the forum discussion.

comment:12 by Tom Carrick, 12 months ago

Owner: changed from nobody to Tom Carrick
Status: newassigned

comment:13 by Tom Carrick, 12 months ago

Has patch: set

comment:14 by Mariusz Felisiak, 12 months ago

Patch needs improvement: set

We need to decide what to do with get() and head().

comment:15 by Mariusz Felisiak, 12 months ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 12 months ago

Resolution: fixed
Status: assignedclosed

In a0359396:

Fixed #14611 -- Added query_params argument to RequestFactory and Client classes.

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