Opened 6 years ago
Closed 6 years ago
#30024 closed New feature (fixed)
The test client request methods should raise an error when passed None as a data value
Reported by: | Jon Dufresne | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
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
Both GET and POST encoded data do not have a concept of None
or NULL
. The closest approximation is an empty string value or omitting the key. For example, in a GET request this could be either /my-url/?my_field=
or simply /my-url/
but not /my-url/?my_field=None
)
When onboarding new developers to projects, this can cause confusion to those less familiar with these details. For example, a new developer may try the following:
def test_setting_value_to_none(self): self.client.post('/my-url/', {'my_field': None}) self.assertIsNone(...)
In current versions of Django, behind the scenes, this None
gets coerced to the string 'None'
by the test client. The Django form field classes don't recognize the string 'None'
as an empty value (good) and so this test doesn't pass. Where the new developer thought a field would be assigned None
they instead get a form error. Depending on the developers' knowledge of these details, this could take much debugging or consulting a colleague.
I think we can recognize this pattern as a programming mistake and raise an informative error to guide the developer. I propose something like the following, but am open to suggestions:
TypeError: Cannot encode None as POST data. Did you mean to pass an empty string or omit the value?
For GET requests, the query string data is processed by django.utils.http.urlencode()
. So perhaps this same check can be done there as encoding None
in a URL query string as 'None'
is rarely the intended behavior. For those that really want the string 'None'
in the query string, they can pass the string 'None'
.
Change History (5)
comment:1 by , 6 years ago
Has patch: | set |
---|
comment:2 by , 6 years ago
I think this is fair. There's no case where None
is the right thing to use. (As Jon says, maybe 'None'
if you really mean that.)
comment:3 by , 6 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:7 by , 6 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
PR