While adding a query_params parameter to test client and request factory methods, I hit the issue that for GET and HEAD requests we can already pass in a data parameter to pass query parameters. The data parameter is also used for form data in other request methods. Everyone (so far) agrees we shouldn’t have both for these methods.
I suggested deprecating the data parameter for GET and HEAD requests. @felixxm instead prefers not adding query_params to them and sticking with data.
I am going to assume that the reason for this is for backwards compatibility, but I’ll let him tell us if there are some other issues.
But the reasons I prefer the deprecation:
All methods will have the same API, I think this is less confusing for beginners (and everyone, honestly).
It’s also easier to use them programmatically. if you need to make lots of different types of requests with query parameters to a URL, you can iterate over the method names and call them all with the same parameters.
I think we can be a bit more lenient on deprecating things in tests, because we shouldn’t be breaking anyone’s site with this change, only their tests.
This kind of change could be done automatically with django-upgrade (though I’m not sure exactly how it works so do correct me if I’m wrong).
It feels to me like the code would be easier to read and maintain.
I feel like query_params and data are both correct names so given that data is backwards compatible and already in a lot of folks’ muscle memory I’d probably lean that way too
The data argument of the test client is designed to hold simulated form data, and is automatically inserted in requests according to these rules.
Ref django-upgrade: I’d love to make it official somehow, so that folks updating would get it automatically, but until such a time we have to assume that (relatively speaking) almost nobody in the user base has heard of it or is using it.
I feel strongly about the need to support the same param name (query_params) across all the test client HTTP methods, including GET and HEAD. Having worked with big and complex code bases for a long time, I can assure you there is some amount of “meta-programming” when writing tests[0], and this meta-programming benefits considerably from having consistency in these calls.
I’m in favor of:
Supporting both data and query_params for GET and HEAD, with a goal of eventually deprecating data
A deprecation warning is shown when running the test suite and there are calls that use both
query_params takes precedence over data when both are given (I think this is safe since a- developers defining query_params are reading and using the latest code/docs, and b- the warning will be shown anyways)
[0] Whether this is or is not a good practice and to which extent, we can discuss separately, but I have good examples where this is very handy and helps readability and maintainability of the test code.
I think a TypeError when both are provided sounds sensible. Silent precedence would lead to tests that look like they do something with the data argument but actually they do nothing.
I am against deprecating data. Yes, django-upgrade could rewrite many cases, as it already does for test client headers. But as Carlton says, the use of django-upgrade is still relatively rare, and it is unofficial. (And who knows if Ruff will duplicate it at some point .) Keeping support for data is cheap, and many tests use the positional argument format anyway, in which case the name doesn’t matter. Also, a precedent is that we didn’t deprecate support for the WSGI-style header format when adding headers.
Re: django-upgrade and a conversation elsewhere about showcasing third party packages, I feel like it’s at least worth a mention there (when it exists) and also in any documentation (do we have much other than the release notes) around upgrading to newer versions.
Back on topic, for now I will continue with the current approach (keep both, error when using both at the same time), but it looks like there are a lot of differing opinions here, though it seems there’s a lot of support for having query_params everywhere, and perhaps the fate of data doesn’t need to be addressed immediately.
)
.) Keeping support for