Opened 7 years ago
Closed 7 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 , 7 years ago
| Has patch: | set |
|---|
comment:2 by , 7 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 , 7 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:7 by , 7 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
PR