#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 , 14 months ago
| Component: | HTTP handling → Testing framework |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:2 by , 14 months ago
Ah yeah, I went back and forth on that when writing the ticket. Okay, let’s do text.
comment:3 by , 14 months ago
Natalia, do you have any opinion on adding in HttpResponse versus patching in the test client?
comment:4 by , 14 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 , 14 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:6 by , 14 months ago
| Has patch: | set |
|---|
comment:7 by , 14 months ago
| Patch needs improvement: | set |
|---|
comment:8 by , 14 months ago
| Patch needs improvement: | unset |
|---|
comment:9 by , 14 months ago
| Needs documentation: | set |
|---|
comment:10 by , 14 months ago
| Needs documentation: | unset |
|---|
comment:11 by , 14 months ago
| Patch needs improvement: | set |
|---|
comment:12 by , 14 months ago
| Patch needs improvement: | unset |
|---|
comment:13 by , 14 months ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:14 by , 14 months ago
| Triage Stage: | Ready for checkin → Accepted |
|---|
comment:15 by , 14 months ago
| Patch needs improvement: | set |
|---|
comment:16 by , 13 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 , 13 months ago
| Patch needs improvement: | unset |
|---|
comment:18 by , 13 months ago
| Patch needs improvement: | set |
|---|
comment:19 by , 12 months ago
| Patch needs improvement: | unset |
|---|
comment:20 by , 12 months ago
| Patch needs improvement: | set |
|---|
comment:21 by , 12 months ago
| Patch needs improvement: | unset |
|---|
comment:22 by , 12 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_contentfor the new attr name, but the example usestext. I personally prefertextbecause it's shorter and the same thatrequestsexposes.