#35727 closed New feature (fixed)
Add `response.text_content` to responses
Reported by: | Adam Johnson | Owned by: | Jae Hyuck Sa |
---|---|---|---|
Component: | HTTP handling | 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
A common pattern in tests is to make assertions on the response content. HttpResponse.content is bytes
, requiring a call to .decode()
for assertions with str
values:
class DiggerLogTests(TestCase): def test_get(self): response = self.client.get('/digger/logs') assert 'Good digging' in response.content.decode() assert '2022-01-01 12:00:00' in response.content.decode()
This is suboptimal for a few reasons:
- It requires extra code.
- Mixed bytes/str errors may confuse beginners.
decode()
imparts a small but measurable overhead, especially if repeated for each assertion as above..decode()
defaults to UTF-8. Whilst this is by far the most common encoding, the response may actually use a different encoding, as per the content-type header.
I propose that responses from the test client include a new attribute, called text_content
. This would be cached property returning content
decoded into a str
, per any charset
in content-type
.
This new attribute would allow the above test to be written as:
class DiggerLogTests(TestCase): def test_get(self): response = self.client.get('/digger/logs') assert 'Good digging' in response.text assert '2022-01-01 12:00:00' in response.text
text_content
could be added either in HttpResponse
directly or attached by the test client, like response.json()
. I favour the first option a little more as it may come in useful outside of tests, and the cache invalidation when content
is mutated would be easier.
The new attribute could also be used to simplify assertContains
, which decodes on every call
I have wanted this for a while and Simon Willison suggested this feature to me on Twitter.
Change History (24)
comment:1 by , 4 months ago
Component: | HTTP handling → Testing framework |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 4 months ago
Ah yeah, I went back and forth on that when writing the ticket. Okay, let’s do text
.
comment:3 by , 4 months ago
Natalia, do you have any opinion on adding in HttpResponse
versus patching in the test client?
comment:4 by , 4 months ago
Component: | Testing framework → HTTP handling |
---|
Thanks for asking Adam! I missed that question/inquiry from the original description. I see now why you chose "HTTP handling" as Component :-).
I agree with your sentiment to make this a new feature/attribute for HttpResponse
, so I'll be switching the component back.
comment:5 by , 4 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:6 by , 4 months ago
Has patch: | set |
---|
comment:7 by , 4 months ago
Patch needs improvement: | set |
---|
comment:8 by , 3 months ago
Patch needs improvement: | unset |
---|
comment:9 by , 3 months ago
Needs documentation: | set |
---|
comment:10 by , 3 months ago
Needs documentation: | unset |
---|
comment:11 by , 3 months ago
Patch needs improvement: | set |
---|
comment:12 by , 3 months ago
Patch needs improvement: | unset |
---|
comment:13 by , 3 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:14 by , 3 months ago
Triage Stage: | Ready for checkin → Accepted |
---|
comment:15 by , 3 months ago
Patch needs improvement: | set |
---|
comment:16 by , 3 months ago
Hi! Sarah.
I’ve noticed that content.decode() is being used in many places within our current test code.
Do you think it would be necessary to make any modifications to this?
Example
comment:17 by , 3 months ago
Patch needs improvement: | unset |
---|
comment:18 by , 3 months ago
Patch needs improvement: | set |
---|
comment:19 by , 2 months ago
Patch needs improvement: | unset |
---|
comment:20 by , 2 months ago
Patch needs improvement: | set |
---|
comment:21 by , 2 months ago
Patch needs improvement: | unset |
---|
comment:22 by , 2 months ago
Triage Stage: | Accepted → Ready for checkin |
---|
I'm a big +1 for this one, I have suffered this multiple times in many projects.
One note though: the issue title and content proposes
text_content
for the new attr name, but the example usestext
. I personally prefertext
because it's shorter and the same thatrequests
exposes.